splett2 commented on a change in pull request #9555:
URL: https://github.com/apache/kafka/pull/9555#discussion_r518283600
##########
File path: core/src/main/scala/kafka/network/SocketServer.scala
##########
@@ -1189,6 +1189,7 @@ class ConnectionQuotas(config: KafkaConfig, time: Time,
metrics: Metrics) extend
@volatile private var defaultMaxConnectionsPerIp: Int =
config.maxConnectionsPerIp
@volatile private var maxConnectionsPerIpOverrides =
config.maxConnectionsPerIpOverrides.map { case (host, count) =>
(InetAddress.getByName(host), count) }
@volatile private var brokerMaxConnections = config.maxConnections
+ @volatile private var interBrokerListenerName =
config.interBrokerListenerName
Review comment:
`interBrokerListenerName` is apparently not a dynamic config.
`DynamicBrokerReconfigurationTest.testAdvertisedListenerUpdate`
...
```
// Verify updating inter-broker listener
val props = new Properties
props.put(KafkaConfig.InterBrokerListenerNameProp, SecureExternal)
try {
reconfigureServers(props, perBrokerConfig = true,
(KafkaConfig.InterBrokerListenerNameProp, SecureExternal))
fail("Inter-broker listener cannot be dynamically updated")
}
```
It does seem like we allow updating the interbroker listener when
adding/removing listeners, but that case is covered.
I will try to verify we allow updating the interbroker listener name through
the inter broker security protocol, since apparently that's another way to
configure the listener.
##########
File path: core/src/main/scala/kafka/network/SocketServer.scala
##########
@@ -1189,6 +1189,7 @@ class ConnectionQuotas(config: KafkaConfig, time: Time,
metrics: Metrics) extend
@volatile private var defaultMaxConnectionsPerIp: Int =
config.maxConnectionsPerIp
@volatile private var maxConnectionsPerIpOverrides =
config.maxConnectionsPerIpOverrides.map { case (host, count) =>
(InetAddress.getByName(host), count) }
@volatile private var brokerMaxConnections = config.maxConnections
+ @volatile private var interBrokerListenerName =
config.interBrokerListenerName
Review comment:
`interBrokerListenerName` is apparently not a dynamic config.
see `DynamicBrokerReconfigurationTest.testAdvertisedListenerUpdate`
...
```
// Verify updating inter-broker listener
val props = new Properties
props.put(KafkaConfig.InterBrokerListenerNameProp, SecureExternal)
try {
reconfigureServers(props, perBrokerConfig = true,
(KafkaConfig.InterBrokerListenerNameProp, SecureExternal))
fail("Inter-broker listener cannot be dynamically updated")
}
```
It does seem like we allow updating the interbroker listener when
adding/removing listeners, but that case is covered.
I will try to verify we allow updating the interbroker listener name through
the inter broker security protocol, since apparently that's another way to
configure the listener.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
