Route custom preferences to search replicas

[suraj-soni-glean]

Description

When strict search-replica routing is enabled (cluster.routing.search_replica.strict, default true) and an index has search replicas, search traffic is expected to stay isolated on those search replicas, keeping it off the primary/writer copies.

Today that isolation only holds when the request has no preference. OperationRouting#searchShards forced an empty preference onto _search_replica, but a caller-supplied custom preference (a non-_ key, typically used for session-consistent routing) fell through to activeInitializingShardsIt(...), which spreads across primaries and writer replicas. So a custom preference silently disabled the strict search-replica isolation.

This change makes the search-replica target the default whenever strict routing applies and the index has search replicas, covering both the empty-preference and custom-preference cases:

• The decision is computed once in searchShards (defaultToSearchReplicas = isStrictSearchOnlyShardRouting && numberOfSearchOnlyReplicas > 0) and passed into preferenceActiveShardIterator.
• For a custom preference, routing is confined to search replicas while preserving preference-based consistency — the preference hash seeds search-replica selection via a new IndexShardRoutingTable#searchReplicaActiveInitializingShardIt(int seed), so the same key resolves to the same search replica across requests.
• Explicit _ preferences (e.g. _primary, _only_nodes) still override, since they are handled before the default kicks in.
• Realtime GET (getShards) is unaffected — it passes false, so it can still reach the primary/writer replicas.

Behavior note (weighted routing): when a custom preference is now confined to search replicas, weighted/AZ routing is not applied to that selection. This matches the existing behavior of the explicit _search_replica preference (which also bypasses weighted routing), so it is internally consistent; but for clusters relying on weighted routing, custom-preference searches against search replicas will not honor those weights.

Related Issues

#22395

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]

PR Reviewer Guide 🔍

(Review updated until commit 5a5085c)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Weighted routing bypassed When defaultToSearchReplicas is true and a custom (non-_) preference is provided, the code returns searchReplicaActiveInitializingShardIt(routingHash) before the weighted routing branch. This silently disables weighted/AZ routing for custom-preference searches on indices with search replicas, even in clusters where strict weighted routing is expected to be enforced. This is called out in the PR description as intentional, but it can cause AZ affinity/traffic-shaping to be violated without operator awareness. Consider whether strict weighted routing should still apply (or at least be logged) when confining to search replicas. if (defaultToSearchReplicas) { return indexShard.searchReplicaActiveInitializingShardIt(routingHash); }

[github-actions]

Persistent review updated to latest commit be08d05

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 5a5085c

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Fallback when no search replicas available When preference is null/empty and defaultToSearchReplicas is true, the code returns a search-replica-only iterator but uses a random seed via shuffler.nextSeed(). This preserves prior behavior for empty preferences, but you should also verify that returning an empty iterator (when there are zero active search replicas but the flag was set) is handled by callers, since the previous code fell back through to shardRoutings. Consider falling back to the normal routing when no active search replicas exist to avoid unnecessarily unassigned iterators. server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java [370-374] if (preference == null || preference.isEmpty()) { if (defaultToSearchReplicas) { - return indexShard.searchReplicaActiveInitializingShardIt(); + ShardIterator searchReplicaIt = indexShard.searchReplicaActiveInitializingShardIt(); + if (searchReplicaIt.size() > 0) { + return searchReplicaIt; + } } return shardRoutings(indexShard, nodes, collectorService, nodeCounts, weightedRoutingMetadata); } Suggestion importance[1-10]: 3 __ Why: The suggestion proposes a behavioral change (fallback to normal routing when no search replicas exist), but this may contradict the intent of "strict" search-only routing, which is meant to strictly confine traffic to search replicas. The suggestion is speculative and not clearly a bug.	Low
Possible issue	Handle empty search-replica iterator fallback Similar to the empty-preference case, when a custom preference is provided and defaultToSearchReplicas is true, this returns an iterator that may be empty if no active search replicas exist. The prior code would have fallen through to weighted/standard routing. Consider falling back to the standard path when the search-replica iterator is empty to prevent search requests from failing to be routed. server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java [445-447] routingHash = 31 * routingHash + indexShard.shardId.hashCode(); if (defaultToSearchReplicas) { - return indexShard.searchReplicaActiveInitializingShardIt(routingHash); + ShardIterator searchReplicaIt = indexShard.searchReplicaActiveInitializingShardIt(routingHash); + if (searchReplicaIt.size() > 0) { + return searchReplicaIt; + } } Suggestion importance[1-10]: 3 __ Why: Similar to suggestion 1, falling back to non-search-replica routing may violate strict search-only routing semantics. The suggestion identifies a potential edge case but the proposed behavior change is not clearly correct.	Low

---

Previous suggestions

Suggestions up to commit e4f6a6d

Category	Suggestion	Impact
General	Verify custom preference short-circuit ordering When defaultToSearchReplicas is true and a custom preference is provided, the method short-circuits and returns the search-replica iterator, but this bypasses the check further above that handles cases where the preference starts with '_' (like shards:, only_nodes:, etc.) that may be combined with a custom key using '|'. Ensure that only truly custom (non-'' prefixed) preferences take this path; verify the ordering with preference.charAt(0) == '' handling above is preserved so explicit shard-type preferences still override search-replica routing. server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java [446-448] +routingHash = 31 * routingHash + indexShard.shardId.hashCode(); +if (defaultToSearchReplicas) { + return indexShard.searchReplicaActiveInitializingShardIt(routingHash); +} +if (WeightedRoutingUtils.shouldPerformStrictWeightedRouting( + isStrictWeightedShardRouting, + ignoreWeightedRouting, + weightedRoutingMetadata - Suggestion importance[1-10]: 3 __ Why: The suggestion is essentially a "verify" note without proposing a concrete change; the improved_code is identical to the existing_code. The concern is also partly addressed by the earlier preference.charAt(0) == '_' branch, so the impact is minimal.	Low

Suggestions up to commit be08d05

Category	Suggestion	Impact
Possible issue	Fix determinism test comparing consumed iterators The firstOrder list is built by iterating groupIterator after it has already been fully consumed by the preceding for loop, so firstOrder will be empty and the equality assertion becomes trivially true (empty == empty), defeating the intent of verifying deterministic ordering. Capture the first iterator's ordering before consuming it, or re-fetch a fresh iterator for the comparison. server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java [1307-1313] // The same custom preference should deterministically resolve to the same ordering across calls. +GroupShardsIterator<ShardIterator> firstIterator = opRouting.searchShards(state, indexNames, null, customPreference); GroupShardsIterator<ShardIterator> repeatIterator = opRouting.searchShards(state, indexNames, null, customPreference); List<ShardRouting> firstOrder = new ArrayList<>(); -groupIterator.iterator().next().forEach(firstOrder::add); +firstIterator.iterator().next().forEach(firstOrder::add); List<ShardRouting> repeatOrder = new ArrayList<>(); repeatIterator.iterator().next().forEach(repeatOrder::add); +assertFalse("Determinism check requires non-empty ordering", firstOrder.isEmpty()); assertEquals("Custom preference routing must be consistent across requests", firstOrder, repeatOrder); Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that groupIterator was already consumed by the preceding for loop (which used enhanced-for iteration on the ShardIterator), so firstOrder may end up empty, making the determinism assertion meaningless. This is a legitimate test correctness issue.	Medium

[github-actions]

Persistent review updated to latest commit e4f6a6d

[github-actions]

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

Persistent review updated to latest commit 5a5085c

[github-actions]

✅ Gradle check result for 5a5085c: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.48%. Comparing base (b99229e) to head (5a5085c).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22394      +/-   ##
============================================
- Coverage     73.52%   73.48%   -0.05%     
+ Complexity    76254    76224      -30     
============================================
  Files          6076     6076              
  Lines        345790   345794       +4     
  Branches      49762    49763       +1     
============================================
- Hits         254236   254100     -136     
- Misses        71371    71485     +114     
- Partials      20183    20209      +26     

☔ View full report in Codecov by Harness.
📢 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.