Re: [PR] KAFKA-16655: Deflaking ZKMigrationIntegrationTest.testDualWrite [kafka]

[via GitHub]

cmccabe merged PR #15845:
URL: https://github.com/apache/kafka/pull/15845


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

Re: [PR] KAFKA-16655: Deflaking ZKMigrationIntegrationTest.testDualWrite [kafka]

[via GitHub]

ahuang98 commented on PR #15845:
URL: https://github.com/apache/kafka/pull/15845#issuecomment-2093452127

   test failures look unrelated


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

Re: [PR] KAFKA-16655: Deflaking ZKMigrationIntegrationTest.testDualWrite [kafka]

[via GitHub]

ahuang98 commented on PR #15845:
URL: https://github.com/apache/kafka/pull/15845#issuecomment-2091208654

   ^ edited title to kick off another build (last one failed due to unrelated 
issues)


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

Re: [PR] KAFKA-16655: Deflaking ZKMigrationIntegrationTest.testDualWrite [kafka]

[via GitHub]

ahuang98 commented on code in PR #15845:
URL: https://github.com/apache/kafka/pull/15845#discussion_r1588067214


##
core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala:
##
@@ -612,7 +613,12 @@ class ZkMigrationIntegrationTest {
   val readyFuture = 
kraftCluster.controllers().values().asScala.head.controller.waitForReadyBrokers(3)
 
   // Allocate a block of producer IDs while in ZK mode
-  val nextProducerId = 
sendAllocateProducerIds(zkCluster.asInstanceOf[ZkClusterInstance]).get(30, 
TimeUnit.SECONDS)
+  var nextProducerId = -1L
+ 
+  TestUtils.retry(6) {
+assertDoesNotThrow((() => nextProducerId = 
sendAllocateProducerIds(zkCluster.asInstanceOf[ZkClusterInstance]).get(20, 
TimeUnit.SECONDS)): Executable)
+  }
+  assertEquals(0, nextProducerId)

Review Comment:
   this checks if the nextProducerId is allocated correctly before moving onto 
the next steps



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

Re: [PR] KAFKA-16655: Deflaking ZKMigrationIntegrationTest.testDualWrite [kafka]

[via GitHub]

johnnychhsu commented on PR #15845:
URL: https://github.com/apache/kafka/pull/15845#issuecomment-2090696169

   thanks for the PR! 
   got a question and left a comment.


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

Re: [PR] KAFKA-16655: Deflaking ZKMigrationIntegrationTest.testDualWrite [kafka]

[via GitHub]

johnnychhsu commented on code in PR #15845:
URL: https://github.com/apache/kafka/pull/15845#discussion_r1587770348


##
core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala:
##
@@ -612,7 +613,12 @@ class ZkMigrationIntegrationTest {
   val readyFuture = 
kraftCluster.controllers().values().asScala.head.controller.waitForReadyBrokers(3)
 
   // Allocate a block of producer IDs while in ZK mode
-  val nextProducerId = 
sendAllocateProducerIds(zkCluster.asInstanceOf[ZkClusterInstance]).get(30, 
TimeUnit.SECONDS)
+  var nextProducerId = -1L
+ 
+  TestUtils.retry(6) {
+assertDoesNotThrow((() => nextProducerId = 
sendAllocateProducerIds(zkCluster.asInstanceOf[ZkClusterInstance]).get(20, 
TimeUnit.SECONDS)): Executable)
+  }
+  assertEquals(0, nextProducerId)

Review Comment:
   may I know why do we need this `assertEquals(0, nextProducerId)` here? 



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

[PR] KAFKA-16655 Deflaking ZKMigrationIntegrationTest.testDualWrite [kafka]

[via GitHub]

ahuang98 opened a new pull request, #15845:
URL: https://github.com/apache/kafka/pull/15845

   This test occasionally fails due to stale broker epoch exceptions, which in 
turn causes allocate producer ids to fail.
   
   Adds retries as stale broker epoch is a retriable issue, and fixes 
sendAllocateProducerIds returning 0 as the producerIdStart in error cases
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


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