KAFKA-8164: Improve test passing rate by rerunning flaky tests by viktorsomogyi · Pull Request #8019 · apache/kafka

viktorsomogyi · 2020-01-29T13:53:33Z

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

viktorsomogyi · 2020-01-29T14:00:07Z

@ijuma I've created the new PR about flaky tests. Currently it lets 3 retries for a test and fails the build after 50 test failure. Both numbers are a bit arbitrary (especially the 50 :) ), but if you know a good constant for them, I'd be happy to change them.

Furthermore I ran into a few cases where the WhateverClass.classMethod failed which usually happens if there were unexpected running threads after the test. These obviously can't be retried and the plugin will fail as well (and subsequently fails the build too):

* What went wrong:
Execution failed for task ':core:test'.
> org.gradle.test-retry was unable to retry the following test methods, which is unexpected. Please file a bug report at https://github.com/gradle/test-retry-gradle-plugin/issues
     kafka.api.MetricsTest#classMethod
     kafka.api.PlaintextAdminIntegrationTest#classMethod
     kafka.api.SslAdminIntegrationTest#classMethod
     kafka.api.ConsumerTopicCreationTest#classMethod
     kafka.api.SaslSslAdminIntegrationTest#classMethod

Not sure if this is a big problem but I wanted to let you know. The old custom flaky retry didn't handle these either.

ijuma · 2020-01-29T16:34:04Z

+
+    retry {
+      maxRetries = 3
+      maxFailures = 50


I would go with something conservative: 1 retry and max 5 failures.We want to fix flaky tests, but if something just fails once, it's not worth failing the build given Jenkins flakiness.

We can adjust after we see how it works in practice, but this would be a good starting point IMO.

Ok. I figured we can also make these configurable. Updated the PR.

ijuma · 2020-01-31T17:14:03Z

  userMaxForks = project.hasProperty('maxParallelForks') ? maxParallelForks.toInteger() : null

+  userFlakyRetries = project.hasProperty('flakyRetries') ? flakyRetries.toInteger() : 1
+  userMaxTestFailures = project.hasProperty('maxTestFailures') ? maxTestFailures.toInteger() : 5


How about maxTestRetries and maxTestRetryFailures? Also, we need to update the Readme to mention these.

viktorsomogyi · 2020-02-05T09:03:42Z

Updated the common configs section

ijuma · 2020-02-05T12:59:16Z

ok to test

chia7712 · 2020-02-05T15:23:16Z

  spotbugs: "3.1.12",
  spotbugsPlugin: "3.0.0",
  spotlessPlugin: "3.27.1",
+  testRetryPlugin: "1.0.2",


How about using latest version (1.1.0)?

I also noticed it was released shortly after this PR was submitted. Updated.

ijuma · 2020-02-05T15:30:38Z

ok to test

ijuma

LGTM

ijuma · 2020-02-05T15:44:58Z

@viktorsomogyi I made some minor changes, please check that you're happy with them.

ijuma · 2020-02-05T15:45:08Z

ok to test

ijuma · 2020-02-05T15:45:49Z

retest this please

ijuma · 2020-02-05T17:42:43Z

Thinking about this some more and after some discussion, I think we should probably default to 0 retries and tweak the PR builder to pass 1 and 5. Will update the PR if @viktorsomogyi doesn't beat me to it.

viktorsomogyi · 2020-02-05T19:11:07Z

@ijuma I was behind you 10 minutes or so, waited for the local build to run and I saw it only after that you already pushed. :)
I'm happy with the changes.

ijuma · 2020-02-05T20:15:01Z

retest this please

ijuma · 2020-02-05T20:17:24Z

Updated Jenkins PR jobs to be:

./jenkins.sh -PmaxParallelForks=2 -PscalaVersion=2.12 -PmaxTestRetries=1 -PmaxTestRetryFailures=5

Branch builds have not been changed.

ijuma · 2020-02-06T00:39:29Z

One flaky failure with retries (2.12 job):

kafka.admin.ResetConsumerGroupOffsetTest.testResetOffsetsExportImportPlan

Three flaky failures without retries (2.13 job):

kafka.admin.DescribeConsumerGroupTest.testDescribeGroupWithShortInitializationTimeout
org.apache.kafka.streams.kstream.internals.KStreamImplTest.shouldSupportTriggerMaterializedWithKTableFromKStream
org.apache.kafka.streams.processor.internals.StoreChangelogReaderTest.shouldRequestCommittedOffsetsAndHandleTimeoutException[1]

I'll go ahead and merge this so that we can evaluate how well it works in practice.

ijuma · 2020-02-06T00:41:46Z

Thanks for the contribution!

viktorsomogyi · 2020-02-06T11:07:27Z

@ijuma thank you for reviewing this change!

Conflicts: * build.gradle: moved avro plugin definition below newly added test retry plugin. * apache-github/trunk: MINOR: further InternalTopologyBuilder cleanup (apache#8046) MINOR: Add timer for update limit offsets (apache#8047) HOTFIX: Fix spotsbug failure in Kafka examples (apache#8051) KAFKA-9447: Add new customized EOS model example (apache#8031) KAFKA-8164: Add support for retrying failed (apache#8019) HOTFIX: checkstyle for newly added unit test KAFKA-9261; Client should handle unavailable leader metadata (apache#7770) MINOR: Fix typos introduced in KIP-559 (apache#8042) MINOR: Fixing null handilg in ValueAndTimestampSerializer (apache#7679) KAFKA-9113: Clean up task management and state management (apache#7997) MINOR: fix checkstyle issue in ConsumerConfig.java (apache#8038) KAFKA-9491; Increment high watermark after full log truncation (apache#8037) KAFKA-9477 Document RoundRobinAssignor as an option for partition.assignment.strategy (apache#8007) KAFKA-9074: Correct Connect’s `Values.parseString` to properly parse a time and timestamp literal (apache#7568) KAFKA-9492; Ignore record errors in ProduceResponse for older versions (apache#8030)

…t-for-generated-requests * apache-github/trunk: (410 commits) KAFKA-8843: KIP-515: Zookeeper TLS support MINOR: Add missing quote for malformed line content (apache#8070) MINOR: Simplify KafkaProducerTest (apache#8044) KAFKA-9507; AdminClient should check for missing committed offsets (apache#8057) KAFKA-9519: Deprecate the --zookeeper flag in ConfigCommand (apache#8056) KAFKA-9509; Fixing flakiness of MirrorConnectorsIntegrationTest.testReplication (apache#8048) HOTFIX: Fix two test failures in JDK11 (apache#8063) DOCS - clarify transactionalID and idempotent behavior (apache#7821) MINOR: further InternalTopologyBuilder cleanup (apache#8046) MINOR: Add timer for update limit offsets (apache#8047) HOTFIX: Fix spotsbug failure in Kafka examples (apache#8051) KAFKA-9447: Add new customized EOS model example (apache#8031) KAFKA-8164: Add support for retrying failed (apache#8019) HOTFIX: checkstyle for newly added unit test KAFKA-9261; Client should handle unavailable leader metadata (apache#7770) MINOR: Fix typos introduced in KIP-559 (apache#8042) MINOR: Fixing null handilg in ValueAndTimestampSerializer (apache#7679) KAFKA-9113: Clean up task management and state management (apache#7997) MINOR: fix checkstyle issue in ConsumerConfig.java (apache#8038) KAFKA-9491; Increment high watermark after full log truncation (apache#8037) ...

akatona84 · 2020-07-07T14:59:02Z

    ./gradlew clients:test --tests RequestResponseTest

+### Specifying test retries ###
+By default, each failed test is retried once up to a maximum of five retries per test run. Tests are retried at the end of the test task. Adjust these parameters in the following way:


Maybe we should update this line that by default it is not retrying.

KAFKA-8164: Improve test passing rate by rerunning flaky tests

da14627

viktorsomogyi requested a review from ijuma January 29, 2020 13:53

ijuma reviewed Jan 29, 2020

View reviewed changes

Lower defaults, add configurable properties

96baf78

ijuma reviewed Jan 31, 2020

View reviewed changes

viktorsomogyi added 2 commits February 3, 2020 17:23

Update readme, correct namings

9891b23

Add new options to common build options

6d9841b

chia7712 reviewed Feb 5, 2020

View reviewed changes

Update testRetry to 1.1.0

0b3ac63

ijuma added 2 commits February 5, 2020 07:41

Adjust readme

e2db38b

Mention that Readme should be updated if defaults are adjusted

0e47aa0

ijuma approved these changes Feb 5, 2020

View reviewed changes

Change the defaults to 0.

078bc69

ijuma merged commit 987f0ee into apache:trunk Feb 6, 2020

viktorsomogyi deleted the gradle-flaky-retry branch February 6, 2020 11:07

akatona84 reviewed Jul 7, 2020

View reviewed changes

Conversation

viktorsomogyi commented Jan 29, 2020

Committer Checklist (excluded from commit message)

Uh oh!

viktorsomogyi commented Jan 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viktorsomogyi commented Feb 5, 2020

Uh oh!

ijuma commented Feb 5, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ijuma commented Feb 5, 2020

Uh oh!

ijuma left a comment

Choose a reason for hiding this comment

Uh oh!

ijuma commented Feb 5, 2020

Uh oh!

ijuma commented Feb 5, 2020

Uh oh!

ijuma commented Feb 5, 2020

Uh oh!

ijuma commented Feb 5, 2020

Uh oh!

viktorsomogyi commented Feb 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ijuma commented Feb 5, 2020

Uh oh!

ijuma commented Feb 5, 2020

Uh oh!

ijuma commented Feb 6, 2020

Uh oh!

ijuma commented Feb 6, 2020

Uh oh!

viktorsomogyi commented Feb 6, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

viktorsomogyi commented Jan 29, 2020 •

edited

Loading

viktorsomogyi commented Feb 5, 2020 •

edited

Loading