You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "kevin-wu24 (via GitHub)" <gi...@apache.org> on 2026/03/31 16:12:53 UTC
[PR] KAFKA-20380: kraft.version=0 leaders should advertise their endpoint from controller.quorum.voters [kafka]
kevin-wu24 opened a new pull request, #21918:
URL: https://github.com/apache/kafka/pull/21918
### What Changed
- When becoming leader in kraft.version=0, the local node should use its endpoint defined in `controller.quorum.voters` when constructing `LeaderState`. This is because the entries in the static config are expected to be routable in all networking configurations.
- This approach is safe in the case of a successful kraft.version upgrade because the contents of `controller.quorum.voters` are written to the log as part of the upgrade, so the leader's current advertised listener is correct. Any subsequent leader would then correctly use the value of localListeners in kraft.version=1.
### Testing
Added `testKRaftVersion0AdvertisesStaticVotersEndpoint`, which initializes a kraft.version=0 leader, and asserts the endpoint it advertises in the `BeginQuorumEpochRequest` is from the static voter set. This test fails against trunk, but passes with the code change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3183401840
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -360,21 +451,28 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _])
} else {
Seq.empty
}
+ val controllerQuorumVotersAddress = QuorumConfig.parseVoterConnections(quorumConfig.voters).asScala.get(nodeId())
Review Comment:
Can we perform this parsing only when it is needed? For example, let move it to line 469, after the if statement with the 3 predicates.
##########
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala:
##########
@@ -349,17 +349,33 @@ class KafkaConfigTest {
}
@Test
- def testEffectAdvertiseControllerListenerForControllerWithoutAdvertisement(): Unit = {
+ def testEffectAdvertiseControllerListenerForControllerWithoutAdvertisementAndVotersConfig(): Unit = {
val props = new Properties()
props.setProperty(KRaftConfigs.PROCESS_ROLES_CONFIG, "controller")
- props.setProperty(SocketServerConfigs.LISTENERS_CONFIG, "CONTROLLER://localhost:9093")
props.setProperty(KRaftConfigs.NODE_ID_CONFIG, "2")
- props.setProperty(QuorumConfig.QUORUM_VOTERS_CONFIG, "2@localhost:9093")
props.setProperty(KRaftConfigs.CONTROLLER_LISTENER_NAMES_CONFIG, "CONTROLLER")
+ props.setProperty(QuorumConfig.QUORUM_VOTERS_CONFIG, "2@lb1.example.com:9092")
+ props.setProperty(SocketServerConfigs.LISTENERS_CONFIG, "CONTROLLER://:9093")
val config = KafkaConfig.fromProps(props)
assertEquals(
- Seq(new Endpoint("CONTROLLER", SecurityProtocol.PLAINTEXT, "localhost", 9093)),
+ Seq(new Endpoint("CONTROLLER", SecurityProtocol.PLAINTEXT, "lb1.example.com", 9092)),
+ config.effectiveAdvertisedControllerListeners
+ )
+ }
+
+ @Test
+ def testEffectAdvertiseControllerListenerForControllerWithoutAdvertisementOrVotersConfig(): Unit = {
Review Comment:
Thanks for the tests!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined [kafka]
Posted by "kevin-wu24 (via GitHub)" <gi...@apache.org>.
kevin-wu24 commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3150151823
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
Review Comment:
This is a slightly different logic than what I documented in the PR description. This logic says the effective advertised controller listener is:
1. `advertised.listeners` if it is defined
2. Otherwise, `listeners` if the endpoint host is not null or 0.0.0.0
3. Otherwise, `controller.quorum.voters` endpoint for the node if that property is defined and the node is a voter
4. Else, kafka will end up doing a reverse lookup
The current implementation makes `controller.quorum.voters` take priority over `listeners` even if `listeners` is set to a non-null host. We only saw this issue when the `listeners` host was null, and kafka did a reverse lookup that made the `listeners` endpoint unresolvable.
My motivation for prioritizing the contents of `controller.quorum.voters` is that the contents of `listeners` are not guaranteed to be routable. If this is a misunderstanding, please let me know. One could imagine a scenario where the operator puts the contents of the reverse lookup (unresolvable) into the `listeners` config and this issue would still occur, even if `controller.quorum.voters` is correct and contains a routable endpoint for the node. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined [kafka]
Posted by "kevin-wu24 (via GitHub)" <gi...@apache.org>.
kevin-wu24 commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3150315516
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
Review Comment:
> > My motivation for prioritizing the contents of controller.quorum.voters is that the contents of listeners are not guaranteed to be routable.
>
> I think that's a configuration bug. Ideally, the controller's advertise listener resolution should be the same at the broker's, see effectiveAdvertisedBrokerListeners. Unfortunately, we can't make it that simple because of backwards compatibility with configs prior to 3.9. What do you think?
Okay, that seems reasonable to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3164504416
##########
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala:
##########
@@ -368,17 +384,17 @@ class KafkaConfigTest {
def testEffectAdvertiseControllerListenerForControllerWithMixedAdvertisement(): Unit = {
val props = new Properties()
props.setProperty(KRaftConfigs.PROCESS_ROLES_CONFIG, "controller")
- props.setProperty(SocketServerConfigs.LISTENERS_CONFIG, "CONTROLLER://localhost:9093,CONTROLLER_NEW://localhost:9094")
props.setProperty(KRaftConfigs.NODE_ID_CONFIG, "2")
- props.setProperty(QuorumConfig.QUORUM_VOTERS_CONFIG, "2@localhost:9093")
props.setProperty(KRaftConfigs.CONTROLLER_LISTENER_NAMES_CONFIG, "CONTROLLER,CONTROLLER_NEW")
props.setProperty(SocketServerConfigs.ADVERTISED_LISTENERS_CONFIG, "CONTROLLER://lb1.example.com:9000")
+ props.setProperty(QuorumConfig.QUORUM_VOTERS_CONFIG, "2@lb2.example.com:9092")
+ props.setProperty(SocketServerConfigs.LISTENERS_CONFIG, "CONTROLLER://:9093,CONTROLLER_NEW://:9094")
val config = KafkaConfig.fromProps(props)
assertEquals(
Seq(
new Endpoint("CONTROLLER", SecurityProtocol.PLAINTEXT, "lb1.example.com", 9000),
- new Endpoint("CONTROLLER_NEW", SecurityProtocol.PLAINTEXT, "localhost", 9094)
+ new Endpoint("CONTROLLER_NEW", SecurityProtocol.PLAINTEXT, "lb2.example.com", 9092)
Review Comment:
This is annoying. This should be a configuration bug. We should only look at the `controller.quorum.voters` if the advertise listener is not set at all and only for the default controller listener. The first listener name in `CONTROLLER_LISTENER_NAMES_CONFIG`. We assume that the endpoint specified in controller quorum voters are for the default listener since the network client is configured to use the default listener.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: kraft.version=0 leaders should advertise their endpoint from controller.quorum.voters [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3017458303
##########
raft/src/main/java/org/apache/kafka/raft/QuorumState.java:
##########
@@ -707,9 +707,24 @@ public <T> LeaderState<T> transitionToLeader(long epochStartOffset, BatchAccumul
// could address this problem by decoupling the local high watermark, but
// we typically expect the state machine to be caught up anyway.
+ // KAFKA-20380: If the last KRaft version supports reconfiguration, then the
+ // leader's endpoints should be determined by the local listeners. If the last
+ // KRaft version does not support reconfiguration, then the leader's endpoints
+ // should be determined from the static voter set since the local listeners may
+ // not be a routable address for other voters in the cluster.
+ Endpoints leaderEndpoints = partitionState.lastKraftVersion().isReconfigSupported()
+ ? localListeners
Review Comment:
Did you consider fixing this by make sure that the localListener passed to KafkaRaftClient is always correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380; backwards compatible advertised.listeners when it is not defined [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio merged PR #21918:
URL: https://github.com/apache/kafka/pull/21918
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined [kafka]
Posted by "kevin-wu24 (via GitHub)" <gi...@apache.org>.
kevin-wu24 commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3150151823
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
Review Comment:
This is a slightly different logic than what I documented in the PR description. This logic says the effective advertised controller listener is:
1. `advertised.listeners` if it is defined
2. Otherwise, `listeners` if the endpoint host is not null or 0.0.0.0
3. Otherwise, `controller.quorum.voters` endpoint for the node if that property is defined and the node is a voter
4. Else, kafka will end up doing a reverse lookup
The current implementation makes `controller.quorum.voters` take priority over `listeners` even if `listeners` is set to a non-null host. We only saw this issue when the `listeners` host was null, and kafka did a reverse lookup that made the `listeners` endpoint unresolvable.
My motivation for prioritizing the contents of `controller.quorum.voters` is that the contents of `listeners` are not guaranteed to be routable. The contents of `controller.quorum.voters` are expected to be routable or this is a misconfiguration. If this is a misunderstanding, please let me know. One could imagine a scenario where the operator puts the contents of the reverse lookup (unresolvable) into the `listeners` config and this issue would still occur, even if `controller.quorum.voters` is correct and contains a routable endpoint for the node. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined [kafka]
Posted by "kevin-wu24 (via GitHub)" <gi...@apache.org>.
kevin-wu24 commented on PR #21918:
URL: https://github.com/apache/kafka/pull/21918#issuecomment-4171044138
Thanks for the feedback @jsancio. I have pushed some changes to make sure the `localListeners` passed to the `KafkaRaftClient` is correct, PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: kraft.version=0 leaders should advertise their endpoint from controller.quorum.voters [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3017458303
##########
raft/src/main/java/org/apache/kafka/raft/QuorumState.java:
##########
@@ -707,9 +707,24 @@ public <T> LeaderState<T> transitionToLeader(long epochStartOffset, BatchAccumul
// could address this problem by decoupling the local high watermark, but
// we typically expect the state machine to be caught up anyway.
+ // KAFKA-20380: If the last KRaft version supports reconfiguration, then the
+ // leader's endpoints should be determined by the local listeners. If the last
+ // KRaft version does not support reconfiguration, then the leader's endpoints
+ // should be determined from the static voter set since the local listeners may
+ // not be a routable address for other voters in the cluster.
+ Endpoints leaderEndpoints = partitionState.lastKraftVersion().isReconfigSupported()
+ ? localListeners
Review Comment:
Did you consider fixing this by making sure that the localListener passed to KafkaRaftClient is always correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: kraft.version=0 leaders should advertise their endpoint from controller.quorum.voters [kafka]
Posted by "kevin-wu24 (via GitHub)" <gi...@apache.org>.
kevin-wu24 commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3017853904
##########
raft/src/main/java/org/apache/kafka/raft/QuorumState.java:
##########
@@ -707,9 +707,24 @@ public <T> LeaderState<T> transitionToLeader(long epochStartOffset, BatchAccumul
// could address this problem by decoupling the local high watermark, but
// we typically expect the state machine to be caught up anyway.
+ // KAFKA-20380: If the last KRaft version supports reconfiguration, then the
+ // leader's endpoints should be determined by the local listeners. If the last
+ // KRaft version does not support reconfiguration, then the leader's endpoints
+ // should be determined from the static voter set since the local listeners may
+ // not be a routable address for other voters in the cluster.
+ Endpoints leaderEndpoints = partitionState.lastKraftVersion().isReconfigSupported()
+ ? localListeners
Review Comment:
I did consider that approach, but my main question was:
How does the caller of the `KafkaRaftClient` constructor, `KafkaRaftManager`, know what its current kraft version is without reading the entire log prior to constructing the client?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: kraft.version=0 leaders should advertise their endpoint from controller.quorum.voters [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3017978693
##########
raft/src/main/java/org/apache/kafka/raft/QuorumState.java:
##########
@@ -707,9 +707,24 @@ public <T> LeaderState<T> transitionToLeader(long epochStartOffset, BatchAccumul
// could address this problem by decoupling the local high watermark, but
// we typically expect the state machine to be caught up anyway.
+ // KAFKA-20380: If the last KRaft version supports reconfiguration, then the
+ // leader's endpoints should be determined by the local listeners. If the last
+ // KRaft version does not support reconfiguration, then the leader's endpoints
+ // should be determined from the static voter set since the local listeners may
+ // not be a routable address for other voters in the cluster.
+ Endpoints leaderEndpoints = partitionState.lastKraftVersion().isReconfigSupported()
+ ? localListeners
Review Comment:
Local endpoint is computed using the configuration. The resolution logic could be in this order:
1. Use the advertise listener if it exists
2. Use the controller.quorum.voters if it exists
3. Do a reverse lookup of the listener.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3150244221
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
Review Comment:
> My motivation for prioritizing the contents of controller.quorum.voters is that the contents of listeners are not guaranteed to be routable.
I think that's a configuration bug. Ideally, the controller's advertise listener resolution should be the same at the broker's, see effectiveAdvertisedBrokerListeners. Unfortunately, we can't make it that simple because of backwards compatibility with configs prior to 3.9. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: kraft.version=0 leaders should advertise their endpoint from controller.quorum.voters [kafka]
Posted by "kevin-wu24 (via GitHub)" <gi...@apache.org>.
kevin-wu24 commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3017853904
##########
raft/src/main/java/org/apache/kafka/raft/QuorumState.java:
##########
@@ -707,9 +707,24 @@ public <T> LeaderState<T> transitionToLeader(long epochStartOffset, BatchAccumul
// could address this problem by decoupling the local high watermark, but
// we typically expect the state machine to be caught up anyway.
+ // KAFKA-20380: If the last KRaft version supports reconfiguration, then the
+ // leader's endpoints should be determined by the local listeners. If the last
+ // KRaft version does not support reconfiguration, then the leader's endpoints
+ // should be determined from the static voter set since the local listeners may
+ // not be a routable address for other voters in the cluster.
+ Endpoints leaderEndpoints = partitionState.lastKraftVersion().isReconfigSupported()
+ ? localListeners
Review Comment:
I did consider that approach, but my main question was:
How does the caller of the `KafkaRaftClient` constructor, `KafkaRaftManager`, know what its current kraft version is without reading the entire log prior to constructing the client? The `KRaftControlRecordStateMachine` already does this read within KRaft.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: kraft.version=0 leaders should advertise their endpoint from controller.quorum.voters [kafka]
Posted by "kevin-wu24 (via GitHub)" <gi...@apache.org>.
kevin-wu24 commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3018081348
##########
raft/src/main/java/org/apache/kafka/raft/QuorumState.java:
##########
@@ -707,9 +707,24 @@ public <T> LeaderState<T> transitionToLeader(long epochStartOffset, BatchAccumul
// could address this problem by decoupling the local high watermark, but
// we typically expect the state machine to be caught up anyway.
+ // KAFKA-20380: If the last KRaft version supports reconfiguration, then the
+ // leader's endpoints should be determined by the local listeners. If the last
+ // KRaft version does not support reconfiguration, then the leader's endpoints
+ // should be determined from the static voter set since the local listeners may
+ // not be a routable address for other voters in the cluster.
+ Endpoints leaderEndpoints = partitionState.lastKraftVersion().isReconfigSupported()
+ ? localListeners
Review Comment:
I see. This issue is technically independent of kraft.version. Let me update the JIRA to reflect that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined [kafka]
Posted by "kevin-wu24 (via GitHub)" <gi...@apache.org>.
kevin-wu24 commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3183505991
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -360,21 +451,28 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _])
} else {
Seq.empty
}
+ val controllerQuorumVotersAddress = QuorumConfig.parseVoterConnections(quorumConfig.voters).asScala.get(nodeId())
Review Comment:
Yeah will do, thanks for catching that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio commented on code in PR #21918:
URL: https://github.com/apache/kafka/pull/21918#discussion_r3149819763
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
Review Comment:
```scala
.map { endpoint =>
if (endpoint.host == null || endpoint.host == "0.0.0.0") {
controllerAddressInControllerQuorum.map { socketAddress =>
/* The user didn't provide an advertise listener;
* look into the voters property make the config backwards compatible
*/
new Endpoint(
endpoint.listener,
endpoint.securityProtocol,
socketAddress.getHostString,
socketAddress.getPort
)
} getOrElse {
/* Removing "0.0.0.0" host to avoid validation errors. This is to be compatible with the old
* behavior before 3.9. The null or "" host does a reverse lookup in
* ListenerInfo#withWildcardHostnamesResolved.
*/
new Endpoint(endpoint.listener, endpoint.securityProtocol, null, endpoint.port)
}
} else {
endpoint
}
}
```
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -360,21 +451,28 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _])
} else {
Seq.empty
}
+ val controllerQuorumVotersAddress = QuorumConfig.parseVoterConnections(quorumConfig.voters).asScala.get(nodeId())
Review Comment:
This does a DNS lookup because this code uses `new InetSocketAddress`. Only the network layer should do DNS lookups. Let's make sure that the code calls `InetSocketAddress.createUnresolved`
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -360,21 +451,28 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _])
} else {
Seq.empty
}
+ val controllerQuorumVotersAddress = QuorumConfig.parseVoterConnections(quorumConfig.voters).asScala.get(nodeId())
val controllerListenersValue = controllerListeners
controllerListenerNames.asScala.flatMap { name =>
controllerAdvertisedListeners
.find(endpoint => ListenerName.normalised(endpoint.listener).equals(ListenerName.normalised(name)))
.orElse(
- // If users don't define advertised.listeners, the advertised controller listeners inherit from listeners configuration
- // which match listener names in controller.listener.names.
+ // If users don't define advertised.listeners, the advertised controller listeners will first try to use
+ // the endpoint defined by the controller.quorum.voters config and inherit from listeners configuration which
+ // match listeners names in controller.listener.names. If that config is not present, the advertised
+ // controller listeners will inherit from listeners configuration which match listener names in controller.listener.names.
+
// Removing "0.0.0.0" host to avoid validation errors. This is to be compatible with the old behavior before 3.9.
// The null or "" host does a reverse lookup in ListenerInfo#withWildcardHostnamesResolved.
controllerListenersValue
.find(endpoint => ListenerName.normalised(endpoint.listener).equals(ListenerName.normalised(name)))
- .map(endpoint => if (endpoint.host == "0.0.0.0") {
+ .map(endpoint => if (controllerQuorumVotersAddress.isDefined) {
+ new Endpoint(endpoint.listener, endpoint.securityProtocol, controllerQuorumVotersAddress.get.getHostName, controllerQuorumVotersAddress.get.getPort)
Review Comment:
`getHostName` does a reverse lookup if necessary. Configs, and ideally Kafka, should never do that. Use `getHostString` instead.
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -360,21 +451,28 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _])
} else {
Seq.empty
}
+ val controllerQuorumVotersAddress = QuorumConfig.parseVoterConnections(quorumConfig.voters).asScala.get(nodeId())
val controllerListenersValue = controllerListeners
controllerListenerNames.asScala.flatMap { name =>
controllerAdvertisedListeners
.find(endpoint => ListenerName.normalised(endpoint.listener).equals(ListenerName.normalised(name)))
.orElse(
- // If users don't define advertised.listeners, the advertised controller listeners inherit from listeners configuration
- // which match listener names in controller.listener.names.
+ // If users don't define advertised.listeners, the advertised controller listeners will first try to use
+ // the endpoint defined by the controller.quorum.voters config and inherit from listeners configuration which
+ // match listeners names in controller.listener.names. If that config is not present, the advertised
+ // controller listeners will inherit from listeners configuration which match listener names in controller.listener.names.
+
// Removing "0.0.0.0" host to avoid validation errors. This is to be compatible with the old behavior before 3.9.
// The null or "" host does a reverse lookup in ListenerInfo#withWildcardHostnamesResolved.
Review Comment:
Let's move this complicated documentation to the related control flow. See my other comment for some suggestions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org