Fix getting replication type in NodeVersionAllocationDecider #12811

KunjueYu · 2024-03-21T06:05:13Z

Description

This PR fix incorrect way of getting replication type from node settings in org.opensearch.cluster.routing.allocation.decider.NodeVersionAllocationDecider. Instead, we should get the replication type from index meta data. Besides, I add a test which verifies that the primary shard can be allocated to a node with higher version when replication type is document.

Related Issues

Resolves #12744

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)
Public documentation issue/PR created

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 · 2024-03-21T06:18:59Z

❌ Gradle check result for d1b71d2: 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?

github-actions · 2024-03-21T06:27:38Z

Compatibility status:

Checks if related components are compatible with change 5c30a74

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/sql.git]

Signed-off-by: ludongyu <heidongjidian@163.com>

github-actions · 2024-03-21T07:21:24Z

✅ Gradle check result for 47434ca: SUCCESS

codecov · 2024-03-21T07:22:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.36%. Comparing base (b15cb0c) to head (5c30a74).
Report is 627 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #12811      +/-   ##
============================================
- Coverage     71.42%   71.36%   -0.06%     
- Complexity    59978    60202     +224     
============================================
  Files          4985     5011      +26     
  Lines        282275   283557    +1282     
  Branches      40946    41089     +143     
============================================
+ Hits         201603   202373     +770     
- Misses        63999    64407     +408     
- Partials      16673    16777     +104

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-03-21T07:55:37Z

❕ Gradle check result for 5c30a74: UNSTABLE

TEST FAILURES:

      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testRestartPrimary_NoReplicas

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Bukhtawar

Thanks for fixing this

kkmr · 2024-03-26T18:40:57Z

Thanks for submitting the PR. I have a couple of questions:

What is the expected behaviour? Is it that we should be able to move replicas between "SEgment replication" nodes to "Document replication"?
Is there an forward-backward compatiability issues to deploy this fix in production?

KunjueYu · 2024-03-27T09:24:34Z

Thanks for submitting the PR. I have a couple of questions:

What is the expected behaviour? Is it that we should be able to move replicas between "SEgment replication" nodes to "Document replication"?

Is there an forward-backward compatiability issues to deploy this fix in production?

the expected behavior is that the primary shard of segment-replication can not be allocated to a node with higher version than the node which the replica shard is on, while the primary shard of document-replication can be allocated to a node with higher version.
As the replication type setting is a index-scope setting, there is not any "Segment replication" nodes or "Document replication" node. There is only "Segment replication" index or "Document replication" index.
I don't think there is any forward-backward compatibility issues to deploy this fix in production.

opensearch-trigger-bot · 2024-04-26T15:20:46Z

This PR is stalled because it has been open for 30 days with no activity.

linuxpi · 2024-05-02T15:41:03Z

[Storage Triage - attendees 1 2 3 4 5 6 7 8 9 10 11 12 13]

@KunjueYu Thanks for opening this PR. Please add a release target label and double check if this is actually related to Remote Store, else remove the label

KunjueYu · 2024-05-06T05:35:03Z

[Storage Triage - attendees 1 2 3 4 5 6 7 8 9 10 11 12 13]

@KunjueYu Thanks for opening this PR. Please add a release target label and double check if this is actually related to Remote Store, else remove the label

I don't have the permission to edit the labels of this PR. This PR is not related to Remote Store, so the label should be removed. I am not familiar with choosing the release target label, maybe label v2.13.1 can be added ?

linuxpi · 2024-05-08T10:04:06Z

[Storage Triage - attendees 1 2 3 4 5 6 7 8 9 10 11 12 13]
@KunjueYu Thanks for opening this PR. Please add a release target label and double check if this is actually related to Remote Store, else remove the label

I don't have the permission to edit the labels of this PR. This PR is not related to Remote Store, so the label should be removed. I am not familiar with choosing the release target label, maybe label v2.13.1 can be added ?

Removed the storage label. If you are targeting v2.15 release(next release), we can add the 2.15 label

gaobinlong

I think we need to add some integration test cases to make sure this allocation decider works in a real cluster, we may add more test cases to ClusterAllocationExplainIT or SegmentReplicationAllocationIT.
By the way, changelog is needed for this change.

gaobinlong · 2024-05-13T02:58:36Z

...ain/java/org/opensearch/cluster/routing/allocation/decider/NodeVersionAllocationDecider.java


    @Override
    public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
        if (shardRouting.primary()) {
+            IndexMetadata indexMd = allocation.metadata().getIndexSafe(shardRouting.index());
+            final ReplicationType replicationType = IndexMetadata.INDEX_REPLICATION_TYPE_SETTING.get(indexMd.getSettings());
            if (replicationType == ReplicationType.SEGMENT) {


Could you help to check why the existing unit tests don't cover this case? I see there're some test cases may cover that but they worked fine, like testRebalanceDoesNotAllocatePrimaryOnHigherVersionNodesSegrepEnabled and testRebalanceDoesNotAllocatePrimaryAndReplicasOnDifferentVersionNodes.

gaobinlong · 2024-05-13T03:04:36Z

...c/test/java/org/opensearch/cluster/routing/allocation/NodeVersionAllocationDeciderTests.java

@@ -671,6 +668,98 @@ public void testRebalanceDoesNotAllocatePrimaryOnHigherVersionNodesSegrepEnabled
        );
    }

+    public void testCanAllocatePrimaryOnHigherVersionNodesDocRepEnabled() {


Since SETTING_REPLICATION_TYPE setting defaults to document replication, so could you check if this new test case is duplicated with the existing test cases?

opensearch-trigger-bot · 2024-06-12T15:20:56Z

This PR is stalled because it has been open for 30 days with no activity.

opensearch-trigger-bot · 2024-07-29T15:22:29Z

This PR is stalled because it has been open for 30 days with no activity.

linuxpi · 2024-08-01T11:39:36Z

@KunjueYu can you check @gaobinlong 's comments and help take the PR to closure

github-actions bot added bug Something isn't working Cluster Manager Indexing:Replication Issues and PRs related to core replication framework eg segrep Storage:Remote labels Mar 21, 2024

KunjueYu added 2 commits March 21, 2024 14:59

Fix getting replication type in NodeVersionAllocationDecider

23ce2e1

Signed-off-by: ludongyu <heidongjidian@163.com>

fix format by spotless

5c30a74

Signed-off-by: ludongyu <heidongjidian@163.com>

KunjueYu force-pushed the fix-NodeVersionAllocationDecider branch from 47434ca to 5c30a74 Compare March 21, 2024 07:00

Bukhtawar reviewed Mar 21, 2024

View reviewed changes

opensearch-trigger-bot bot added stalled Issues that have stalled and removed stalled Issues that have stalled labels Apr 26, 2024

KunjueYu requested a review from Bukhtawar April 28, 2024 08:57

linuxpi removed the Storage:Remote label May 8, 2024

gaobinlong reviewed May 13, 2024

View reviewed changes

opensearch-trigger-bot bot added stalled Issues that have stalled and removed stalled Issues that have stalled labels Jun 12, 2024

opensearch-trigger-bot bot added the stalled Issues that have stalled label Jul 29, 2024

vikasvb90 added Storage Issues and PRs relating to data and metadata storage Storage:Remote and removed Indexing:Replication Issues and PRs related to core replication framework eg segrep labels Aug 5, 2024

opensearch-trigger-bot bot removed the stalled Issues that have stalled label Aug 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix getting replication type in NodeVersionAllocationDecider #12811

Fix getting replication type in NodeVersionAllocationDecider #12811

KunjueYu commented Mar 21, 2024

github-actions bot commented Mar 21, 2024

github-actions bot commented Mar 21, 2024 •

edited

Loading

github-actions bot commented Mar 21, 2024

codecov bot commented Mar 21, 2024 •

edited

Loading

github-actions bot commented Mar 21, 2024

Bukhtawar left a comment

kkmr commented Mar 26, 2024

KunjueYu commented Mar 27, 2024

opensearch-trigger-bot bot commented Apr 26, 2024

linuxpi commented May 2, 2024 •

edited

Loading

KunjueYu commented May 6, 2024

linuxpi commented May 8, 2024

gaobinlong left a comment

gaobinlong May 13, 2024

gaobinlong May 13, 2024

opensearch-trigger-bot bot commented Jun 12, 2024

opensearch-trigger-bot bot commented Jul 29, 2024

linuxpi commented Aug 1, 2024

Fix getting replication type in NodeVersionAllocationDecider #12811

Are you sure you want to change the base?

Fix getting replication type in NodeVersionAllocationDecider #12811

Conversation

KunjueYu commented Mar 21, 2024

Description

Related Issues

Check List

github-actions bot commented Mar 21, 2024

github-actions bot commented Mar 21, 2024 • edited Loading

Compatibility status:

Incompatible components

Skipped components

Compatible components

github-actions bot commented Mar 21, 2024

codecov bot commented Mar 21, 2024 • edited Loading

Codecov Report

github-actions bot commented Mar 21, 2024

Bukhtawar left a comment

Choose a reason for hiding this comment

kkmr commented Mar 26, 2024

KunjueYu commented Mar 27, 2024

opensearch-trigger-bot bot commented Apr 26, 2024

linuxpi commented May 2, 2024 • edited Loading

KunjueYu commented May 6, 2024

linuxpi commented May 8, 2024

gaobinlong left a comment

Choose a reason for hiding this comment

gaobinlong May 13, 2024

Choose a reason for hiding this comment

gaobinlong May 13, 2024

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Jun 12, 2024

opensearch-trigger-bot bot commented Jul 29, 2024

linuxpi commented Aug 1, 2024

github-actions bot commented Mar 21, 2024 •

edited

Loading

codecov bot commented Mar 21, 2024 •

edited

Loading

linuxpi commented May 2, 2024 •

edited

Loading