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

[Segment Replication] Prevent store clean up post reader close and refactor #8463

Merged
merged 10 commits into from
Jul 8, 2023

Conversation

dreamer-89
Copy link
Member

@dreamer-89 dreamer-89 commented Jul 6, 2023

Description

  1. Prevents the store clean up via cleanupAndPreserveLatestCommitPoint method on OpenSearchReaderManager close operation. Removes the cleanupTempFiles boolean flag from store cleanup as only files which are dec'ref to 0 are cleaned.
  2. Refactors SegmentReplicationTarget finalize replication by moving to store.
  3. Refactor Store clean up utilities.

Related Issues

Resolves #8286

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

github-actions bot commented Jul 6, 2023

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Suraj Singh <[email protected]>
@dreamer-89 dreamer-89 changed the title [Segment Replication] Prevent store clean up post reader close and cleanup [Segment Replication] Prevent store clean up post reader close and refactor Jul 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2023

Gradle Check (Jenkins) Run Completed with:

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

github-actions bot commented Jul 7, 2023

Gradle Check (Jenkins) Run Completed with:

@dreamer-89
Copy link
Member Author

Gradle Check (Jenkins) Run Completed with:

Flaky test failure. Tracked in #6287

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      2 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.classMethod
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testPressureServiceStats

@codecov
Copy link

codecov bot commented Jul 8, 2023

Codecov Report

Merging #8463 (ae456c0) into main (4657fe7) will decrease coverage by 0.08%.
The diff coverage is 89.28%.

@@             Coverage Diff              @@
##               main    #8463      +/-   ##
============================================
- Coverage     70.89%   70.82%   -0.08%     
+ Complexity    56927    56854      -73     
============================================
  Files          4758     4758              
  Lines        269219   269222       +3     
  Branches      39407    39405       -2     
============================================
- Hits         190876   190687     -189     
- Misses        62268    62449     +181     
- Partials      16075    16086      +11     
Impacted Files Coverage Δ
...arch/index/engine/NRTReplicationReaderManager.java 90.90% <ø> (ø)
...rc/main/java/org/opensearch/index/store/Store.java 80.83% <85.00%> (+2.74%) ⬆️
.../opensearch/index/engine/NRTReplicationEngine.java 71.79% <100.00%> (-4.90%) ⬇️
...g/opensearch/indices/recovery/MultiFileWriter.java 85.14% <100.00%> (+0.14%) ⬆️
.../indices/replication/SegmentReplicationTarget.java 84.34% <100.00%> (-2.10%) ⬇️

... and 470 files with indirect coverage changes

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Suraj Singh <[email protected]>
Signed-off-by: Suraj Singh <[email protected]>
Copy link
Member

@mch2 mch2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for cleaning this up.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.snapshots.DedicatedClusterSnapshotRestoreIT.testIndexDeletionDuringSnapshotCreationInQueue

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Gradle Check (Jenkins) Run Completed with:

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Gradle Check (Jenkins) Run Completed with:

@dreamer-89 dreamer-89 merged commit 516685d into opensearch-project:main Jul 8, 2023
@dreamer-89 dreamer-89 added the backport 2.x Backport to 2.x branch label Jul 8, 2023
@dreamer-89
Copy link
Member Author

@mch2 : Is this fix also needed in 2.7 and 2.8 branch ?

opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 8, 2023
…factor (#8463)

* [Segment Replication] Prevent store clean up on reader close action

Signed-off-by: Suraj Singh <[email protected]>

* [Segment Replication] Self review

Signed-off-by: Suraj Singh <[email protected]>

* Address review comment

Signed-off-by: Suraj Singh <[email protected]>

* Address review comments & refactor

Signed-off-by: Suraj Singh <[email protected]>

* Comment

Signed-off-by: Suraj Singh <[email protected]>

* Fix unit test

Signed-off-by: Suraj Singh <[email protected]>

* Unit test to verify temporary files are not deleted from commits

Signed-off-by: Suraj Singh <[email protected]>

* Compilation error fix

Signed-off-by: Suraj Singh <[email protected]>

* Javadoc

Signed-off-by: Suraj Singh <[email protected]>

* Skip testScrollWithConcurrentIndexAndSearch with remote store

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Suraj Singh <[email protected]>
(cherry picked from commit 516685d)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dreamer-89 added a commit that referenced this pull request Jul 9, 2023
…er close and refactor (#8548)

* [Segment Replication] Prevent store clean up post reader close and refactor (#8463)

* [Segment Replication] Prevent store clean up on reader close action

Signed-off-by: Suraj Singh <[email protected]>

* [Segment Replication] Self review

Signed-off-by: Suraj Singh <[email protected]>

* Address review comment

Signed-off-by: Suraj Singh <[email protected]>

* Address review comments & refactor

Signed-off-by: Suraj Singh <[email protected]>

* Comment

Signed-off-by: Suraj Singh <[email protected]>

* Fix unit test

Signed-off-by: Suraj Singh <[email protected]>

* Unit test to verify temporary files are not deleted from commits

Signed-off-by: Suraj Singh <[email protected]>

* Compilation error fix

Signed-off-by: Suraj Singh <[email protected]>

* Javadoc

Signed-off-by: Suraj Singh <[email protected]>

* Skip testScrollWithConcurrentIndexAndSearch with remote store

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Suraj Singh <[email protected]>
(cherry picked from commit 516685d)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Use master eligible node

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Suraj Singh <[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>
Co-authored-by: Suraj Singh <[email protected]>
vikasvb90 pushed a commit to raghuvanshraj/OpenSearch that referenced this pull request Jul 12, 2023
…factor (opensearch-project#8463)

* [Segment Replication] Prevent store clean up on reader close action

Signed-off-by: Suraj Singh <[email protected]>

* [Segment Replication] Self review

Signed-off-by: Suraj Singh <[email protected]>

* Address review comment

Signed-off-by: Suraj Singh <[email protected]>

* Address review comments & refactor

Signed-off-by: Suraj Singh <[email protected]>

* Comment

Signed-off-by: Suraj Singh <[email protected]>

* Fix unit test

Signed-off-by: Suraj Singh <[email protected]>

* Unit test to verify temporary files are not deleted from commits

Signed-off-by: Suraj Singh <[email protected]>

* Compilation error fix

Signed-off-by: Suraj Singh <[email protected]>

* Javadoc

Signed-off-by: Suraj Singh <[email protected]>

* Skip testScrollWithConcurrentIndexAndSearch with remote store

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Suraj Singh <[email protected]>
raghuvanshraj pushed a commit to raghuvanshraj/OpenSearch that referenced this pull request Jul 12, 2023
…factor (opensearch-project#8463)

* [Segment Replication] Prevent store clean up on reader close action

Signed-off-by: Suraj Singh <[email protected]>

* [Segment Replication] Self review

Signed-off-by: Suraj Singh <[email protected]>

* Address review comment

Signed-off-by: Suraj Singh <[email protected]>

* Address review comments & refactor

Signed-off-by: Suraj Singh <[email protected]>

* Comment

Signed-off-by: Suraj Singh <[email protected]>

* Fix unit test

Signed-off-by: Suraj Singh <[email protected]>

* Unit test to verify temporary files are not deleted from commits

Signed-off-by: Suraj Singh <[email protected]>

* Compilation error fix

Signed-off-by: Suraj Singh <[email protected]>

* Javadoc

Signed-off-by: Suraj Singh <[email protected]>

* Skip testScrollWithConcurrentIndexAndSearch with remote store

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Suraj Singh <[email protected]>
dzane17 pushed a commit to dzane17/OpenSearch that referenced this pull request Jul 12, 2023
…factor (opensearch-project#8463)

* [Segment Replication] Prevent store clean up on reader close action

Signed-off-by: Suraj Singh <[email protected]>

* [Segment Replication] Self review

Signed-off-by: Suraj Singh <[email protected]>

* Address review comment

Signed-off-by: Suraj Singh <[email protected]>

* Address review comments & refactor

Signed-off-by: Suraj Singh <[email protected]>

* Comment

Signed-off-by: Suraj Singh <[email protected]>

* Fix unit test

Signed-off-by: Suraj Singh <[email protected]>

* Unit test to verify temporary files are not deleted from commits

Signed-off-by: Suraj Singh <[email protected]>

* Compilation error fix

Signed-off-by: Suraj Singh <[email protected]>

* Javadoc

Signed-off-by: Suraj Singh <[email protected]>

* Skip testScrollWithConcurrentIndexAndSearch with remote store

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Suraj Singh <[email protected]>
buddharajusahil pushed a commit to buddharajusahil/OpenSearch that referenced this pull request Jul 18, 2023
…factor (opensearch-project#8463)

* [Segment Replication] Prevent store clean up on reader close action

Signed-off-by: Suraj Singh <[email protected]>

* [Segment Replication] Self review

Signed-off-by: Suraj Singh <[email protected]>

* Address review comment

Signed-off-by: Suraj Singh <[email protected]>

* Address review comments & refactor

Signed-off-by: Suraj Singh <[email protected]>

* Comment

Signed-off-by: Suraj Singh <[email protected]>

* Fix unit test

Signed-off-by: Suraj Singh <[email protected]>

* Unit test to verify temporary files are not deleted from commits

Signed-off-by: Suraj Singh <[email protected]>

* Compilation error fix

Signed-off-by: Suraj Singh <[email protected]>

* Javadoc

Signed-off-by: Suraj Singh <[email protected]>

* Skip testScrollWithConcurrentIndexAndSearch with remote store

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Suraj Singh <[email protected]>
Signed-off-by: sahil buddharaju <[email protected]>
baba-devv pushed a commit to baba-devv/OpenSearch that referenced this pull request Jul 29, 2023
…factor (opensearch-project#8463)

* [Segment Replication] Prevent store clean up on reader close action

Signed-off-by: Suraj Singh <[email protected]>

* [Segment Replication] Self review

Signed-off-by: Suraj Singh <[email protected]>

* Address review comment

Signed-off-by: Suraj Singh <[email protected]>

* Address review comments & refactor

Signed-off-by: Suraj Singh <[email protected]>

* Comment

Signed-off-by: Suraj Singh <[email protected]>

* Fix unit test

Signed-off-by: Suraj Singh <[email protected]>

* Unit test to verify temporary files are not deleted from commits

Signed-off-by: Suraj Singh <[email protected]>

* Compilation error fix

Signed-off-by: Suraj Singh <[email protected]>

* Javadoc

Signed-off-by: Suraj Singh <[email protected]>

* Skip testScrollWithConcurrentIndexAndSearch with remote store

Signed-off-by: Suraj Singh <[email protected]>

---------

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

* [Segment Replication] Prevent store clean up on reader close action

Signed-off-by: Suraj Singh <[email protected]>

* [Segment Replication] Self review

Signed-off-by: Suraj Singh <[email protected]>

* Address review comment

Signed-off-by: Suraj Singh <[email protected]>

* Address review comments & refactor

Signed-off-by: Suraj Singh <[email protected]>

* Comment

Signed-off-by: Suraj Singh <[email protected]>

* Fix unit test

Signed-off-by: Suraj Singh <[email protected]>

* Unit test to verify temporary files are not deleted from commits

Signed-off-by: Suraj Singh <[email protected]>

* Compilation error fix

Signed-off-by: Suraj Singh <[email protected]>

* Javadoc

Signed-off-by: Suraj Singh <[email protected]>

* Skip testScrollWithConcurrentIndexAndSearch with remote store

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Suraj Singh <[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] [Segment Replication] replica fails after write some data
2 participants