Skip to content
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

Bugfix: remove assert on non-empty translog reader list #9458

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

sachinpkale
Copy link
Member

@sachinpkale sachinpkale commented Aug 21, 2023

Description

  • Currently, in RemoteFsTranslog, as part of trimUnreferencedReaders, we delete stale remote primary terms.
  • The deleteStaleRemotePrimaryTerms function assumes existing list of translog readers to be non empty and have a corresponding assert.
  • This assert results in flaky behavior of tests like: [BUG] org.opensearch.index.translog.RemoteFSTranslogTests.testConcurrentWriteViewsAndSnapshot is flaky #9455
  • In this PR, we remove the assert and return from deleteStaleRemotePrimaryTerms if translog reader list is empty.
  • It is safe to remove the assert as translog reader list can be empty due to super.trimUnreferencedReaders() that is executed prior.

Related Issues

Resolves #9455

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
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:

Checks if related components are compatible with change 784a473

Incompatible components

Incompatible components: [https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/asynchronous-search.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git]

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #9458 (175ba4d) into main (784a473) will decrease coverage by 0.04%.
Report is 1 commits behind head on main.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main    #9458      +/-   ##
============================================
- Coverage     71.18%   71.14%   -0.04%     
+ Complexity    57453    57432      -21     
============================================
  Files          4776     4776              
  Lines        270813   270815       +2     
  Branches      39582    39582              
============================================
- Hits         192780   192680     -100     
- Misses        61871    61928      +57     
- Partials      16162    16207      +45     
Files Changed Coverage Δ
...rg/opensearch/index/translog/RemoteFsTranslog.java 70.10% <0.00%> (-0.78%) ⬇️

... and 438 files with indirect coverage changes

Signed-off-by: Sachin Kale <[email protected]>
@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:

Checks if related components are compatible with change 61c5f17

Incompatible components

Incompatible components: [https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/asynchronous-search.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/reporting.git]

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@sachinpkale
Copy link
Member Author

Byapssing codecov/patch as the changes fix a flaky test.

@sachinpkale sachinpkale merged commit 48d4087 into opensearch-project:main Aug 22, 2023
7 of 11 checks passed
@sachinpkale sachinpkale added the backport 2.x Backport to 2.x branch label Aug 22, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 22, 2023
---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
(cherry picked from commit 48d4087)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
tlfeng pushed a commit that referenced this pull request Aug 22, 2023
austintlee pushed a commit to austintlee/OpenSearch that referenced this pull request Aug 25, 2023
Gaganjuneja pushed a commit to Gaganjuneja/OpenSearch that referenced this pull request Aug 28, 2023
Gaganjuneja pushed a commit to Gaganjuneja/OpenSearch that referenced this pull request Aug 28, 2023
…roject#9458)

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
Signed-off-by: Gagan Juneja <[email protected]>
kkmr pushed a commit to kkmr/OpenSearch that referenced this pull request Aug 28, 2023
…roject#9458)

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
Signed-off-by: Kiran Reddy <[email protected]>
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
…roject#9458)

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
Signed-off-by: Kaushal Kumar <[email protected]>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…roject#9458)

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
Signed-off-by: Ivan Brusic <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…roject#9458)

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] org.opensearch.index.translog.RemoteFSTranslogTests.testConcurrentWriteViewsAndSnapshot is flaky
3 participants