You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/05/11 03:35:52 UTC

[GitHub] [kafka] showuon opened a new pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

showuon opened a new pull request #10665:
URL: https://github.com/apache/kafka/pull/10665


   We used to set a low `replica.lag.time.max.ms` value (2 sec) to speed up the test, but the 2 sec is not long enough in slow Jenkins env, and caused the follower got kicked out from ISR, so the `UnderReplicatedPartitions` count will be added. Increase the `replica.lag.time.max.ms` value to make this test more reliable.
   
   ### 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.

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

[GitHub] [kafka] showuon commented on pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#issuecomment-858472811


   @mimaison , thanks for your time to review. :)


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

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

[GitHub] [kafka] showuon commented on pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#issuecomment-837734046


   @edoardocomar @mimaison , could you help review this PR to make the test reliable. Thank you.


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

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

[GitHub] [kafka] showuon commented on pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#issuecomment-856392681


   @mimaison , please help have a 2nd review. Thank you.
   
   The failed tests are unrelated and flaky.
   ```
       Build / JDK 15 and Scala 2.13 / kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopics()
       Build / JDK 15 and Scala 2.13 / kafka.server.RaftClusterTest.testCreateClusterAndCreateListDeleteTopic()
       Build / JDK 15 and Scala 2.13 / kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopics()
   ```


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

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

[GitHub] [kafka] mimaison merged pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
mimaison merged pull request #10665:
URL: https://github.com/apache/kafka/pull/10665


   


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

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

[GitHub] [kafka] showuon commented on pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#issuecomment-837956607


   Failed tests are un-related. Thanks.
   ```
       Build / JDK 15 and Scala 2.13 / org.apache.kafka.common.record.MemoryRecordsBuilderTest.[6] magic=2, bufferOffset=0, compressionType=GZIP
       Build / JDK 15 and Scala 2.13 / kafka.api.TransactionsBounceTest.testWithGroupMetadata()
   ```


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

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

[GitHub] [kafka] showuon commented on pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#issuecomment-845948314


   @mimaison , thanks for your comments. I've updated. Please take a look again. Thank you.


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

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

[GitHub] [kafka] showuon commented on pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#issuecomment-842772461


   @edoardocomar @mimaison  , could you help review this PR to make the test reliable. Thank you.


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

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

[GitHub] [kafka] showuon commented on a change in pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#discussion_r636865778



##########
File path: core/src/test/scala/unit/kafka/integration/MetricsDuringTopicCreationDeletionTest.scala
##########
@@ -116,9 +116,9 @@ class MetricsDuringTopicCreationDeletionTest extends KafkaServerTestHarness with
     running = false;
     thread.join
 
-    assert(offlinePartitionsCount==0, "OfflinePartitionCount not 0: "+ offlinePartitionsCount)
-    assert(preferredReplicaImbalanceCount==0, "PreferredReplicaImbalanceCount not 0: " + preferredReplicaImbalanceCount)
-    assert(underReplicatedPartitionCount==0, "UnderReplicatedPartitionCount not 0: " + underReplicatedPartitionCount)
+    assert(offlinePartitionsCount==0, "Expect OfflinePartitionCount to be 0, but got: "+ offlinePartitionsCount)

Review comment:
       Will update it later. Thanks. 




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

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

[GitHub] [kafka] mimaison commented on a change in pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
mimaison commented on a change in pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#discussion_r636816944



##########
File path: core/src/test/scala/unit/kafka/integration/MetricsDuringTopicCreationDeletionTest.scala
##########
@@ -70,12 +70,12 @@ class MetricsDuringTopicCreationDeletionTest extends KafkaServerTestHarness with
   @Test
   def testMetricsDuringTopicCreateDelete(): Unit = {
 
-    // For UnderReplicatedPartitions, because of https://issues.apache.org/jira/browse/KAFKA-4605
+    // For UnderReplicatedPartitions, because of KAFKA-4605

Review comment:
       Why are we removing the links?

##########
File path: core/src/test/scala/unit/kafka/integration/MetricsDuringTopicCreationDeletionTest.scala
##########
@@ -116,9 +116,9 @@ class MetricsDuringTopicCreationDeletionTest extends KafkaServerTestHarness with
     running = false;
     thread.join
 
-    assert(offlinePartitionsCount==0, "OfflinePartitionCount not 0: "+ offlinePartitionsCount)
-    assert(preferredReplicaImbalanceCount==0, "PreferredReplicaImbalanceCount not 0: " + preferredReplicaImbalanceCount)
-    assert(underReplicatedPartitionCount==0, "UnderReplicatedPartitionCount not 0: " + underReplicatedPartitionCount)
+    assert(offlinePartitionsCount==0, "Expect OfflinePartitionCount to be 0, but got: "+ offlinePartitionsCount)

Review comment:
       We can use string interpolation here:
   ```
   assert(offlinePartitionsCount==0, s"Expect OfflinePartitionCount to be 0, but got: $offlinePartitionsCount")
   ```




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

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

[GitHub] [kafka] showuon commented on a change in pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#discussion_r636915183



##########
File path: core/src/test/scala/unit/kafka/integration/MetricsDuringTopicCreationDeletionTest.scala
##########
@@ -70,12 +70,12 @@ class MetricsDuringTopicCreationDeletionTest extends KafkaServerTestHarness with
   @Test
   def testMetricsDuringTopicCreateDelete(): Unit = {
 
-    // For UnderReplicatedPartitions, because of https://issues.apache.org/jira/browse/KAFKA-4605
+    // For UnderReplicatedPartitions, because of KAFKA-4605

Review comment:
       I just reverted my change to put whole URL here. I think either way is OK. Thanks.




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

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

[GitHub] [kafka] mimaison commented on pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
mimaison commented on pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#issuecomment-858466707


   @showuon Sorry for the delay, thanks for the PR


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

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

[GitHub] [kafka] showuon commented on a change in pull request #10665: KAFKA-9009: increase replica.lag.time.max.ms to make the test reliable

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #10665:
URL: https://github.com/apache/kafka/pull/10665#discussion_r636866557



##########
File path: core/src/test/scala/unit/kafka/integration/MetricsDuringTopicCreationDeletionTest.scala
##########
@@ -70,12 +70,12 @@ class MetricsDuringTopicCreationDeletionTest extends KafkaServerTestHarness with
   @Test
   def testMetricsDuringTopicCreateDelete(): Unit = {
 
-    // For UnderReplicatedPartitions, because of https://issues.apache.org/jira/browse/KAFKA-4605
+    // For UnderReplicatedPartitions, because of KAFKA-4605

Review comment:
       I just thought the ticket Id: KAFKA-4605 is enough, no need to put  the whole URL. What do you think?




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

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