kevin-wu24 commented on code in PR #18949:
URL: https://github.com/apache/kafka/pull/18949#discussion_r1984227562
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -194,6 +194,7 @@ class BrokerServer(
val clientMetricsReceiverPlugin = new ClientMetricsReceiverPlugin()
config.dynamicConfig.initialize(Some(clientMetricsReceiverPlugin))
+ DynamicBrokerConfig.readDynamicBrokerConfigsFromSnapshot(raftManager,
config, quotaManagers)
Review Comment:
> we need to load the dynamic configurations prior to sending out the
initial dynamic configurations
I'm a bit confused about what this means. Where in sharedServer do we "send
out initial dynamic configurations"?
From a high level, I think the current implementation is doing the
equivalent in KRaft, since the KRaft layer, which reads the snapshot, on the
broker is acting like ZK (although possibly stale) to store broker configs
(correct me if I'm wrong, I also will look more into this tmrw).
The only config I see that is used in `sharedServer/raftManager` that is
also part of `DynamicBrokerConfig#AllDynamicConfigs` is the
`LISTENER_SECURITY_PROTOCOL_MAP_CONFIG`. This config is read when building the
KRaft network client for the broker during startup, but only the entry for the
controller listener is used. From my understanding, the only way to invalidate
this static config if it was previously value would be to mess with
server.properties directly and then restart the broker. Is that also a case we
also want to cover with this 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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
