-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
allocator: add support for store pool liveness overrides #91965
allocator: add support for store pool liveness overrides #91965
Conversation
d1ed4e0
to
74a4d61
Compare
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
06e8e09
to
a8b0179
Compare
a8b0179
to
225fa94
Compare
225fa94
to
45dd43f
Compare
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
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andrewbaptist and @kvoli)
pkg/kv/kvserver/allocator/storepool/override_store_pool_test.go
line 179 at r1 (raw file):
Previously, kvoli (Austen) wrote…
Why not use the
require
lib in these tests?
Done.
bors r+ |
Build succeeded: |
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 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 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 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 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 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
While previously the allocator only evaluated using liveness obtained
from gossip, this change introduces a new
OverrideStorePool
structwhich can be used to override the liveness of a node for the purposes of
evaluating allocator actions and targets. This
OverrideStorePool
isbacked by an existing actual
StorePool
, which retains the majority ofits logic.
Depends on #91461.
Part of #91570.
Release note: None