divijvaidya commented on code in PR #14176:
URL: https://github.com/apache/kafka/pull/14176#discussion_r1291265129
##########
core/src/main/scala/kafka/server/ControllerServer.scala:
##########
@@ -231,7 +231,7 @@ class ControllerServer(
setMetrics(quorumControllerMetrics).
setCreateTopicPolicy(createTopicPolicy.asJava).
setAlterConfigPolicy(alterConfigPolicy.asJava).
- setConfigurationValidator(new ControllerConfigurationValidator()).
+ setConfigurationValidator(new
ControllerConfigurationValidator(sharedServer.brokerConfig)).
Review Comment:
Thinking more about it...this is more complicated than I expected.
We want to block enablement of a Topic level config if all broker don't have
TS enabled on them. We need a way to determine that the TS has been enabled on
all brokers. In Kraft world, no component has a view of all broker configs, not
even the controller (correct me if I am wrong here) because broker level config
is in their separate server.properties files.
As an example, what happens when we are in a rolling restart, some brokers
have TS enabled on them and some don't. We send an alter config call to enable
TS for a topic, it hits the one which has TS enabled, this broker forwards it
to the controller and controller will send the config update to all brokers.
When another broker which doesn't have TS enabled gets this config change, it
"should" fail to apply it. But failing now is too late since alterConfig has
already succeeded since controller->broker config propagation is done async.
With this limitation in mind, the ideal solution is:
1. add a logic in controller such that it knows broker level config of all
brokers (does it already know that in metadata?)
2. when request to enable TS for a topic arrives, ensure that all brokers
have TS enabled, if not, then reject.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
