Search Replica Allocation and Recovery

[vinaykpud]

Description

• Modified the search replica allocation based on node attribute

• In this PR we restrict Search replica to assign to only nodes with searchonly:true attribute

• Also in this PR, we have made changes to treat search and regular replicas differently so unable to allocate one do not block the other: [RW Separation] Treat Regular and Search Replicas Separately to Prevent Allocation Blocking #17421

• I also fixed the recovery of search replica when there is a node left scenario: [RW Separation] Search replica recovery flow breaks when search shard allocated to new node after node drop #17334

Related Issues

Resolves #17422
Resolves #17421
Resolves #17334
Related to #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.

[github-actions]

❌ Gradle check result for f9c54c5: 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]

❕ Gradle check result for 2d5b977: 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.

[codecov]

Codecov Report

Attention: Patch coverage is 84.61538% with 6 lines in your changes missing coverage. Please review.

Project coverage is 72.50%. Comparing base (2ee8660) to head (7d5ab5a).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...java/org/opensearch/index/shard/StoreRecovery.java	84.00%	1 Missing and 3 partials ⚠️
...ava/org/opensearch/cluster/node/DiscoveryNode.java	0.00%	0 Missing and 1 partial ⚠️
...cation/decider/SearchReplicaAllocationDecider.java	90.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #17457      +/-   ##
============================================
+ Coverage     72.43%   72.50%   +0.07%     
- Complexity    65694    65769      +75     
============================================
  Files          5311     5311              
  Lines        304937   304932       -5     
  Branches      44226    44230       +4     
============================================
+ Hits         220872   221104     +232     
+ Misses        65912    65733     -179     
+ Partials      18153    18095      -58     

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

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

❌ Gradle check result for 69f13dd: 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]

✅ Gradle check result for 6e3be41: SUCCESS

[vinaykpud]

s have to be added explicitly. This espl gets tricky if there are concurrent modifications lets say there was a node removal and a new node addition concurrently. In such a case if the addition doesn't read the deleted state and overrides the node list with a new node, we might run into issues.

Thanks @Bukhtawar

From your response, it seems the concern is about keeping the filter updated while handling concurrent node additions and removals.

However, in this implementation, users can define a set of nodes—let’s say a "Searcher fleet"—by assigning a custom attribute in the YAML configuration. This means that as long as there are nodes matching this attribute, search replicas will be assigned accordingly. Essentially, we are designating nodes dynamically as part of the "Searcher fleet."

For example, if I have 5 nodes and configure 3 of them with a custom attribute:

node.attr.rackid: "rack2"

I can then make them part of the "Searcher fleet" using the following API:

curl -X PUT "http://localhost:9200/_cluster/settings" \
-H "Content-Type: application/json" \
-d '{
  "persistent": {
    "cluster.routing.allocation.search.replica.dedicated.include.rackid": "rack2"
  }
}'

With this setup, any node with rackid: rack2 will be considered a search-dedicated node. This means that regardless of how many nodes are added or removed from the cluster, as long as they have the matching attribute, they will be automatically recognized as part of the "Reader fleet."

This approach provides flexibility for users to use any attribute-based selection for search nodes.

That said, I’d like to understand if there are any additional concerns, such as potential performance implications, with this approach.

I think the filter logic should be used in cases where we actually need to add an exception(exclusion) or override the allocation logic on top of a generic node roles.

Do you mean we are incorrectly utilizing the inclusion filter here, or is this approach misaligned with its intended purpose? Do you see any limitations or unintended behavior in using it this way?

Also please find my comment on the approach here : #17422 (comment)

[github-actions]

❌ Gradle check result for 858f984: 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]

❌ Gradle check result for 8f94e31: 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]

❌ Gradle check result for 66085c5: 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]

❌ Gradle check result for 66085c5: 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]

❌ Gradle check result for 5836b20: 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]

❕ Gradle check result for 7d5ab5a: UNSTABLE

• TEST FAILURES:

      1 org.opensearch.cluster.MinimumClusterManagerNodesIT.testThreeNodesNoClusterManagerBlock

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

[mch2]

why?

[vinaykpud]

there is no real reason why search replica should come later. I am making an assumption here to allocate Replica first then allocate search replica. So that allocation is consistent.

Allocation works like this:
Assume these are share shards:

P1, R1, R1, S1, S1

First allocates primary to some node.

In the next round it starts allocating replicas:

R1, R1, S1, S1

Above logic will help in sorting Replicas first and Search Replicas next.
So allocator will allocate one copy of replica ie R1, then move to allocate copy of search replica ie S1.

So in next iteration it will be

R1, S1

[mch2]

can we delegate to a separate function from internalRecoverFromStore in the SR case rather than updating this?

[vinaykpud]

I dint get this, do you mean a separate internalRecoverFromStore method just for search replica?

[vinaykpud]

Also "updating this" means this method or just this line?