Skip to content

Conversation

@KunjueYu
Copy link

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.

@opensearch-trigger-bot opensearch-trigger-bot bot removed the stalled Issues that have stalled label Apr 27, 2024
@KunjueYu KunjueYu requested a review from Bukhtawar April 28, 2024 08:57
@linuxpi
Copy link
Contributor

linuxpi commented May 2, 2024

[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
Copy link
Author

KunjueYu commented May 6, 2024

[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
Copy link
Contributor

linuxpi commented May 8, 2024

[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

Copy link
Contributor

@gaobinlong gaobinlong left a comment

Choose a reason for hiding this comment

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

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.

@opensearch-trigger-bot
Copy link
Contributor

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

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

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

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Jul 29, 2024
@linuxpi
Copy link
Contributor

linuxpi commented Aug 1, 2024

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

@vikasvb90 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 opensearch-trigger-bot bot removed the stalled Issues that have stalled label Aug 11, 2024
@github-actions github-actions bot added Indexing:Replication Issues and PRs related to core replication framework eg segrep labels Feb 10, 2025
@KunjueYu
Copy link
Author

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.

I think the test cases in NodeVersionAllocationDeciderTests is enough to cover this case.
Changlog is added now.

@KunjueYu
Copy link
Author

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

Sorry for the lag. I have checked all the comments and added changlog.

@KunjueYu KunjueYu requested a review from gaobinlong February 10, 2025 03:50
@github-actions
Copy link
Contributor

❕ Gradle check result for 964c881: UNSTABLE

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Cluster Manager Indexing:Replication Issues and PRs related to core replication framework eg segrep Storage:Remote Storage Issues and PRs relating to data and metadata storage

Projects

Status: No status
Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

[BUG] NodeVersionAllocationDecider should get Replication Type from index meta instead of node settings

6 participants