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

Initial commit to support a search only replica for RW separation. #15410

Merged
merged 13 commits into from
Aug 30, 2024

Conversation

mch2
Copy link
Member

@mch2 mch2 commented Aug 26, 2024

Description

Initial commit for the search only replica for reader writer split.
This PR contains the following:

  1. Introduce searchOnly flag on ShardRouting.
  2. Add feature flag to enable/disable the feature.
  3. support both create and update APIs to toggle search replica count.
  4. Changes to exclude search replicas from primary eligibility.
  5. Changes to prevent any replicationOperation from routing to a search replica.
  6. Small change to return [s] instead of [r] for search replicas in /_cat/shards

Related Issues

Resolves #15368
related #15306

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

@mch2 mch2 changed the title Initial commit for search only replica. Initial commit for search only replica for RW separation. Aug 26, 2024
@mch2 mch2 force-pushed the split branch 2 times, most recently from a1b71a9 to e50f2dc Compare August 26, 2024 06:18
@mch2 mch2 changed the title Initial commit for search only replica for RW separation. Initial commit to support a search only replica for RW separation. Aug 26, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
Copy link
Contributor

❌ Gradle check result for c5d29a9: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

✅ Gradle check result for e182c9e: SUCCESS

@mch2
Copy link
Member Author

mch2 commented Aug 29, 2024

❌ Gradle check result for c5d29a9: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

#10820

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

This comment was marked as outdated.

Copy link
Contributor

✅ Gradle check result for 2207a10: SUCCESS

@andrross andrross merged commit 1e9fdb4 into opensearch-project:main Aug 30, 2024
34 checks passed
@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:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-15410-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1e9fdb452101476a24737cc1b3aa1fad15df8fca
# Push it to GitHub
git push --set-upstream origin backport/backport-15410-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

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

mch2 added a commit to mch2/OpenSearch that referenced this pull request Aug 30, 2024
…pensearch-project#15410)

* Initial commit for search only replica.
This PR contains the following:
1. Introduce searchOnly flag on ShardRouting.
2. Added feature flag to enable/disable the feature.
3. supports both create and update APIs to toggle search replica count.
4. Changes to exclude search replicas from primary eligibility.
5. Changes to prevent replicationOperations from routing to search replicas.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* add some missing feature flag checks

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Pr feedback from @andrross

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Add more unit tests for settings create and update

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Fix broken tests from setting rename

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Fix broken tests and add changelog entry

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* More PR feedback.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* add missing searchOnly property to initializeTargetRelocatingShard.

Without this search replicas will become regular replicas on relocation.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* test fixes

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* spotless

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

---------

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
(cherry picked from commit 1e9fdb4)
Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
@reta reta added v3.0.0 Issues and PRs related to version 3.0.0 v2.17.0 labels Aug 30, 2024
andrross pushed a commit that referenced this pull request Aug 30, 2024
… separation (#15535)

* Initial commit to support a search only replica for RW separation. (#15410)

* Initial commit for search only replica.
This PR contains the following:
1. Introduce searchOnly flag on ShardRouting.
2. Added feature flag to enable/disable the feature.
3. supports both create and update APIs to toggle search replica count.
4. Changes to exclude search replicas from primary eligibility.
5. Changes to prevent replicationOperations from routing to search replicas.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* add some missing feature flag checks

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Pr feedback from @andrross

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Add more unit tests for settings create and update

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Fix broken tests from setting rename

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Fix broken tests and add changelog entry

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* More PR feedback.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* add missing searchOnly property to initializeTargetRelocatingShard.

Without this search replicas will become regular replicas on relocation.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* test fixes

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* spotless

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

---------

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
(cherry picked from commit 1e9fdb4)
Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Update wire compatibility version to 2_17_0

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* change assertion for ff disabled from SettingsException to IllegalArgumentException

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

---------

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
@@ -357,7 +357,7 @@ private ShardRouting initializingShard(ShardRouting shardRouting, String current
@Override
public Decision canMoveAway(ShardRouting shardRouting, RoutingAllocation allocation) {
int outgoingRecoveries = 0;
if (!shardRouting.primary()) {
if (!shardRouting.primary() && !shardRouting.isSearchOnly()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry couldn't understand the context for this change. This is meant to move the replica and check if the outgoing recovery from it's peer primary is getting throttled. Wouldn't search only replica recover from the primary copy or the plan is to make recoveries totally independent? cc: @mch2

Copy link
Member Author

Choose a reason for hiding this comment

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

Err thanks for calling this out I'm not handling this correclty.

This was to resolve an NPE a few lines later when we only have search replicas and the primary has a node drop. In this case the NPE occurs right after this where we fetch the active primary as null and then invoke .currentNodeId on it. We should be setting outgoingRecoveries to 0 in this case instead.

To answer your question the plan is to have search replica recover directly from the remote store when its enabled but we need to still honor node-node with active primary.

akolarkunnu pushed a commit to akolarkunnu/OpenSearch that referenced this pull request Sep 10, 2024
…pensearch-project#15410)

* Initial commit for search only replica.
This PR contains the following:
1. Introduce searchOnly flag on ShardRouting.
2. Added feature flag to enable/disable the feature.
3. supports both create and update APIs to toggle search replica count.
4. Changes to exclude search replicas from primary eligibility.
5. Changes to prevent replicationOperations from routing to search replicas.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* add some missing feature flag checks

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Pr feedback from @andrross

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Add more unit tests for settings create and update

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Fix broken tests from setting rename

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* Fix broken tests and add changelog entry

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* More PR feedback.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* add missing searchOnly property to initializeTargetRelocatingShard.

Without this search replicas will become regular replicas on relocation.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* test fixes

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

* spotless

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>

---------

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
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 backport-failed v2.17.0 v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RW Separation] Introduce new search replica concept and changes to create API
4 participants