soarez commented on code in PR #15744:
URL: https://github.com/apache/kafka/pull/15744#discussion_r1573203822
##########
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:
That makes sense. I'm just concerned this retry could hide instability in
the operation we're trying to test. I'm thinking we could bypass the client API
and retry just on the precondition of the controller being KRaft by e.g.
reading the /controller znode, and then expect `incrementalAlterConfigs` to
succeed at once. WDYT?
--
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
