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 flaky test SegmentReplicationIT.testScrollWithOngoingSegmentReplication #7572

Merged
merged 1 commit into from
May 16, 2023

Conversation

mch2
Copy link
Member

@mch2 mch2 commented May 15, 2023

Description

Fix flaky test SegmentReplicationIT.testScrollWithOngoingSegmentReplication

This test has flaky failures for two reasons:

  1. Fetches list of temporary files on disk starting with ".replication" before the replica has time to
    flush received chunks to disk. Fixed by wrapping the assertion that a tmp file exists with assertBusy.
  2. Asserts that the count of tmp files is exactly the same before/after a scroll request is cleared. However,
    it is possible that additional tmp files have been written to disk concurrently, causing a count mismatch. Fixed
    by removing the size assertion. For the sake of this test we don't care if this is the case, as long as the tmp files
    originally fetched remain after a scroll query is cleared.

Related Issues

resolves #7569

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.

…cation

This test has flaky failures for two reasons:
1. Fetches list of temporary files on disk starting with ".replication" before the replica has time to
flush received chunks to disk. Fixed by wrapping the assertion that a tmp file exists with assertBusy.
2. Asserts that the count of tmp files is exactly the same before/after a scroll request is cleared. However,
it is possible that additional tmp files have been written to disk concurrently, causing a count mismatch. Fixed
by removing the size assertion.  For the sake of this test we don't care if this is the case, as long as the tmp files
originally fetched remain after a scroll query is cleared.

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

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.RemoteStoreRefreshListenerIT.testRemoteRefreshRetryOnFailure

@codecov
Copy link

codecov bot commented May 15, 2023

Codecov Report

Merging #7572 (197c84c) into main (27dbcd5) will increase coverage by 0.05%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #7572      +/-   ##
============================================
+ Coverage     70.54%   70.60%   +0.05%     
- Complexity    59713    59753      +40     
============================================
  Files          4896     4896              
  Lines        286798   286798              
  Branches      41331    41331              
============================================
+ Hits         202334   202481     +147     
+ Misses        67761    67646     -115     
+ Partials      16703    16671      -32     

see 495 files with indirect coverage changes

@dreamer-89 dreamer-89 merged commit 804bef4 into opensearch-project:main May 16, 2023
@dreamer-89 dreamer-89 added the backport 2.x Backport to 2.x branch label May 16, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 16, 2023
…cation (#7572)

This test has flaky failures for two reasons:
1. Fetches list of temporary files on disk starting with ".replication" before the replica has time to
flush received chunks to disk. Fixed by wrapping the assertion that a tmp file exists with assertBusy.
2. Asserts that the count of tmp files is exactly the same before/after a scroll request is cleared. However,
it is possible that additional tmp files have been written to disk concurrently, causing a count mismatch. Fixed
by removing the size assertion.  For the sake of this test we don't care if this is the case, as long as the tmp files
originally fetched remain after a scroll query is cleared.

Signed-off-by: Marc Handalian <[email protected]>
(cherry picked from commit 804bef4)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dreamer-89 pushed a commit that referenced this pull request May 16, 2023
…cation (#7572) (#7588)

This test has flaky failures for two reasons:
1. Fetches list of temporary files on disk starting with ".replication" before the replica has time to
flush received chunks to disk. Fixed by wrapping the assertion that a tmp file exists with assertBusy.
2. Asserts that the count of tmp files is exactly the same before/after a scroll request is cleared. However,
it is possible that additional tmp files have been written to disk concurrently, causing a count mismatch. Fixed
by removing the size assertion.  For the sake of this test we don't care if this is the case, as long as the tmp files
originally fetched remain after a scroll query is cleared.


(cherry picked from commit 804bef4)

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>
@mch2 mch2 deleted the 7569 branch May 16, 2023 23:39
stephen-crawford pushed a commit to stephen-crawford/OpenSearch that referenced this pull request May 31, 2023
…cation (opensearch-project#7572)

This test has flaky failures for two reasons:
1. Fetches list of temporary files on disk starting with ".replication" before the replica has time to
flush received chunks to disk. Fixed by wrapping the assertion that a tmp file exists with assertBusy.
2. Asserts that the count of tmp files is exactly the same before/after a scroll request is cleared. However,
it is possible that additional tmp files have been written to disk concurrently, causing a count mismatch. Fixed
by removing the size assertion.  For the sake of this test we don't care if this is the case, as long as the tmp files
originally fetched remain after a scroll query is cleared.

Signed-off-by: Marc Handalian <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…cation (opensearch-project#7572)

This test has flaky failures for two reasons:
1. Fetches list of temporary files on disk starting with ".replication" before the replica has time to
flush received chunks to disk. Fixed by wrapping the assertion that a tmp file exists with assertBusy.
2. Asserts that the count of tmp files is exactly the same before/after a scroll request is cleared. However,
it is possible that additional tmp files have been written to disk concurrently, causing a count mismatch. Fixed
by removing the size assertion.  For the sake of this test we don't care if this is the case, as long as the tmp files
originally fetched remain after a scroll query is cleared.

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] Flaky test SegmentReplicationIT.testScrollWithOngoingSegmentReplication
2 participants