Gradle check retry #2638

kotwanikunal · 2022-03-29T06:43:11Z

Description

Adds a retry mechanism for all test classes under the root project
This should help with gradle check failures

Issues Resolved

Retry failed flakey tests #2547

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Kunal Kotwani <[email protected]>

opensearch-ci-bot · 2022-03-29T07:28:48Z

✅ Gradle Check success 811ee66
Log 3841

Reports 3841

reta · 2022-03-29T15:36:57Z

build.gradle

+  apply plugin: "org.gradle.test-retry"
+  tasks.withType(Test).configureEach {
+    retry {
+      if (isCiServer) {


You could probably just not configure the task if it is not CI:

if (isCiServer) { tasks.withType(Test).configureEach { retry { .... } }

Also, I would highly recommend to set maxFailures to some reasonable value, 5 for example:

* The maximum number of test failures that are allowed before retrying is disabled.

Wdyt?

I think we should start by retrying once (maxRetries = 1), which should weed out some of the worst offenders and point us to the failures we should focus on fixing. Otherwise we'll start relying on retries too much. WDYT?

I would make maxFailures a bit larger to really capture broken code vs. multiple flukes, like maybe 10, but don't feel strongly about it.

We would really benefit from something like gradle/test-retry-gradle-plugin#29 for our use case to catch the worst offenders.
I agree with giving it a kick start with 1-2 retries and stopping on multiple test failures. From what I have noticed, it usually takes 2 retries recently to get it going. But let's start small enough to give a boost and not over rely.

I'll update the PR with { maxFailures = 10, maxRetries = 1 }.

dblock · 2022-03-29T16:28:11Z

The fact that gradle check succeeded here from the first try tells me that we're on the right track :)

Signed-off-by: Kunal Kotwani <[email protected]>

opensearch-ci-bot · 2022-03-29T17:59:06Z

❌ Gradle Check failure 3dcf356
Log 3865

Reports 3865

kotwanikunal · 2022-03-29T18:08:30Z

> Task :client:rest-high-level:asyncIntegTest

REPRODUCE WITH: ./gradlew ':client:rest-high-level:asyncIntegTest' --tests "org.opensearch.client.TasksIT.testGetValidTask" -Dtests.seed=91F1364AFC43A70 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=sr-ME -Dtests.timezone=Pacific/Enderbury -Druntime.java=17

org.opensearch.client.TasksIT > testGetValidTask FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([91F1364AFC43A70:9573CA83C6999BB]:0)
        at org.junit.Assert.fail(Assert.java:87)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.junit.Assert.assertTrue(Assert.java:53)
        at org.opensearch.client.OpenSearchRestHighLevelClientTestCase.lambda$checkTaskCompletionStatus$2(OpenSearchRestHighLevelClientTestCase.java:361)
        at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1060)
        at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1033)
        at org.opensearch.client.TasksIT.testGetValidTask(TasksIT.java:123)

Another random failure. Runs fine on my devbox -

> Task :client:rest-high-level:asyncIntegTest
OpenJDK 64-Bit Server VM warning: Ignoring option --illegal-access=warn; support was removed in 17.0
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.opensearch.bootstrap.BootstrapForTesting (file:/Users/kkotwani/workspace/OpenSearch/test/framework/build/distributions/framework-2.0.0-SNAPSHOT.jar)
WARNING: Please consider reporting this to the maintainers of org.opensearch.bootstrap.BootstrapForTesting
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/Users/kkotwani/.gradle/wrapper/dists/gradle-7.3.3-all/4295vidhdd9hd3gbjyw1xqxpo/gradle-7.3.3/lib/plugins/gradle-testing-base-7.3.3.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release

BUILD SUCCESSFUL in 34s
181 actionable tasks: 11 executed, 170 up-to-date

kotwanikunal · 2022-03-29T18:14:33Z

start gradle check

dblock · 2022-03-29T18:17:52Z

@kotwanikunal Did it retry org.opensearch.client.TasksIT.testGetValidTask or asyncIntegTest? Where do you see it in the log?

reta · 2022-03-29T18:25:58Z

@kotwanikunal @dblock I believe the asyncIntegTest may not fall into Test task type:

tasks.withType(Test).configureEach {
    ...
}

We do some tricks here as well https://github.com/opensearch-project/OpenSearch/blob/main/gradle/code-coverage.gradle#L47

kotwanikunal · 2022-03-29T18:29:15Z

@kotwanikunal Did it retry org.opensearch.client.TasksIT.testGetValidTask or asyncIntegTest? Where do you see it in the log?

It only retries the failing tests. I verified that on my local machine with different scenarios including integTest, internalClusterTest

In the individual project logs, you usually see x tests completed, n tests failed with the html report where n counts all the retries.

I will try force failing an asyncIntegTest on my devbox just to be sure.

kotwanikunal · 2022-03-29T18:34:45Z

@kotwanikunal @dblock I believe the asyncIntegTest may not fall into Test task type:
tasks.withType(Test).configureEach {
    ...
}
We do some tricks here as well https://github.com/opensearch-project/OpenSearch/blob/main/gradle/code-coverage.gradle#L47

It does infact cover the asyncIntegTest type.

I ran ./gradlew ':client:rest-high-level:asyncIntegTest' --tests "org.opensearch.client.TasksIT.testGetValidTask" -Dtests.seed=91F1364AFC43A70 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=sr-ME -Dtests.timezone=Pacific/Enderbury -Druntime.java=17 on my devbox by adding in a failing assert.

It looks like it retries the failing asyncIntegTest type -

dblock

This is great! If maxRetries = 1 is too noisy, I am ok with 2 or 3.

tlfeng · 2022-03-29T18:41:04Z

About asyncIntegTest task, seems it's under Test type.

Its direct type is RestIntegTestTask https://github.com/opensearch-project/OpenSearch/blob/1.3.0/client/rest-high-level/build.gradle#L84.
Then RestIntegTestTask extends StandaloneRestIntegTestTask, which extends Test.
https://github.com/opensearch-project/OpenSearch/blob/1.3.0/buildSrc/src/main/java/org/opensearch/gradle/test/RestIntegTestTask.java#L44
https://github.com/opensearch-project/OpenSearch/blob/1.3.0/buildSrc/src/main/java/org/opensearch/gradle/testclusters/StandaloneRestIntegTestTask.java#L61

tlfeng · 2022-03-29T18:43:40Z

What wired here is I didn't see the retry occurs in the above failed gradle check, in the "Reports 3865"

opensearch-ci-bot · 2022-03-29T18:44:43Z

❌ Gradle Check failure 3dcf356
Log 3868

Reports 3868

dblock · 2022-03-29T18:48:20Z

What wired here is I didn't see the retry occurs in the above failed gradle check, in the "report 3865"

Is boolean isCiServer = System.getenv().containsKey("CI") failing here? Maybe just put retry all the time? It's useful for dev too.

kotwanikunal · 2022-03-29T18:50:38Z

What wired here is I didn't see the retry occurs in the above failed gradle check, in the "report 3865"

Is boolean isCiServer = System.getenv().containsKey("CI") failing here? Maybe just put retry all the time? It's useful for dev too.

Looks like it. Since it's a single retry, let me get rid of the CI condition. I can iteratively improve on it if we decide to have different configs for dev/CI.

reta · 2022-03-29T18:53:59Z

@kotwanikunal use BuildParams instead?

if (BuildParams.isCi() == true) {
}

kotwanikunal · 2022-03-29T19:00:25Z

@kotwanikunal use BuildParams instead?
if (BuildParams.isCi() == true) {
}

@reta How about adding in a single retry for dev and CI for now?

dblock · 2022-03-29T19:02:25Z

@kotwanikunal use BuildParams instead?
if (BuildParams.isCi() == true) {
}
@reta How about adding in a single retry for dev and CI for now?

Let's vote and if we have a deadlock, we'll just try again? :) I'm good with anything.

reta · 2022-03-29T19:18:09Z

Let's vote and if we have a deadlock, we'll just try again? :) I'm good with anything.

I am fine with that ... for now

Signed-off-by: Kunal Kotwani <[email protected]>

mch2 · 2022-03-29T19:22:51Z

How do we ensure we aren't swallowing failures with reproducible seeds? Does it retry with the same seed?

reta · 2022-03-29T19:40:35Z

How do we ensure we aren't swallowing failures with reproducible seeds? Does it retry with the same seed?

I am not 100% sure but I would doubt the seed is preserved

opensearch-ci-bot · 2022-03-29T19:53:17Z

❌ Gradle Check failure 37e398d
Log 3874

Reports 3874

reta · 2022-03-29T20:05:01Z

Aha, this one seems to retried

And same seed was used

testHAProxyModeConnectionWorks
java.lang.AssertionError
	at __randomizedtesting.SeedInfo.seed([108B309DF15FC962:17FDE556CAD9BF3F]:0)
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)

java.lang.AssertionError
	at __randomizedtesting.SeedInfo.seed([108B309DF15FC962:17FDE556CAD9BF3F]:0)
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)
	at org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks(RemoteClustersIT.java:125)
	at java.base/jdk.internal.

@mch2 ^^

@dblock seems like 1 retry is not enough ... or we need to retry the while suite?

kotwanikunal · 2022-03-29T20:30:54Z

Open issue for this test: #1703

kotwanikunal · 2022-03-29T20:35:39Z

Aha, this one seems to retried

And same seed was used

testHAProxyModeConnectionWorks
java.lang.AssertionError
	at __randomizedtesting.SeedInfo.seed([108B309DF15FC962:17FDE556CAD9BF3F]:0)
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)

java.lang.AssertionError
	at __randomizedtesting.SeedInfo.seed([108B309DF15FC962:17FDE556CAD9BF3F]:0)
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)
	at org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks(RemoteClustersIT.java:125)
	at java.base/jdk.internal.

@mch2 ^^

@dblock seems like 1 retry is not enough ... or we need to retry the while suite?

Retrying the whole suite will make it difficult to pinpoint flakiness to particular tests. This helps us get to the most frequent offenders IMHO.
But I am all ears about what y'all have to say 🙂

reta · 2022-03-29T21:21:14Z

Retrying the whole suite will make it difficult to pinpoint flakiness to particular tests. This helps us get to the most frequent offenders IMHO.

I agree, I don't like that at all, thinking loudly - if cluster ended up in "bad" ( == not expected) state at startup, the test may never succeed ... but we should consider that to be separate problem I guess

dblock · 2022-03-29T23:14:46Z

Do you want to increase the number of retries @kotwanikunal?

Signed-off-by: Kunal Kotwani <[email protected]>

opensearch-ci-bot · 2022-03-30T00:43:26Z

✅ Gradle Check success 7e141ba
Log 3880

Reports 3880

* Add retry plugin support for Test implementations Signed-off-by: Kunal Kotwani <[email protected]> * Update test retry parameters Signed-off-by: Kunal Kotwani <[email protected]> * Remove CI environment check for test retries Signed-off-by: Kunal Kotwani <[email protected]> * Update retry count for tests Signed-off-by: Kunal Kotwani <[email protected]> (cherry picked from commit 65cc56e)

opensearch-trigger-bot · 2022-03-30T12:25:13Z

The backport to 1.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.x 1.x
# Navigate to the new working tree
cd .worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-2638-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 65cc56e754e4a854963663ead5d06ea6e975d1eb
# Push it to GitHub
git push --set-upstream origin backport/backport-2638-to-1.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.x

Then, create a pull request where the base branch is 1.x and the compare/head branch is backport/backport-2638-to-1.x.

Add retry plugin support for Test implementations Signed-off-by: Kunal Kotwani <[email protected]> (cherry picked from commit 65cc56e) Co-authored-by: Kunal Kotwani <[email protected]>

Add retry plugin support for Test implementations Signed-off-by: Kunal Kotwani <[email protected]> (cherry picked from commit 65cc56e)

tlfeng · 2022-03-31T06:16:46Z

@kotwanikunal Would you backport the change into 1.x branch?

* Add retry plugin support for Test implementations Signed-off-by: Kunal Kotwani <[email protected]> * Update test retry parameters Signed-off-by: Kunal Kotwani <[email protected]> * Remove CI environment check for test retries Signed-off-by: Kunal Kotwani <[email protected]> * Update retry count for tests Signed-off-by: Kunal Kotwani <[email protected]> (cherry picked from commit 65cc56e)

* Adds a retry mechanism for all test classes under the root project * This will help with gradle check failed by flaky tests Signed-off-by: Kunal Kotwani <[email protected]> (cherry picked from commit 65cc56e)

kotwanikunal added 2 commits March 28, 2022 23:40

Add retry plugin support for Test implementations

8353a75

Signed-off-by: Kunal Kotwani <[email protected]>

Merge branch 'opensearch-project:main' into gradle-check-retry

811ee66

kotwanikunal requested a review from a team as a code owner March 29, 2022 06:43

reta reviewed Mar 29, 2022

View reviewed changes

Update test retry parameters

3dcf356

Signed-off-by: Kunal Kotwani <[email protected]>

kartg approved these changes Mar 29, 2022

View reviewed changes

dblock approved these changes Mar 29, 2022

View reviewed changes

Remove CI environment check for test retries

37e398d

Signed-off-by: Kunal Kotwani <[email protected]>

Update retry count for tests

7e141ba

Signed-off-by: Kunal Kotwani <[email protected]>

kotwanikunal merged commit 65cc56e into opensearch-project:main Mar 30, 2022

kotwanikunal deleted the gradle-check-retry branch March 30, 2022 04:03

dblock added backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch backport 1.x labels Mar 30, 2022

opensearch-trigger-bot bot mentioned this pull request Mar 30, 2022

[Backport 2.0] Gradle check retry #2661

Merged

opensearch-trigger-bot bot mentioned this pull request Mar 30, 2022

[Backport 2.x] Gradle check retry #2662

Merged

nknize pushed a commit that referenced this pull request Mar 30, 2022

Gradle check retry (#2638) (#2662)

e705e72

Add retry plugin support for Test implementations Signed-off-by: Kunal Kotwani <[email protected]> (cherry picked from commit 65cc56e)

This was referenced Mar 31, 2022

Adding test-retry plugin opensearch-project/anomaly-detection#456

Merged

Detailed test report for github actions opensearch-project/anomaly-detection#480

Open

dblock mentioned this pull request Apr 6, 2022

[PROPOSAL] Add flaky test retry opensearch-project/opensearch-plugins#139

Open

kotwanikunal mentioned this pull request Apr 6, 2022

[Gradle Check Retry] Publish/Record tests which pass after a retry to address flakiness #2797

Open

kotwanikunal mentioned this pull request Nov 14, 2022

[BUG] Gradle check retry not working as expected #5239

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gradle check retry #2638

Gradle check retry #2638

kotwanikunal commented Mar 29, 2022

opensearch-ci-bot commented Mar 29, 2022

reta Mar 29, 2022 •

edited

Loading

reta Mar 29, 2022

dblock Mar 29, 2022

kotwanikunal Mar 29, 2022

dblock commented Mar 29, 2022

opensearch-ci-bot commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022 •

edited

Loading

kotwanikunal commented Mar 29, 2022

dblock commented Mar 29, 2022 •

edited

Loading

reta commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

dblock left a comment •

edited

Loading

tlfeng commented Mar 29, 2022

tlfeng commented Mar 29, 2022 •

edited

Loading

opensearch-ci-bot commented Mar 29, 2022

dblock commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

reta commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

dblock commented Mar 29, 2022 •

edited

Loading

reta commented Mar 29, 2022

mch2 commented Mar 29, 2022

reta commented Mar 29, 2022

opensearch-ci-bot commented Mar 29, 2022

reta commented Mar 29, 2022 •

edited

Loading

kotwanikunal commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

reta commented Mar 29, 2022

dblock commented Mar 29, 2022

opensearch-ci-bot commented Mar 30, 2022

opensearch-trigger-bot bot commented Mar 30, 2022

tlfeng commented Mar 31, 2022

Gradle check retry #2638

Gradle check retry #2638

Conversation

kotwanikunal commented Mar 29, 2022

Description

Issues Resolved

Check List

opensearch-ci-bot commented Mar 29, 2022

reta Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

reta Mar 29, 2022

Choose a reason for hiding this comment

dblock Mar 29, 2022

Choose a reason for hiding this comment

kotwanikunal Mar 29, 2022

Choose a reason for hiding this comment

dblock commented Mar 29, 2022

opensearch-ci-bot commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022 • edited Loading

kotwanikunal commented Mar 29, 2022

dblock commented Mar 29, 2022 • edited Loading

reta commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

dblock left a comment • edited Loading

Choose a reason for hiding this comment

tlfeng commented Mar 29, 2022

tlfeng commented Mar 29, 2022 • edited Loading

opensearch-ci-bot commented Mar 29, 2022

dblock commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

reta commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

dblock commented Mar 29, 2022 • edited Loading

reta commented Mar 29, 2022

mch2 commented Mar 29, 2022

reta commented Mar 29, 2022

opensearch-ci-bot commented Mar 29, 2022

reta commented Mar 29, 2022 • edited Loading

kotwanikunal commented Mar 29, 2022

kotwanikunal commented Mar 29, 2022

reta commented Mar 29, 2022

dblock commented Mar 29, 2022

opensearch-ci-bot commented Mar 30, 2022

opensearch-trigger-bot bot commented Mar 30, 2022

tlfeng commented Mar 31, 2022

reta Mar 29, 2022 •

edited

Loading

kotwanikunal commented Mar 29, 2022 •

edited

Loading

dblock commented Mar 29, 2022 •

edited

Loading

dblock left a comment •

edited

Loading

tlfeng commented Mar 29, 2022 •

edited

Loading

dblock commented Mar 29, 2022 •

edited

Loading

reta commented Mar 29, 2022 •

edited

Loading