cmccabe commented on code in PR #12063:
URL: https://github.com/apache/kafka/pull/12063#discussion_r852482123
##########
core/src/main/scala/kafka/server/DynamicBrokerConfig.scala:
##########
@@ -950,16 +963,17 @@ class DynamicListenerConfig(server: KafkaBroker) extends
BrokerReconfigurable wi
val listenersAdded = newListeners.filterNot(e =>
oldListenerMap.contains(e.listenerName))
// Clear SASL login cache to force re-login
- if (listenersAdded.nonEmpty || listenersRemoved.nonEmpty)
- LoginManager.closeAll()
-
- server.socketServer.removeListeners(listenersRemoved)
- if (listenersAdded.nonEmpty)
- server.socketServer.addListeners(listenersAdded)
-
- server match {
- case kafkaServer: KafkaServer =>
kafkaServer.kafkaController.updateBrokerInfo(kafkaServer.createBrokerInfo)
- case _ =>
+ LoginManager.closeAll()
Review Comment:
So, what is confusing about this code is that the same key can be registered
for multiple `Reconfigurable` objects. In this case, `DynamicListenerConfig`
(created in `DynamicBrokerConfig#addReconfigurables`) is registered for
`max.connections` (and the other properties listed in
`DynamicListenerConfig#ReconfigurableConfigs`) but SocketServer is also
registered for those properties. If you notice, `DynamicListenerConfig` doesn't
do anything with `max.connections`, or even look at it.
So *unless* you are adding or removing a listener, the only effect of
`DynamicListenerConfig` is to call `LoginManager#closeAll`. This is needed for
some SASL things. For example, if you change `sasl.jaas.config` I suspect you
need to do this.
I suppose this raises the question of if we should be doing something to
ensure the SocketServer's reconfiguration happens before or after the call to
`LoginManager#closeAll`. Given that this is an existing code path, I left it
the way it was. Maybe we should file a follow-up JIRA about this and pull in
some people who know more about SASL.
I will also add a comment here about this.
--
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
