-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 flaky ResourceAwareTasksTests.testBasicTaskResourceTracking test #8993
Conversation
Signed-off-by: Ketan Verma <ketan9495@gmail.com>
Gradle Check (Jenkins) Run Completed with:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On #8213, this test failure seems hard to reproduce on local. How are we confident this change will resolve the issue on our agent nodes?
Currently this class suite is part of the retry classes - #8825 - We can merge in this change and monitor to see if any further failures occur for a fixed time period (~2 weeks) before we remove it from the list of retried classes.
@Poojita-Raj I can reliably reproduce the issue with this command:
This test fails due to the following reasons – (1) addition of new code paths have increased the class loading overhead, (2) the randomized test seed influences the test execution order in this suite, which in turn influences when the class loading overhead is incurred. Using a different seed, for example, may not fail the test. |
Thanks for clarifying @ketanv3 :) This PR lgtm! |
…pensearch-project#8993) Signed-off-by: Ketan Verma <ketan9495@gmail.com> Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>
…pensearch-project#8993) Signed-off-by: Ketan Verma <ketan9495@gmail.com> Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>
…pensearch-project#8993) Signed-off-by: Ketan Verma <ketan9495@gmail.com> Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Description
The
assertMemoryUsageWithinLimits
method checks if the memory usage is within 5% (up to 200 KB) of the expected memory usage. This additional buffer is to account for the class loading overhead when new code is encountered at runtime. Over time, addition of new code paths has resulted in greater overhead. This PR bumps up the 200 KB limit to 500 KB.Related Issues
Resolves #8213
Check List
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.