allocator: carve out a package boundary#79991
Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom Apr 22, 2022
Merged
Conversation
Member
|
This change is |
b25f7d7 to
aa1db0e
Compare
232c880 to
f73578d
Compare
41a18b3 to
e73b494
Compare
This commit tries to pave the way for the allocator itself to be carved out (storepool probably being a direct dependency, but ideally through an interface). This commit shamelessly exports any and everything needed in order for StorePool to sit outside the kvserver package boundary. It's unfortunate how tangled up this component is, including in tests (some of which reach deep into inner mutexes and mutate state). We're not doing anything to improve the status quo other than moving it and maybe pointing to the awkwardness. Release note: None
e73b494 to
fc79409
Compare
kvoli
suggested changes
Apr 19, 2022
Do the same for kvserver/replicastats, a type that's used as an input for the allocator at various points (needed to break the dependency between the allocatorimpl and kvserver). We take the same approach as the last commit, exporting anything and everything needed to introduce a hard package boundary. Release note: None
fc79409 to
4f96b1b
Compare
kvoli
approved these changes
Apr 22, 2022
Contributor
Author
|
🍺 bors r+ |
Contributor
|
Build succeeded: |
irfansharif
added a commit
to irfansharif/cockroach
that referenced
this pull request
Oct 14, 2022
Manual reconstruction of cockroachdb#84863, mostly due to merge conflicts from \cockroachdb#79991 which will not make it to 22.1. Previously, lease preferences were not considered when selecting a voter to become the leaseholder after replica rebalancing. Instead, the voter on the store with the least QPS was selected. This patch introduces a check to try and satisfy the lease preference from the rebalanced voting set of replicas, if possible. Among voters satisfying the preference, the one on a store with the least QPS is selected to be the leaseholder. When not possible, where none of the voting replicas satisfy the lease preference, the store rebalancer will pick the miminum QPS store, as before. Release note (bug fix, performance improvement): Previously it was possible for leases to temporarily move outside of explicitly configured regions. This often happened during load based-rebalancing, something CRDB continually does across the cluster. Because of this it was also possible to observe a continual rate of lease thrashing as leases moved out of configured zones, triggered rebalancing, induce other leases to move out of configured zone, while the original set moved back, and so on. This is now fixed. Release justification: Bug fix.
irfansharif
added a commit
to irfansharif/cockroach
that referenced
this pull request
Oct 14, 2022
Manual reconstruction of cockroachdb#84863, mostly due to merge conflicts from \cockroachdb#79991 which will not make it to 22.1. Previously, lease preferences were not considered when selecting a voter to become the leaseholder after replica rebalancing. Instead, the voter on the store with the least QPS was selected. This patch introduces a check to try and satisfy the lease preference from the rebalanced voting set of replicas, if possible. Among voters satisfying the preference, the one on a store with the least QPS is selected to be the leaseholder. When not possible, where none of the voting replicas satisfy the lease preference, the store rebalancer will pick the miminum QPS store, as before. Release note (bug fix, performance improvement): Previously it was possible for leases to temporarily move outside of explicitly configured regions. This often happened during load based-rebalancing, something CRDB continually does across the cluster. Because of this it was also possible to observe a continual rate of lease thrashing as leases moved out of configured zones, triggered rebalancing, induce other leases to move out of configured zone, while the original set moved back, and so on. This is now fixed. Release justification: Bug fix.
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
3 participants
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.
allocator/storepool: carve out package from kvserver
This commit tries to pave the way for the allocator itself to be carved
out (storepool probably being a direct dependency, but ideally through
an interface). This commit shamelessly exports any and everything needed
in order for StorePool to sit outside the kvserver package boundary.
It's unfortunate how tangled up this component is, including in tests
(some of which reach deep into inner mutexes and mutate state). We're
not doing anything to improve the status quo other than moving it and
maybe pointing to the awkwardness.
allocatorimpl: carve out an allocator package
Do the same for kvserver/replicastats, a type that's used as an input
for the allocator at various points (needed to break the dependency
between the allocatorimpl and kvserver). We take the same approach as
the last commit, exporting anything and everything needed to introduce a
hard package boundary.