Conversation
Ring.Get() when a large number of instances are in a state for which the extended replication set is requestedSigned-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
9f1c505 to
7d33cd7
Compare
Signed-off-by: Marco Pracucci <marco@pracucci.com>
56quarters
approved these changes
Jan 27, 2026
| t.Run("zero capacity buffer switches to map immediately on first add", func(t *testing.T) { | ||
| t.Parallel() | ||
| buf := make([]string, 0) | ||
| s := newStringSet(buf) |
Contributor
There was a problem hiding this comment.
Maybe assert that setMap is nil before any adds?
Signed-off-by: Marco Pracucci <marco@pracucci.com>
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.
What this PR does:
The
Ring.Get()is very inefficient when a large number of instances are in a state for which the extended replication set is requested. This is not a new issue, and there was a previous optimization attempt in #672 by @56quarters. Unfortunately the issue is not fixed yet and we can see it when the magnitude of instances increases.In this PR I propose an optimization based on the fact that looking up small maps is slower than just searching for corresponding items in slices (maps become fasters when the number of unique items increases, but for few items slices are just faster).
The result of this optimization is not huge, but I think it's still a step forward. It's still very slow when we have a large number of instances in a state for which the extended replication set is requested, compared to the case all instances are ACTIVE.
Which issue(s) this PR fixes:
N/A
Checklist
Note
Improves performance of
Ring.Get()when many instances extend the replica set, especially under zone-awareness.distinctHostsslice/map usage with a hybridstringSet(util.go) to reduce lookups/allocations; adds comprehensive teststotalHostsPerZone,examinedHostsPerZone,foundHostsPerZone); updatescanStopLooking()signature accordinglyzoneIndexresolution with validation and panics on misuse if zone-awareness disabledstringSet.len()/contains()/add()and new stop conditionBenchmarkRing_Get_OneZoneLeaving(largerinstances, more zones, and both with/without one zone leaving) with deterministic assertionsNet effect: substantial reductions in
Ring.Get()time in pathological scenarios; no API changes.Written by Cursor Bugbot for commit 9f1c505. This will update automatically on new commits. Configure here.