Fix skewed search replica request distribution when write replicas exist#22528
Open
mars2cbh wants to merge 2 commits into
Open
Fix skewed search replica request distribution when write replicas exist#22528mars2cbh wants to merge 2 commits into
mars2cbh wants to merge 2 commits into
Conversation
Contributor
PR Reviewer Guide 🔍(Review updated until commit 0243728)Here are some key observations to aid the review process:
|
mars2cbh
force-pushed
the
fix-search-replica-rotation-skew
branch
from
July 21, 2026 13:45
8eb6487 to
6925ae6
Compare
Contributor
|
Persistent review updated to latest commit 6925ae6 |
Contributor
|
❌ Gradle check result for 6925ae6: 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? |
filterAndOrderShards rotated the full replicas list before filtering, so rotation offsets landing on filtered-out write replicas collapsed onto the same search replica, giving it a fixed 2/(n+1) share of requests in reader/writer separated clusters. Filter first, then rotate only the matching active replicas so the round-robin stays uniform. Signed-off-by: David Choi <themars@gmail.com>
mars2cbh
force-pushed
the
fix-search-replica-rotation-skew
branch
from
July 21, 2026 14:22
6925ae6 to
bea0a45
Compare
Contributor
|
Persistent review updated to latest commit bea0a45 |
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22528 +/- ##
============================================
- Coverage 73.48% 73.44% -0.04%
+ Complexity 76460 76451 -9
============================================
Files 6104 6104
Lines 346582 346586 +4
Branches 49879 49879
============================================
- Hits 254675 254543 -132
- Misses 71664 71782 +118
- Partials 20243 20261 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Persistent review updated to latest commit 0243728 |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
IndexShardRoutingTable#filterAndOrderShards(used bysearchReplicaActiveInitializingShardIt()/ the_search_replicapreference path) rotated the fullreplicaslist before applying the search-only filter.RotationShardShuffleris a pure round-robin rotation, so rotation offsets that land on a filtered-out write replica collapse onto the same head shard as the adjacent offset. In a reader/writer separated cluster with one write replica and n search replicas, the search replica that precedes the write replica in list order becomes the iterator head for two of the n+1 rotation offsets, permanently receiving a 2/(n+1) share of search traffic (exactly 2x the others). Withreplicas = [W, S1, S2, S3]:This change applies the filter first and rotates only the filtered active list, restoring a uniform round-robin. Active shards still come before initializing ones.
replicaActiveInitializingShardIt()(filter accepts everything, so no offset collapse) and the zero-write-replica case keep their previous uniform behavior.Added tests:
testSearchReplicaShardIteratorEvenDistribution— randomizes 0–3 write replicas, 2–5 search replicas, and list order, then asserts each search replica is the iterator head an exactly equal number of times. Fails on the old implementation whenever a write replica is present.testSearchReplicaShardIteratorReturnsActiveBeforeInitializing— asserts only search replicas are returned, active before initializing.Related Issues
Resolves #22527
Check List
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.