kvclient: cache the sorted list of replicas #114033
Draft
Conversation
This file contains 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
|
This change is |
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
2 times, most recently
from
6adcf16 to
278925c
Compare
|
@nvanbenschoten FYI - still a work in progress, but I think the change here will be pretty straighforward to get working well. |
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
10 times, most recently
from
e558e26 to
5e459c1
Compare
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
4 times, most recently
from
745c274 to
4925b69
Compare
|
Running against one node (after writing ~2 minutes) Small but measurable performance. With the additional changes to include the health, it removes a call to NodeDialer.ConnHealth (0.7%) and therefore should be an even larger improvement AND have better behavior. Calls to DistSender.sendToReplicas: Old: Calls to OptimizeReplicaOrder: Old: |
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
from
4925b69 to
2f30378
Compare
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
from
2f30378 to
b14615f
Compare
|
@nvanbenschoten or @arulajmani if you get a chance to review this (or comment on just the first commit which is in #114429) I can try and get this merged this week which will unblock adding checks as the default setting. |
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
from
b14615f to
edb27b7
Compare
andrewbaptist
removed request for
a team,
srosenberg,
DarrylWong,
rharding6373,
sumeerbhola,
dt,
jayshrivastava and
xinhaoz
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
from
12ab827 to
debe389
Compare
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
2 times, most recently
from
8020503 to
78fcf5c
Compare
Remove the EvictByKey from the interface. This replaces everything in the cache, and it is better to update entries through an EvictionToken. Epic: none Release note: None
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
2 times, most recently
from
98be0b0 to
19d6f86
Compare
The internal CacheEntry inside the RangeCache should not be publicly exposed. This commit changes GetCached to return the public RangeInfo instead. This is a test only menthod. Epic: none Release note: None
CacheEntry is not intended to be exposed externally. This commit makes it a private type in the RangeCache. Epic: none Release note: None
Now that the RangeCache.CacheEntry is private, we no longer need all the getters for the different fields. This simplifies some test code that was converting back and froth from pointers, structs. Epic: none Release note: None
The cache.Entry is not intended to be public, instead return a RangeInfo. Epic: none Release note: None
This method was only used internally and is simple to create manually now. It was confusing to have it on the public interface as its not the right way to create a token. Epic: none Release note: None
This PR replaces ReplicaSlice with ReplicaSet outside of the DistSender. ReplicaSlice is an internal implementation which contains information only required for sorting the replicas. Outside of DistSender the additional sorting information is unnecessary and unused. Epic: none Informs: cockroachdb#112351 Release note: None
In preperation for caching the sorted replicas, pull out the function to generate the two sorted lists. This is meant to be an intermediate commit as it will have worse performance characteristics since it always computes the list twice. Epic: none Release note: None
Calling OptimizeReplicaOrder is expensive and can show up as a hotspot in many high QPS workloads. This PR caches the sorted list on a per-range basis and updates the cache at most once every 3s. Epic: none Release note: None
andrewbaptist
force-pushed
the
2023-11-07-cache-replica-slice
branch
from
19d6f86 to
b9025b8
Compare
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.
Calling OptimizeReplicaOrder is expensive and can show up as a hotspot in many high QPS workloads. This PR caches the sorted list on a per-range basis and updates the cache at most once every 3s.
Epic: none
Release note: None