KAFKA-18405: Remove ZooKeeper logic from DynamicBrokerConfig#18508
KAFKA-18405: Remove ZooKeeper logic from DynamicBrokerConfig#18508chia7712 merged 3 commits intoapache:trunkfrom
Conversation
|
@FrankYang0529 Do you plan to handle #18384 (comment) in this PR? or this PR is used to remove @mimaison WDYT? |
|
Hi @chia7712, I will handle it in this PR because this Jira is created by that thread. Thanks. |
FrankYang0529
force-pushed
the
KAFKA-18405
branch
from
cb12723 to
ab32a19
Compare
|
Hi @chia7712, I have updated PR to handle "advertised.listeners" and failed cases in CI are flaky:
|
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
@FrankYang0529 thanks for this patch!
| throw new ConfigException(s"Security protocol cannot be updated for existing listener $listenerName") | ||
| } | ||
| if (!newAdvertisedListeners.contains(newConfig.interBrokerListenerName)) | ||
| if (!oldAdvertisedListeners.contains(newConfig.interBrokerListenerName)) |
There was a problem hiding this comment.
Is interBrokerListenerName a kind of dynamic config? If not, KafkaConfig has checked it in initialization.
| val oldConfig = server.config | ||
| val newListeners = listenersToMap(newConfig.listeners) | ||
| val newAdvertisedListeners = listenersToMap(newConfig.effectiveAdvertisedBrokerListeners) | ||
| val oldAdvertisedListeners = listenersToMap(oldConfig.effectiveAdvertisedBrokerListeners) |
There was a problem hiding this comment.
it seems we can use Set instead of Map, right?
FrankYang0529
force-pushed
the
KAFKA-18405
branch
from
ab32a19 to
121c75b
Compare
FrankYang0529
force-pushed
the
KAFKA-18405
branch
from
121c75b to
bb66345
Compare
Signed-off-by: PoAn Yang <payang@apache.org>
Signed-off-by: PoAn Yang <payang@apache.org>
Signed-off-by: PoAn Yang <payang@apache.org>
FrankYang0529
force-pushed
the
KAFKA-18405
branch
from
bb66345 to
eb24f7e
Compare
|
Hi @chia7712, I address all comments. Could you take a look when you have time? Thank you. |
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
LGTM, but I have one follow-up comment.
| */ | ||
| val ReconfigurableConfigs = Set( | ||
| // Listener configs | ||
| SocketServerConfigs.ADVERTISED_LISTENERS_CONFIG, |
There was a problem hiding this comment.
@FrankYang0529 Could you please file a minor follow-up to add this change to zk2kraft.html?
There was a problem hiding this comment.
Yes, I will create a minor PR for it. Thanks.
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
…8508) Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
…8508) Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
…8508) Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
2 participants
zkClientOptin DynamicBrokerConfig.advertised.listenersfromReconfigurableConfigsand related logic.Committer Checklist (excluded from commit message)