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

Fix SegmentReplicationUsingRemoteStoreIT.testReplicaHasDiffFilesThanPrimary. #8863

Merged
merged 1 commit into from
Jul 26, 2023

Conversation

mch2
Copy link
Member

@mch2 mch2 commented Jul 25, 2023

Description

This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case it is not guaranteed the directory implementation will replace the existing file when copying from the store. This change ensures the mismatched file is cleaned up but only if the shard is not serving reads. In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.

Related Issues

Resolves #7643

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.

This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case
it is not guaranteed the directory implementation will replace the existing file when copying from the store.  This change ensures the mismatched file is cleaned up but only if the shard is not serving reads.  In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.

Signed-off-by: Marc Handalian <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #8863 (2ae5f8f) into main (d9abf12) will decrease coverage by 0.08%.
The diff coverage is 40.00%.

@@             Coverage Diff              @@
##               main    #8863      +/-   ##
============================================
- Coverage     71.12%   71.04%   -0.08%     
+ Complexity    57233    57213      -20     
============================================
  Files          4757     4757              
  Lines        269804   269807       +3     
  Branches      39454    39455       +1     
============================================
- Hits         191898   191694     -204     
- Misses        61698    61954     +256     
+ Partials      16208    16159      -49     
Files Changed Coverage Δ
...in/java/org/opensearch/index/shard/IndexShard.java 69.51% <0.00%> (+0.20%) ⬆️
...search/index/shard/RemoteStoreRefreshListener.java 84.61% <100.00%> (+0.54%) ⬆️

... and 467 files with indirect coverage changes

@mch2 mch2 merged commit e1a4125 into opensearch-project:main Jul 26, 2023
10 of 34 checks passed
@mch2 mch2 added the backport 2.x Backport to 2.x branch label Jul 26, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 26, 2023
This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case
it is not guaranteed the directory implementation will replace the existing file when copying from the store.  This change ensures the mismatched file is cleaned up but only if the shard is not serving reads.  In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.

Signed-off-by: Marc Handalian <[email protected]>
(cherry picked from commit e1a4125)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
mch2 pushed a commit that referenced this pull request Jul 26, 2023
This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case
it is not guaranteed the directory implementation will replace the existing file when copying from the store.  This change ensures the mismatched file is cleaned up but only if the shard is not serving reads.  In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.

Signed-off-by: Marc Handalian <[email protected]>
(cherry picked from commit e1a4125)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
mch2 pushed a commit that referenced this pull request Jul 26, 2023
This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case
it is not guaranteed the directory implementation will replace the existing file when copying from the store.  This change ensures the mismatched file is cleaned up but only if the shard is not serving reads.  In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.


(cherry picked from commit e1a4125)

Signed-off-by: Marc Handalian <[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>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/OpenSearch that referenced this pull request Jul 27, 2023
This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case
it is not guaranteed the directory implementation will replace the existing file when copying from the store.  This change ensures the mismatched file is cleaned up but only if the shard is not serving reads.  In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.

Signed-off-by: Marc Handalian <[email protected]>
baba-devv pushed a commit to baba-devv/OpenSearch that referenced this pull request Jul 29, 2023
This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case
it is not guaranteed the directory implementation will replace the existing file when copying from the store.  This change ensures the mismatched file is cleaned up but only if the shard is not serving reads.  In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.

Signed-off-by: Marc Handalian <[email protected]>
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case
it is not guaranteed the directory implementation will replace the existing file when copying from the store.  This change ensures the mismatched file is cleaned up but only if the shard is not serving reads.  In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.

Signed-off-by: Marc Handalian <[email protected]>
Signed-off-by: Kaushal Kumar <[email protected]>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case
it is not guaranteed the directory implementation will replace the existing file when copying from the store.  This change ensures the mismatched file is cleaned up but only if the shard is not serving reads.  In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.

Signed-off-by: Marc Handalian <[email protected]>
Signed-off-by: Ivan Brusic <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
This test is failing in two ways.
First it fails when copying segments from the remote store and there is a cksum mismatch. In this case
it is not guaranteed the directory implementation will replace the existing file when copying from the store.  This change ensures the mismatched file is cleaned up but only if the shard is not serving reads.  In that case we fail the shard so it is re-recovered rather than deleting the segment underneath it.

This test also fails with a divide by 0 in RemoteStoreRefreshListener.

Signed-off-by: Marc Handalian <[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.remotestore.SegmentReplicationRemoteStoreIT.testReplicaHasDiffFilesThanPrimary is flaky
4 participants