mumrah commented on code in PR #15744:
URL: https://github.com/apache/kafka/pull/15744#discussion_r1572776248


##########
core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala:
##########
@@ -950,16 +980,47 @@ class ZkMigrationIntegrationTest {
     dataOpt.map(ProducerIdBlockZNode.parseProducerIdBlockData).get
   }
 
-  def alterTopicConfig(admin: Admin): AlterConfigsResult = {
+  def alterBrokerConfigs(admin: Admin): Unit = {
+    val defaultBrokerResource = new ConfigResource(ConfigResource.Type.BROKER, 
"")
+    val defaultBrokerConfigs = Seq(
+      new AlterConfigOp(new 
ConfigEntry(KafkaConfig.LogRetentionTimeMillisProp, "86400000"), 
AlterConfigOp.OpType.SET),
+    ).asJavaCollection
+    val broker0Resource = new ConfigResource(ConfigResource.Type.BROKER, "0")
+    val broker1Resource = new ConfigResource(ConfigResource.Type.BROKER, "1")
+    val specificBrokerConfigs = Seq(
+      new AlterConfigOp(new 
ConfigEntry(KafkaConfig.LogRetentionTimeMillisProp, "43200000"), 
AlterConfigOp.OpType.SET),
+    ).asJavaCollection
+
+    TestUtils.retry(60000) {
+      val result = admin.incrementalAlterConfigs(Map(
+        defaultBrokerResource -> defaultBrokerConfigs,
+        broker0Resource -> specificBrokerConfigs,
+        broker1Resource -> specificBrokerConfigs
+      ).asJava)
+      try {
+        result.all().get(10, TimeUnit.SECONDS)
+      } catch {
+        case t: Throwable => fail("Alter Broker Configs had an error", t)
+      }
+    }

Review Comment:
   > Could we verify that a KRaft controller is ready before testing this 
operation to make the outcome more predictable?
   
   I tried this at first, but we don't actually expose this to the client 
(whether the controller is ZK or KRaft). In fact, we lie to the client and tell 
it that a random broker is the controller. I could have "pierced the veil" to 
access the underlying controller classes, but a retry seemed easier. 



-- 
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

Reply via email to