allocator: add support for store pool liveness overrides#91965
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Dec 10, 2022
Merged
allocator: add support for store pool liveness overrides#91965craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Member
|
This change is |
d1ed4e0 to
74a4d61
Compare
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Nov 16, 2022
This change adds methods to be to evaluate allocator actions and targets utilizing a passed-in `StorePool` object, allowing for the allocator to consider potential scenarios rather than those simply based on the current liveness. Depends on cockroachdb#91461, cockroachdb#91965. Part of cockroachdb#91570. Release note: None
This was referenced Nov 16, 2022
06e8e09 to
a8b0179
Compare
a8b0179 to
225fa94
Compare
225fa94 to
45dd43f
Compare
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Nov 18, 2022
This change adds methods to be to evaluate allocator actions and targets utilizing a passed-in `StorePool` object, allowing for the allocator to consider potential scenarios rather than those simply based on the current liveness. Depends on cockroachdb#91461, cockroachdb#91965. Part of cockroachdb#91570. Release note: None
45dd43f to
e79788d
Compare
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Nov 23, 2022
This change adds methods to be to evaluate allocator actions and targets utilizing a passed-in `StorePool` object, allowing for the allocator to consider potential scenarios rather than those simply based on the current liveness. Depends on cockroachdb#91461, cockroachdb#91965. Part of cockroachdb#91570. Release note: None
kvoli
approved these changes
Nov 29, 2022
Contributor
kvoli
left a comment
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @AlexTalks and @andrewbaptist)
pkg/kv/kvserver/allocator/storepool/override_store_pool.go line 29 at r1 (raw file):
// The OverrideStorePool's methods fall through to the underlying StorePool // while passing in the configured NodeLivenessFunc to be used in place of the // underlying StorePool's. This is only the case, however, for StorePool methods
nit: This last sentence is hard to parse - could it be reworded to be clearer?
Code quote:
// underlying StorePool's. This is only the case, however, for StorePool methods
// that are read-only, despite StorePool.DetailsMu being held in write mode in
// some functions, avoiding the possibility of mutating underlying state.pkg/kv/kvserver/allocator/storepool/override_store_pool_test.go line 179 at r1 (raw file):
liveReplicas, deadReplicas := sp.LiveAndDeadReplicas(replicas, false /* includeSuspectAndDrainingStores */) if len(liveReplicas) != 5 { t.Fatalf("expected five live replicas, found %d (%v)", len(liveReplicas), liveReplicas)
Why not use the require lib in these tests?
While previously the allocator only evaluated using liveness obtained from gossip, this change introduces a new `OverrideStorePool` struct which can be used to override the liveness of a node for the purposes of evaluating allocator actions and targets. This `OverrideStorePool` is backed by an existing actual `StorePool`, which retains the majority of its logic. Depends on cockroachdb#91461. Part of cockroachdb#91570. Release note: None
e79788d to
8ae3466
Compare
AlexTalks
commented
Dec 9, 2022
Contributor
Author
AlexTalks
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/allocator/storepool/override_store_pool_test.go line 179 at r1 (raw file):
Previously, kvoli (Austen) wrote…
Why not use the
requirelib in these tests?
Done.
Contributor
Author
|
bors r+ |
Contributor
|
Build succeeded: |
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Dec 15, 2022
This change adds methods to be to evaluate allocator actions and targets utilizing a passed-in `StorePool` object, allowing for the allocator to consider potential scenarios rather than those simply based on the current liveness. Depends on cockroachdb#91461, cockroachdb#91965. Part of cockroachdb#91570. Release note: None
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Dec 19, 2022
This change adds methods to be to evaluate allocator actions and targets utilizing a passed-in `StorePool` object, allowing for the allocator to consider potential scenarios rather than those simply based on the current liveness. Depends on cockroachdb#91461, cockroachdb#91965. Part of cockroachdb#91570. Release note: None
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Dec 20, 2022
This change adds methods to be to evaluate allocator actions and targets utilizing a passed-in `StorePool` object, allowing for the allocator to consider potential scenarios rather than those simply based on the current liveness. Depends on cockroachdb#91461, cockroachdb#91965. Part of cockroachdb#91570. Release note: None
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Dec 21, 2022
This change adds methods to evaluate allocator actions and targets utilizing a passed-in `StorePool` object, allowing for the allocator to consider potential scenarios rather than those simply based on the current liveness. Depends on cockroachdb#91461, cockroachdb#91965. Part of cockroachdb#91570. Release note: None
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Dec 21, 2022
This change adds methods to evaluate allocator actions and targets utilizing a passed-in `StorePool` object, allowing for the allocator to consider potential scenarios rather than those simply based on the current liveness. Depends on cockroachdb#91461, cockroachdb#91965. Part of cockroachdb#91570. Release note: None
adityamaru
pushed a commit
to adityamaru/cockroach
that referenced
this pull request
Dec 22, 2022
This change adds methods to evaluate allocator actions and targets utilizing a passed-in `StorePool` object, allowing for the allocator to consider potential scenarios rather than those simply based on the current liveness. Depends on cockroachdb#91461, cockroachdb#91965. Part of cockroachdb#91570. Release note: None
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.
While previously the allocator only evaluated using liveness obtained
from gossip, this change introduces a new
OverrideStorePoolstructwhich can be used to override the liveness of a node for the purposes of
evaluating allocator actions and targets. This
OverrideStorePoolisbacked by an existing actual
StorePool, which retains the majority ofits logic.
Depends on #91461.
Part of #91570.
Release note: None