junrao commented on code in PR #20405:
URL: https://github.com/apache/kafka/pull/20405#discussion_r2298598556
##########
core/src/main/scala/kafka/server/DynamicBrokerConfig.scala:
##########
@@ -231,15 +230,15 @@ object DynamicBrokerConfig {
if (configRecord.resourceName().isEmpty) {
putOrRemoveIfNull(dynamicDefaultConfigs,
configRecord.name(), configRecord.value())
} else if (configRecord.resourceName() ==
config.brokerId.toString) {
- putOrRemoveIfNull(dynamicPerBrokerConfigs,
configRecord.name(), configRecord.value())
+ putOrRemoveIfNull(dynamicDefaultConfigs,
configRecord.name(), configRecord.value())
}
}
}
}
}
val configHandler = new BrokerConfigHandler(config, quotaManagers)
- configHandler.processConfigChanges("", dynamicPerBrokerConfigs)
- configHandler.processConfigChanges(config.brokerId.toString,
dynamicPerBrokerConfigs)
+ configHandler.processConfigChanges("", dynamicDefaultConfigs)
+ configHandler.processConfigChanges(config.brokerId.toString,
dynamicDefaultConfigs)
Review Comment:
It seems that we should continue to use dynamicPerBrokerConfigs here?
##########
core/src/main/scala/kafka/server/DynamicBrokerConfig.scala:
##########
@@ -231,15 +230,15 @@ object DynamicBrokerConfig {
if (configRecord.resourceName().isEmpty) {
putOrRemoveIfNull(dynamicDefaultConfigs,
configRecord.name(), configRecord.value())
} else if (configRecord.resourceName() ==
config.brokerId.toString) {
- putOrRemoveIfNull(dynamicPerBrokerConfigs,
configRecord.name(), configRecord.value())
+ putOrRemoveIfNull(dynamicDefaultConfigs,
configRecord.name(), configRecord.value())
Review Comment:
Hmm, this doesn't look right. If `configRecord.resourceName() ==
config.brokerId.toString`, it's a broker level config and it seems that we
should use dynamicPerBrokerConfigs.
--
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
