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] Add PIT/Scroll compatibility with Segment Replication #6644

Closed
wants to merge 3 commits into from

Conversation

mch2
Copy link
Member

@mch2 mch2 commented Mar 13, 2023

Description

This change makes updates to make PIT/Scroll queries compatible with Segment Replication. It does this by refcounting files when a new reader is created, and discarding those files after a reader is closed.

Issues Resolved

#6519

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

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Mar 13, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@ad823b6). Click here to learn what that means.
The diff coverage is 84.44%.

❗ Current head 7a60f4e differs from pull request most recent head 939ff3a. Consider uploading reports for the commit 939ff3a to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main    #6644   +/-   ##
=======================================
  Coverage        ?   70.73%           
  Complexity      ?    59206           
=======================================
  Files           ?     4810           
  Lines           ?   283497           
  Branches        ?    40873           
=======================================
  Hits            ?   200545           
  Misses          ?    66459           
  Partials        ?    16493           
Impacted Files Coverage Δ
...rc/main/java/org/opensearch/index/store/Store.java 80.05% <72.72%> (ø)
.../opensearch/index/engine/NRTReplicationEngine.java 76.25% <80.00%> (ø)
...org/opensearch/index/store/ReplicaFileDeleter.java 87.50% <87.50%> (ø)
...arch/index/engine/NRTReplicationReaderManager.java 90.90% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.indices.replication.SegmentReplicationIT.testPitCreatedOnReplica

This change makes updates to make PIT/Scroll queries compatibile with Segment Replication.
It does this by refcounting files when a new reader is created, and discarding those files after
a reader is closed.

Signed-off-by: Marc Handalian <handalm@amazon.com>
Signed-off-by: Marc Handalian <handalm@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

…of current store state.

Signed-off-by: Marc Handalian <handalm@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@dreamer-89
Copy link
Member

Closing this PR in favour of #6765 (author is on leave)

CC @anasalkouz

@dreamer-89 dreamer-89 closed this Mar 20, 2023
dreamer-89 added a commit that referenced this pull request Apr 5, 2023
…cation #6644 (#6765)

* Segment Replication - PIT/Scroll compatibility.

This change makes updates to make PIT/Scroll queries compatibile with Segment Replication.
It does this by refcounting files when a new reader is created, and discarding those files after
a reader is closed.

Signed-off-by: Marc Handalian <handalm@amazon.com>

* Fix broken test.

Signed-off-by: Marc Handalian <handalm@amazon.com>

* Fix test bug with PIT where snapshotted segments are queried instead of current store state.

Signed-off-by: Marc Handalian <handalm@amazon.com>

* Address review comments and prevent temp file deletion during reader close

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Fix precommit failure

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Use last committed segment infos reference from replication engine

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Clean up and prevent incref on segment info file copied from primary

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Fix failing test

Signed-off-by: Suraj Singh <surajrider@gmail.com>

---------

Signed-off-by: Marc Handalian <handalm@amazon.com>
Signed-off-by: Suraj Singh <surajrider@gmail.com>
Co-authored-by: Marc Handalian <handalm@amazon.com>
dreamer-89 added a commit to dreamer-89/OpenSearch that referenced this pull request Apr 5, 2023
…cation opensearch-project#6644 (opensearch-project#6765)

* Segment Replication - PIT/Scroll compatibility.

This change makes updates to make PIT/Scroll queries compatibile with Segment Replication.
It does this by refcounting files when a new reader is created, and discarding those files after
a reader is closed.

Signed-off-by: Marc Handalian <handalm@amazon.com>

* Fix broken test.

Signed-off-by: Marc Handalian <handalm@amazon.com>

* Fix test bug with PIT where snapshotted segments are queried instead of current store state.

Signed-off-by: Marc Handalian <handalm@amazon.com>

* Address review comments and prevent temp file deletion during reader close

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Fix precommit failure

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Use last committed segment infos reference from replication engine

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Clean up and prevent incref on segment info file copied from primary

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Fix failing test

Signed-off-by: Suraj Singh <surajrider@gmail.com>

---------

Signed-off-by: Marc Handalian <handalm@amazon.com>
Signed-off-by: Suraj Singh <surajrider@gmail.com>
Co-authored-by: Marc Handalian <handalm@amazon.com>
dreamer-89 added a commit that referenced this pull request Apr 5, 2023
…cation (#7010)

* [Segment Replication] Add PIT/Scroll compatibility with Segment Replication #6644 (#6765)

* Segment Replication - PIT/Scroll compatibility.

This change makes updates to make PIT/Scroll queries compatibile with Segment Replication.
It does this by refcounting files when a new reader is created, and discarding those files after
a reader is closed.

Signed-off-by: Marc Handalian <handalm@amazon.com>

* Fix broken test.

Signed-off-by: Marc Handalian <handalm@amazon.com>

* Fix test bug with PIT where snapshotted segments are queried instead of current store state.

Signed-off-by: Marc Handalian <handalm@amazon.com>

* Address review comments and prevent temp file deletion during reader close

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Fix precommit failure

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Use last committed segment infos reference from replication engine

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Clean up and prevent incref on segment info file copied from primary

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Fix failing test

Signed-off-by: Suraj Singh <surajrider@gmail.com>

---------

Signed-off-by: Marc Handalian <handalm@amazon.com>
Signed-off-by: Suraj Singh <surajrider@gmail.com>
Co-authored-by: Marc Handalian <handalm@amazon.com>

* Add param definition causing precommit failure

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Remove unnecessary override annotation

Signed-off-by: Suraj Singh <surajrider@gmail.com>

---------

Signed-off-by: Marc Handalian <handalm@amazon.com>
Signed-off-by: Suraj Singh <surajrider@gmail.com>
Co-authored-by: Marc Handalian <handalm@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants