divijvaidya commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r977506540
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2523,19 +2522,29 @@ class PlaintextAdminIntegrationTest extends
BaseAdminIntegrationTest {
def testCreateTopicsReturnsConfigs(quorum: String): Unit = {
client = Admin.create(super.createConfig)
- val alterMap = new util.HashMap[ConfigResource,
util.Collection[AlterConfigOp]]
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER, ""),
util.Arrays.asList(
- new AlterConfigOp(new
ConfigEntry(KafkaConfig.LogRetentionTimeMillisProp, "10800000"), OpType.SET)))
- (brokers.map(_.config) ++ controllerServers.map(_.config)).foreach { case
config =>
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER,
config.nodeId.toString()),
- util.Arrays.asList(new AlterConfigOp(new ConfigEntry(
- KafkaConfig.LogCleanerDeleteRetentionMsProp, "34"), OpType.SET)))
+ val newLogRetentionProperties = new Properties
+ newLogRetentionProperties.put(KafkaConfig.LogRetentionTimeMillisProp,
"10800000")
+ TestUtils.incrementalAlterConfigs(null, client, newLogRetentionProperties,
perBrokerConfig = false)
+ .all().get(15, TimeUnit.SECONDS)
+
+ val newLogCleanerDeleteRetention = new Properties
+
newLogCleanerDeleteRetention.put(KafkaConfig.LogCleanerDeleteRetentionMsProp,
"34")
+ TestUtils.incrementalAlterConfigs(brokers, client,
newLogCleanerDeleteRetention, perBrokerConfig = true)
+ .all().get(15, TimeUnit.SECONDS)
+
+ if (isKRaftTest()) {
+ ensureConsistentKRaftMetadata()
+ } else {
+ waitUntilTrue(() => brokers.forall(_.config.originals.getOrDefault(
Review Comment:
Please correct me if I am wrong here.
My understanding is that `ensureConsistentKRaftMetadata` should ensure that
the brokers have caught with the metadata available in kraft controllers. For
alter config in this scenario, we are sure that one of the controller server
contains the new configuration because we wait for the AlterConfig future to
complete. Controller should have stored the new configuration in its metadata
log before completing the future and `ensureConsistentKRaftMetadata` will
ensure that all brokers are consistent with the controller.
Also note that other tests such as `testAppendConfig`,
`testInvalidIncrementalAlterConfigs` and many more rely on
`ensureConsistentKRaftMetadata` to ensure propagating before asserting in test.
--
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
