KAFKA-6772: Load credentials from ZK before accepting connections#4867
KAFKA-6772: Load credentials from ZK before accepting connections#4867rajinisivaram merged 4 commits intoapache:trunkfrom
Conversation
|
LGTM. Just for my understanding, Will there be any issue, if server starts receiving incoming connections only after completing the initialization of the server? |
|
@omkreddy Thanks for the review. At the moment we create and start the acceptors and processors in SocketServer together. We need to start acceptors early on in order to register brokers in ZK with the bound port for the listeners. My second choice was to delay starting processors, but since the simple reordering worked, I didn't actually try that out. |
|
@omkreddy I tried out delayed processor start and that wasn't as big a change as I expected. So I have updated the PR. I think this will be easier to maintain compared to managing the ordering within KafkaServer. |
junrao
left a comment
junrao
left a comment
There was a problem hiding this comment.
@rajinisivaram : Thanks for the patch. LGTM. Just a minor comment below.
There was a problem hiding this comment.
This is not directly related to this patch. However, it seems that we need some synchronization on processor between the reader and the writer?
There was a problem hiding this comment.
@junrao Thanks for the review. We were already synchronizing on all read/write accesses to processors apart from shutdown(). I think that was safe anyway, but I have added synchronize in shutdown as well to be consistent.
rajinisivaram
force-pushed
the
KAFKA-6772
branch
from
e70ed96 to
48c60b6
Compare
ijuma
left a comment
ijuma
left a comment
There was a problem hiding this comment.
Thanks for the PR, left a couple of minor comments.
|
|
||
| def startProcessors(): Unit = synchronized { | ||
| acceptors.values.asScala.foreach { _.startProcessors() } | ||
| info("Started processors for " + acceptors.size + " acceptors") |
| // that credentials have been loaded before processing authentications. | ||
| socketServer = new SocketServer(config, metrics, time, credentialProvider) | ||
| socketServer.startup() | ||
| socketServer.startup(startupProcessors = false) |
There was a problem hiding this comment.
We should document the new parameter in the scaladoc for startup and the reason why it's needed (i.e. people shouldn't have to read the comments in a particular usage to understand it). We should also include the actual behaviour impact (e.g. what happens to the requests that arrive while the processors are not started).
|
@ijuma Thanks for the review, I have addressed the comments. |
|
Merging to trunk, 1.1 and 1.0. |
) Start processing client connections only after completing KafkaServer initialization to ensure that credentials are loaded from ZK into cache before authentications are processed. Acceptors are started earlier so that bound port is known for registering in ZK. Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, Jun Rao <junrao@gmail.com>, Ismael Juma <ismael@juma.me.uk>
) Start processing client connections only after completing KafkaServer initialization to ensure that credentials are loaded from ZK into cache before authentications are processed. Acceptors are started earlier so that bound port is known for registering in ZK. Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, Jun Rao <junrao@gmail.com>, Ismael Juma <ismael@juma.me.uk>
* confluent/1.1: (60 commits) MINOR: Fix kafka-run-class for Java 10 (apache#4895) KAFKA-6772: Load credentials from ZK before accepting connections (apache#4867) KAFKA-6742: TopologyTestDriver error when dealing with stores from GlobalKTable MINOR: Mention that -1 disables retention by time (apache#4881) KAFKA-6790: Fix Streams processor node broken link (apache#4874) MINOR: Java 10 fixes so that the build passes (apache#4839) MINOR: Update Jackson to 2.9.5 (apache#4776) MINOR: Downgrade to Gradle 4.5.1 (apache#4791) MINOR: Java 9/10 fixes, gradle and minor deps update (apache#4725) KAFKA-6752: Enable unclean leader election metric (apache#4838) KAFKA-6054: Fix upgrade path from Kafka Streams v0.10.0 (apache#4773) KAFKA-6747 Check whether there is in-flight transaction before aborting transaction (apache#4826) KAFKA-6748: double check before scheduling a new task after the punctuate call (apache#4827) KAFKA-6739; Ignore headers when down-converting from V2 to V0/V1 (apache#4813) KAFKA-6728: Corrected the worker’s instantiation of the HeaderConverter KAFKA-6731: waitOnState should check the state to be the target start. (apache#4808) HOTFIX: Enforce a rebalance upon task migration (apache#4802) MINOR: Remove 1.2.0 changes from streams doc (apache#4784) MINOR: Update version numbers to 1.1.1-SNAPSHOT MINOR: Fix ReassignPartitionsClusterTest.testHwAfterPartitionReassignment test (apache#4781) ...
…ache#4867) Start processing client connections only after completing KafkaServer initialization to ensure that credentials are loaded from ZK into cache before authentications are processed. Acceptors are started earlier so that bound port is known for registering in ZK. Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, Jun Rao <junrao@gmail.com>, Ismael Juma <ismael@juma.me.uk>
…ache#4867) Start processing client connections only after completing KafkaServer initialization to ensure that credentials are loaded from ZK into cache before authentications are processed. Acceptors are started earlier so that bound port is known for registering in ZK. Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, Jun Rao <junrao@gmail.com>, Ismael Juma <ismael@juma.me.uk>
…:1.1-nflx to 1.1-nflx * commit '84eeea7fe4b3a64b84b87d231969acfee4fb7544': Fix a bug where the ReqeustPerSec API version hash map is not updated. KAFKA-6772: Load credentials from ZK before accepting connections (apache#4867) KAFKA-6742: TopologyTestDriver error when dealing with stores from GlobalKTable MINOR: Mention that -1 disables retention by time (apache#4881) KAFKA-6790: Fix Streams processor node broken link (apache#4874) MINOR: Java 10 fixes so that the build passes (apache#4839) MINOR: Update Jackson to 2.9.5 (apache#4776) MINOR: Downgrade to Gradle 4.5.1 (apache#4791) MINOR: Java 9/10 fixes, gradle and minor deps update (apache#4725) KAFKA-6752: Enable unclean leader election metric (apache#4838) KAFKA-6054: Fix upgrade path from Kafka Streams v0.10.0 (apache#4773) KAFKA-6747 Check whether there is in-flight transaction before aborting transaction (apache#4826) KAFKA-6748: double check before scheduling a new task after the punctuate call (apache#4827)
4 participants
Start processing client connections only after completing KafkaServer initialization to ensure that credentials are loaded from ZK into cache before authentications are processed. Acceptors are started earlier so that bound port is known for registering in ZK.
Committer Checklist (excluded from commit message)