Re: [PR] KAFKA-15793 Fix ZkMigrationIntegrationTest#testMigrateTopicDeletions [kafka]

["soarez (via GitHub)" <gi...@apache.org>]

soarez commented on code in PR #17004:
URL: https://github.com/apache/kafka/pull/17004#discussion_r1740188303


##########
core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala:
##########
@@ -391,21 +405,25 @@ class ZkMigrationIntegrationTest {
       TestUtils.waitUntilTrue(
         () => topicsAllRecreated(admin),
         "Timed out waiting for topics to be created",
-        30000,
+        70000,
         1000)
+      log.info("Topics to be re-created")

Review Comment:
   Same here, did you mean "Topics have been re-created"?



##########
core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala:
##########
@@ -369,18 +382,19 @@ class ZkMigrationIntegrationTest {
       TestUtils.waitUntilTrue(
         () => topicsAllDeleted(admin),
         "Timed out waiting for topics to be deleted",
-        30000,
+        307000,
         1000)
+      log.info("Topics to be deleted")

Review Comment:
   Should this log say "Topics have been deleted" or something similar?



##########
core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala:
##########
@@ -391,21 +405,25 @@ class ZkMigrationIntegrationTest {
       TestUtils.waitUntilTrue(
         () => topicsAllRecreated(admin),
         "Timed out waiting for topics to be created",
-        30000,
+        70000,
         1000)
+      log.info("Topics to be re-created")
+
+      TestUtils.retry(311000) {
+        log.info("Waiting for consistent topic state")
 
-      TestUtils.retry(300000) {
         // Need a retry here since topic metadata may be inconsistent between brokers
         val topicDescriptions = try {
           admin.describeTopics(util.Arrays.asList(
             "test-topic-1", "test-topic-2", "test-topic-3"
           )).topicNameValues().asScala.map { case (name, description) =>
-            name -> description.get(60, TimeUnit.SECONDS)
+            name -> description.get(72, TimeUnit.SECONDS)
           }.toMap
         } catch {
           case e: ExecutionException if e.getCause.isInstanceOf[UnknownTopicOrPartitionException] => Map.empty[String, TopicDescription]
           case t: Throwable => fail("Error describing topics", t.getCause)
         }
+        log.info("Topic describe: {}", topicDescriptions);

Review Comment:
   As this is inside a retry loop, should this be debug level?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org