Skip to content

Conversation

@sachinpkale
Copy link
Member

@sachinpkale sachinpkale commented Jul 6, 2023

Description

  • RemoteStoreRefreshListener is one of the refresh listeners that uploads segments to remote segment store.
  • Currently, it reads segmentInfosSnapshot from InternalEngine and adds files referenced by segments_N file.
  • This is a defensive approach that was taken so that we do not lose track of any segment file but this can create issue in scenarios such as forcemerge, where segmentInfosSnapshot is referencing to segments_N file but the files referenced by segments_N are merged away.
  • In this PR, we use segment files referenced by segmentInfosSnapshot alone.
  • Added integ tests around the forcemerge use case. Existing integ tests should pass.

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:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      2 org.opensearch.common.util.concurrent.QueueResizableOpenSearchThreadPoolExecutorTests.classMethod
      1 org.opensearch.common.util.concurrent.QueueResizableOpenSearchThreadPoolExecutorTests.testResizeQueueDown

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      2 org.opensearch.index.shard.RemoteStoreRefreshListenerTests.classMethod
      2 org.opensearch.cluster.service.MasterServiceTests.classMethod
      1 org.opensearch.indices.replication.SegmentReplicationIT.testPitCreatedOnReplica
      1 org.opensearch.index.shard.RemoteStoreRefreshListenerTests.testRefreshAfterCommit
      1 org.opensearch.cluster.service.MasterServiceTests.testThrottlingForMultipleTaskTypes
      1 org.opensearch.client.ReindexIT.testReindexTask

@sachinpkale sachinpkale marked this pull request as draft July 10, 2023 19:15
@sachinpkale
Copy link
Member Author

Taking a look at test failures in org.opensearch.index.shard.RemoteStoreRefreshListenerTests

@sachinpkale
Copy link
Member Author

New integ tests are executed 180+ times on local to avoid checking in flaky tests.

Copy link
Member

@ashking94 ashking94 left a comment

Choose a reason for hiding this comment

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

LGTM.

@sachinpkale sachinpkale force-pushed the use-segment-infos-snapshot branch from b8c3aa8 to 3590576 Compare July 17, 2023 18:33
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testPressureServiceStats
      1 org.opensearch.remotestore.RemoteStoreIT.testStaleCommitDeletionWithInvokeFlush

@sachinpkale sachinpkale force-pushed the use-segment-infos-snapshot branch from 3590576 to cec103f Compare July 18, 2023 09:03
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Sachin Kale added 7 commits July 18, 2023 15:27
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: Sachin Kale <[email protected]>
@sachinpkale sachinpkale force-pushed the use-segment-infos-snapshot branch from cec103f to 373976d Compare July 18, 2023 09:57
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.RemoteStoreIT.testStaleCommitDeletionWithInvokeFlush
      1 org.opensearch.index.IndexSettingsTests.testDefaultSearchPipelineWithoutFeatureFlag

@sachinpkale sachinpkale merged commit 230a06d into opensearch-project:main Jul 18, 2023
@sachinpkale sachinpkale added the backport 2.x Backport to 2.x branch label Jul 18, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-8487-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 230a06daee9c8484e38d6aad867a98c3c10c2485
# Push it to GitHub
git push --set-upstream origin backport/backport-8487-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-8487-to-2.x.

sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Jul 19, 2023
…pensearch-project#8487)

* Use files from segmentInfosSnapshot in upload

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Jul 19, 2023
…pensearch-project#8487)

* Use files from segmentInfosSnapshot in upload

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Jul 19, 2023
…pensearch-project#8487)

* Use files from segmentInfosSnapshot in upload

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
sachinpkale added a commit that referenced this pull request Jul 19, 2023
…8487) (#8772)

* Use files from segmentInfosSnapshot in upload

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
baba-devv pushed a commit to baba-devv/OpenSearch that referenced this pull request Jul 29, 2023
…pensearch-project#8487)

* Use files from segmentInfosSnapshot in upload

---------

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

* Use files from segmentInfosSnapshot in upload

---------

Signed-off-by: Sachin Kale <[email protected]>
Co-authored-by: Sachin Kale <[email protected]>
Signed-off-by: Kaushal Kumar <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…pensearch-project#8487)

* Use files from segmentInfosSnapshot in upload

---------

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.

3 participants