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.
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")
}
```
I don't think we allow updating inter broker listener at all, so I think we
can remove the test I added. I actually wasn't sure if we allowed it or not,
but the code seems to suggest otherwise.
##########
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")
}
```
I don't think we allow updating inter broker listener at all, so I think we
can remove the test I added. I actually wasn't sure if we allowed it or not,
but the code seems to suggest that it isn't.
----------------------------------------------------------------
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
