Skip to content

Commit 8de88db

Browse files
KAFKA-19642 Replace dynamicPerBrokerConfigs with dynamicDefaultConfigs (#20405)
- **Changes**: Replace misused dynamicPerBrokerConfigs with dynamicDefaultConfigs - **Reasons**: KRaft servers don't handle the cluser-level configs in starting from: https://github.com/apache/kafka/pull/18949/files#r2296809389 Reviewers: Jun Rao <[email protected]>, Jhen-Yung Hsu <[email protected]>, PoAn Yang <[email protected]>, Chia-Ping Tsai <[email protected]> --------- Co-authored-by: PoAn Yang <[email protected]>
1 parent 500bd70 commit 8de88db

File tree

2 files changed

+80
-1
lines changed

2 files changed

+80
-1
lines changed

core/src/main/scala/kafka/server/DynamicBrokerConfig.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ object DynamicBrokerConfig {
238238
}
239239
}
240240
val configHandler = new BrokerConfigHandler(config, quotaManagers)
241-
configHandler.processConfigChanges("", dynamicPerBrokerConfigs)
241+
configHandler.processConfigChanges("", dynamicDefaultConfigs)
242242
configHandler.processConfigChanges(config.brokerId.toString, dynamicPerBrokerConfigs)
243243
}
244244
}

core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,16 +1099,29 @@ class DynamicBrokerReconfigurationTest extends QuorumTestHarness with SaslSetup
10991099
@ParameterizedTest(name = TestInfoUtils.TestWithParameterizedGroupProtocolNames)
11001100
@MethodSource(Array("getTestGroupProtocolParametersAll"))
11011101
def testServersCanStartWithInvalidStaticConfigsAndValidDynamicConfigs(groupProtocol: String): Unit = {
1102+
TestNumReplicaFetcherMetricsReporter.testReporters.clear()
1103+
11021104
// modify snapshot interval config to explicitly take snapshot on a broker with valid dynamic configs
11031105
val props = defaultStaticConfig(numServers)
11041106
props.put(MetadataLogConfig.METADATA_SNAPSHOT_MAX_INTERVAL_MS_CONFIG, "10000")
1107+
props.put(MetricConfigs.METRIC_REPORTER_CLASSES_CONFIG, classOf[TestNumReplicaFetcherMetricsReporter].getName)
1108+
props.put(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG, "1")
11051109

11061110
val kafkaConfig = KafkaConfig.fromProps(props)
11071111
val newBroker = createBroker(kafkaConfig).asInstanceOf[BrokerServer]
11081112
servers += newBroker
11091113

11101114
alterSslKeystoreUsingConfigCommand(sslProperties1, listenerPrefix(SecureExternal))
11111115

1116+
// Add num.replica.fetchers to the cluster-level config.
1117+
val clusterLevelProps = new Properties
1118+
clusterLevelProps.put(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG, "2")
1119+
reconfigureServers(clusterLevelProps, perBrokerConfig = false, (ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG, "2"))
1120+
1121+
// Wait for the metrics reporter to be configured
1122+
val initialReporter = TestNumReplicaFetcherMetricsReporter.waitForReporters(1).head
1123+
initialReporter.verifyState(reconfigureCount = 1, numFetcher = 2)
1124+
11121125
TestUtils.ensureConsistentKRaftMetadata(servers, controllerServer)
11131126

11141127
TestUtils.waitUntilTrue(
@@ -1121,11 +1134,19 @@ class DynamicBrokerReconfigurationTest extends QuorumTestHarness with SaslSetup
11211134
newBroker.shutdown()
11221135
newBroker.awaitShutdown()
11231136

1137+
// Clean up the test reporter
1138+
TestNumReplicaFetcherMetricsReporter.testReporters.clear()
1139+
11241140
val invalidStaticConfigs = defaultStaticConfig(newBroker.config.brokerId)
11251141
invalidStaticConfigs.putAll(securityProps(invalidSslConfigs, KEYSTORE_PROPS, listenerPrefix(SecureExternal)))
11261142
newBroker.config.updateCurrentConfig(KafkaConfig.fromProps(invalidStaticConfigs))
11271143

11281144
newBroker.startup()
1145+
1146+
// Verify that the custom MetricsReporter is not reconfigured after restart.
1147+
// If readDynamicBrokerConfigsFromSnapshot works correctly, the reporter should maintain its state.
1148+
val reporterAfterRestart = TestNumReplicaFetcherMetricsReporter.waitForReporters(1).head
1149+
reporterAfterRestart.verifyState(reconfigureCount = 0, numFetcher = 2)
11291150
}
11301151

11311152
private def awaitInitialPositions(consumer: Consumer[_, _]): Unit = {
@@ -1634,6 +1655,64 @@ class TestMetricsReporter extends MetricsReporter with Reconfigurable with Close
16341655
}
16351656
}
16361657

1658+
object TestNumReplicaFetcherMetricsReporter {
1659+
val testReporters = new ConcurrentLinkedQueue[TestNumReplicaFetcherMetricsReporter]()
1660+
1661+
def waitForReporters(count: Int): List[TestNumReplicaFetcherMetricsReporter] = {
1662+
TestUtils.waitUntilTrue(() => testReporters.size == count, msg = "Metrics reporters size not matched. Expected: " + count + ", actual: " + testReporters.size())
1663+
1664+
val reporters = testReporters.asScala.toList
1665+
TestUtils.waitUntilTrue(() => reporters.forall(_.configureCount == 1), msg = "Metrics reporters not configured")
1666+
reporters
1667+
}
1668+
}
1669+
1670+
1671+
class TestNumReplicaFetcherMetricsReporter extends MetricsReporter {
1672+
import TestNumReplicaFetcherMetricsReporter._
1673+
@volatile var configureCount = 0
1674+
@volatile var reconfigureCount = 0
1675+
@volatile var numFetchers: Int = 1
1676+
testReporters.add(this)
1677+
1678+
override def init(metrics: util.List[KafkaMetric]): Unit = {
1679+
}
1680+
1681+
override def configure(configs: util.Map[String, _]): Unit = {
1682+
configureCount += 1
1683+
numFetchers = configs.get(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG).toString.toInt
1684+
}
1685+
1686+
override def metricChange(metric: KafkaMetric): Unit = {
1687+
}
1688+
1689+
override def metricRemoval(metric: KafkaMetric): Unit = {
1690+
}
1691+
1692+
override def reconfigurableConfigs(): util.Set[String] = {
1693+
util.Set.of(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG)
1694+
}
1695+
1696+
override def validateReconfiguration(configs: util.Map[String, _]): Unit = {
1697+
val numFetchers = configs.get(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG).toString.toInt
1698+
if (numFetchers <= 0)
1699+
throw new ConfigException(s"Invalid num.replica.fetchers $numFetchers")
1700+
}
1701+
1702+
override def reconfigure(configs: util.Map[String, _]): Unit = {
1703+
reconfigureCount += 1
1704+
numFetchers = configs.get(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG).toString.toInt
1705+
}
1706+
1707+
override def close(): Unit = {
1708+
}
1709+
1710+
def verifyState(reconfigureCount: Int, numFetcher: Int = 1): Unit = {
1711+
assertEquals(reconfigureCount, this.reconfigureCount)
1712+
assertEquals(numFetcher, this.numFetchers)
1713+
}
1714+
}
1715+
16371716

16381717
class MockFileConfigProvider extends FileConfigProvider {
16391718
@throws(classOf[IOException])

0 commit comments

Comments
 (0)