-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #7972 SearchBackpressureIT flaky tests #8063
Fix #7972 SearchBackpressureIT flaky tests #8063
Conversation
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Failure does not seem related:
|
@scrawfor99 can you rebase your branch with latest |
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Stephen Crawford <[email protected]>
75efaf2
to
5b11848
Compare
Signed-off-by: Stephen Crawford <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
@dbwiddis, @reta, or @owaiskazi19 would any of you being able to merge this? Thank you. |
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
@scrawfor99 do you think we can document the test setup you tried for reproducing flaky tests like somewhere? I don't know the right place probably in TESTING.md. Let's see if folks agree to it. |
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Hi @owaiskazi19, I can definitely add something but to be honest it was not very scientific... I use IntelliJ so I just ran the tests a handful of times with the IDE testing framework. @ketanv3 had provided quite a bit of insight here as someone who understands the use case and components around these tests much better than I do. |
* fix thread issue Signed-off-by: Stephen Crawford <[email protected]> * fix thread issue Signed-off-by: Stephen Crawford <[email protected]> * Fix thresholds Signed-off-by: Stephen Crawford <[email protected]> * Swap to object based Signed-off-by: Stephen Crawford <[email protected]> * Spotless Signed-off-by: Stephen Crawford <[email protected]> * Swap to preserve nulls Signed-off-by: Stephen Crawford <[email protected]> * Spotless Signed-off-by: Stephen Crawford <[email protected]> * Resolve npe Signed-off-by: Stephen Crawford <[email protected]> * remove final declerations Signed-off-by: Stephen Crawford <[email protected]> * spotless Signed-off-by: Stephen Crawford <[email protected]> * add annotations Signed-off-by: Stephen Crawford <[email protected]> * push to rerun tests Signed-off-by: Stephen Crawford <[email protected]> * Fix idea Signed-off-by: Stephen Crawford <[email protected]> * Fix idea Signed-off-by: Stephen Crawford <[email protected]> --------- Signed-off-by: Stephen Crawford <[email protected]> (cherry picked from commit 63dc6aa) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix thread issue * fix thread issue * Fix thresholds * Swap to object based * Spotless * Swap to preserve nulls * Spotless * Resolve npe * remove final declerations * spotless * add annotations * push to rerun tests * Fix idea * Fix idea --------- (cherry picked from commit 63dc6aa) Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…rch-project#8063) (opensearch-project#8217) * fix thread issue * fix thread issue * Fix thresholds * Swap to object based * Spotless * Swap to preserve nulls * Spotless * Resolve npe * remove final declerations * spotless * add annotations * push to rerun tests * Fix idea * Fix idea --------- (cherry picked from commit 63dc6aa) Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…rch-project#8063) * fix thread issue Signed-off-by: Stephen Crawford <[email protected]> * fix thread issue Signed-off-by: Stephen Crawford <[email protected]> * Fix thresholds Signed-off-by: Stephen Crawford <[email protected]> * Swap to object based Signed-off-by: Stephen Crawford <[email protected]> * Spotless Signed-off-by: Stephen Crawford <[email protected]> * Swap to preserve nulls Signed-off-by: Stephen Crawford <[email protected]> * Spotless Signed-off-by: Stephen Crawford <[email protected]> * Resolve npe Signed-off-by: Stephen Crawford <[email protected]> * remove final declerations Signed-off-by: Stephen Crawford <[email protected]> * spotless Signed-off-by: Stephen Crawford <[email protected]> * add annotations Signed-off-by: Stephen Crawford <[email protected]> * push to rerun tests Signed-off-by: Stephen Crawford <[email protected]> * Fix idea Signed-off-by: Stephen Crawford <[email protected]> * Fix idea Signed-off-by: Stephen Crawford <[email protected]> --------- Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Rishab Nahata <[email protected]>
…rch-project#8063) * fix thread issue Signed-off-by: Stephen Crawford <[email protected]> * fix thread issue Signed-off-by: Stephen Crawford <[email protected]> * Fix thresholds Signed-off-by: Stephen Crawford <[email protected]> * Swap to object based Signed-off-by: Stephen Crawford <[email protected]> * Spotless Signed-off-by: Stephen Crawford <[email protected]> * Swap to preserve nulls Signed-off-by: Stephen Crawford <[email protected]> * Spotless Signed-off-by: Stephen Crawford <[email protected]> * Resolve npe Signed-off-by: Stephen Crawford <[email protected]> * remove final declerations Signed-off-by: Stephen Crawford <[email protected]> * spotless Signed-off-by: Stephen Crawford <[email protected]> * add annotations Signed-off-by: Stephen Crawford <[email protected]> * push to rerun tests Signed-off-by: Stephen Crawford <[email protected]> * Fix idea Signed-off-by: Stephen Crawford <[email protected]> * Fix idea Signed-off-by: Stephen Crawford <[email protected]> --------- Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Description
This PR is attempt number 2 at remediating the flaky tests in
**/SearchBackpressureIT
. The two tests focusing on high CPU usage were prone to failure do to thread concurrency issues where the assignment of the CancellableTask values were not synchronized across the nodes.To resolve the issue, I added in a small static class as suggested by @ketanv3 and used an atomic reference through the SetOnce class. This allows for atomic assignments of the class variables without requiring a synchronized block around the
cancel(reason)
method.After the change, I ran a runner for 10 times with no failures so it appears to be fixed.
One note about this test setup: I am not sure we want tests in this vain. The way the tests are made, we end up with non-deterministic behavior. For instance, we set the HIGH_CPU tests to be 1000 iterations, but that is just an arbitrary choice for a iteration count. On a more or less powerful processor this could have different results. This is especially true with different architectures that could have stack unwinding or other features.
Related Issues
Resolves #7972
Check List
New functionality includes testing.All tests passNew functionality has been documented.New functionality has javadoc addedBy 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.