-
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 RemoteFSTranslogTests testMetadataFileDeletion test #9605 #11899
Conversation
…rch-project#9605 Signed-off-by: Ashish Singh <ssashish@amazon.com>
Compatibility status:Checks if related components are compatible with change a63fa6a Incompatible componentsIncompatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/performance-analyzer-rca.git] Skipped componentsCompatible componentsCompatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/k-nn.git] |
❕ Gradle check result for a63fa6a: UNSTABLE
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11899 +/- ##
=========================================
Coverage 71.33% 71.33%
+ Complexity 59300 59273 -27
=========================================
Files 4921 4921
Lines 278989 278989
Branches 40543 40543
=========================================
+ Hits 199014 199027 +13
+ Misses 63444 63375 -69
- Partials 16531 16587 +56 ☔ View full report in Codecov by Sentry. |
Flaky test - #9191 |
server/src/test/java/org/opensearch/index/translog/RemoteFsTranslogTests.java
Show resolved
Hide resolved
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.
How many times did we run the test to see if the flakiness still persists?
Had run it for 1K+ iterations on local. Running the entire class test suite for couple of hours more. |
2K iterations have run without any failures. |
3K iterations ran without any failures. |
904c9a9
into
opensearch-project:main
…rch-project#9605 (opensearch-project#11899) Signed-off-by: Ashish Singh <ssashish@amazon.com>
…rch-project#9605 (opensearch-project#11899) Signed-off-by: Ashish Singh <ssashish@amazon.com>
…rch-project#9605 (opensearch-project#11899) Signed-off-by: Ashish Singh <ssashish@amazon.com> Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Description
This closes #9605. The flakiness in this test is on account of translog.trimUnreferencedReaders() being no-op when called due to previous deletion still ongoing and holding the permits. The current run of translog.trimUnreferencedReaders() on checking for permits sees that the required permits are not available and skips the deletion. To elaborate more on generation deletion, the approach is to acquire the permits, trigger async deletion and post successful deletion, release the permits. With this PR, I am making sure that after
trimUnreferencedReaders()
, we check that the required permits are available before calling the same method back again.Related Issues
Resolves #9605
Check List
New functionality includes testing.New functionality has been documented.New functionality has javadoc addedCommit changes are listed out in CHANGELOG.md file (See: Changelog)Public documentation issue/PR createdBy 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.