Skip to content

Commit

Permalink
kvserver: rename allocator actions and priorities
Browse files Browse the repository at this point in the history 
This commit renames the existing `AllocatorAction`s and priorities to
indicate that they pertain only to voting replicas. This is intended to
pave the way for new non-voter specific allocator actions and priorities
that follow in the subsequent commits in this PR.

Release note: None
  • Loading branch information
@aayushshah15
aayushshah15 committed Feb 1, 2021
1 parent bea5b13 commit 3222cc9
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 73 deletions.
74 changes: 38 additions & 36 deletions pkg/kv/kvserver/allocator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ const (
minReplicaWeight = 0.001

// Priorities for various repair operations.
finalizeAtomicReplicationChangePriority float64 = 12002
removeLearnerReplicaPriority float64 = 12001
addDeadReplacementPriority float64 = 12000
addMissingReplicaPriority float64 = 10000
addDecommissioningReplacementPriority float64 = 5000
removeDeadReplicaPriority float64 = 1000
removeDecommissioningReplicaPriority float64 = 200
removeExtraReplicaPriority float64 = 100
finalizeAtomicReplicationChangePriority float64 = 12002
removeLearnerReplicaPriority float64 = 12001
addDeadReplacementVoterPriority float64 = 12000
addMissingVoterPriority float64 = 10000
addDecommissioningReplacementVoterPriority float64 = 5000
removeDeadVoterPriority float64 = 1000
removeDecommissioningVoterPriority float64 = 200
removeExtraVoterPriority float64 = 100
)

// MinLeaseTransferStatsDuration configures the minimum amount of time a
Expand Down Expand Up @@ -99,27 +99,29 @@ type AllocatorAction int
const (
_ AllocatorAction = iota
AllocatorNoop
AllocatorRemove
AllocatorAdd
AllocatorReplaceDead
AllocatorRemoveDead
AllocatorReplaceDecommissioning
AllocatorRemoveDecommissioning
AllocatorRemoveVoter
AllocatorAddVoter
AllocatorReplaceDeadVoter
AllocatorRemoveDeadVoter
AllocatorReplaceDecommissioningVoter
AllocatorRemoveDecommissioningVoter
AllocatorRemoveLearner
AllocatorConsiderRebalance
AllocatorRangeUnavailable
AllocatorFinalizeAtomicReplicationChange
)

var allocatorActionNames = map[AllocatorAction]string{
AllocatorNoop: "noop",
AllocatorRemove: "remove",
AllocatorAdd: "add",
AllocatorReplaceDead: "replace dead",
AllocatorRemoveDead: "remove dead",
AllocatorReplaceDecommissioning: "replace decommissioning",
AllocatorRemoveDecommissioning: "remove decommissioning",
AllocatorRemoveLearner: "remove learner",
AllocatorNoop: "noop",
AllocatorRemoveVoter: "remove voter",
AllocatorAddVoter: "add voter",
AllocatorReplaceDeadVoter: "replace dead voter",
AllocatorRemoveDeadVoter: "remove dead voter",
AllocatorReplaceDecommissioningVoter: "replace decommissioning voter",
AllocatorRemoveDecommissioningVoter: "remove decommissioning voter",
AllocatorRemoveLearner: "remove learner",
// TODO(aayush): Rationalize whether or not rebalancing of non-voters needs to
// be dictated by a distinct allocator action.
AllocatorConsiderRebalance: "consider rebalance",
AllocatorRangeUnavailable: "range unavailable",
AllocatorFinalizeAtomicReplicationChange: "finalize conf change",
Expand Down Expand Up @@ -350,8 +352,8 @@ func (a *Allocator) ComputeAction(
// TODO(dan): Since this goes before anything else, the priority here should
// be influenced by whatever operations would happen right after the learner
// is removed. In the meantime, we don't want to block something important
// from happening (like addDeadReplacementPriority) by queueing this at a
// low priority so until this TODO is done, keep
// from happening (like addDeadReplacementVoterPriority) by queueing this at
// a low priority so until this TODO is done, keep
// removeLearnerReplicaPriority as the highest priority.
return AllocatorRemoveLearner, removeLearnerReplicaPriority
}
Expand All @@ -373,8 +375,8 @@ func (a *Allocator) computeAction(
// Range is under-replicated, and should add an additional replica.
// Priority is adjusted by the difference between the current replica
// count and the quorum of the desired replica count.
priority := addMissingReplicaPriority + float64(desiredQuorum-have)
action := AllocatorAdd
priority := addMissingVoterPriority + float64(desiredQuorum-have)
action := AllocatorAddVoter
log.VEventf(ctx, 3, "%s - missing replica need=%d, have=%d, priority=%.2f",
action, need, have, priority)
return action, priority
Expand All @@ -397,17 +399,17 @@ func (a *Allocator) computeAction(
// where the node is only temporarily dead, but we remove it from the range
// and lose a second node before we can up-replicate (#25392).
// The dead replica(s) will be down-replicated later.
priority := addDeadReplacementPriority
action := AllocatorReplaceDead
priority := addDeadReplacementVoterPriority
action := AllocatorReplaceDeadVoter
log.VEventf(ctx, 3, "%s - replacement for %d dead replicas priority=%.2f",
action, len(deadVoterReplicas), priority)
return action, priority
}

if have == need && len(decommissioningReplicas) > 0 {
// Range has decommissioning replica(s), which should be replaced.
priority := addDecommissioningReplacementPriority
action := AllocatorReplaceDecommissioning
priority := addDecommissioningReplacementVoterPriority
action := AllocatorReplaceDecommissioningVoter
log.VEventf(ctx, 3, "%s - replacement for %d decommissioning replicas priority=%.2f",
action, len(decommissioningReplicas), priority)
return action, priority
Expand All @@ -421,8 +423,8 @@ func (a *Allocator) computeAction(
// replicas.
if len(deadVoterReplicas) > 0 {
// The range has dead replicas, which should be removed immediately.
priority := removeDeadReplicaPriority + float64(quorum-len(liveVoterReplicas))
action := AllocatorRemoveDead
priority := removeDeadVoterPriority + float64(quorum-len(liveVoterReplicas))
action := AllocatorRemoveDeadVoter
log.VEventf(ctx, 3, "%s - dead=%d, live=%d, quorum=%d, priority=%.2f",
action, len(deadVoterReplicas), len(liveVoterReplicas), quorum, priority)
return action, priority
Expand All @@ -431,8 +433,8 @@ func (a *Allocator) computeAction(
if len(decommissioningReplicas) > 0 {
// Range is over-replicated, and has a decommissioning replica which
// should be removed.
priority := removeDecommissioningReplicaPriority
action := AllocatorRemoveDecommissioning
priority := removeDecommissioningVoterPriority
action := AllocatorRemoveDecommissioningVoter
log.VEventf(ctx, 3,
"%s - need=%d, have=%d, num_decommissioning=%d, priority=%.2f",
action, need, have, len(decommissioningReplicas), priority)
Expand All @@ -443,8 +445,8 @@ func (a *Allocator) computeAction(
// Range is over-replicated, and should remove a replica.
// Ranges with an even number of replicas get extra priority because
// they have a more fragile quorum.
priority := removeExtraReplicaPriority - float64(have%2)
action := AllocatorRemove
priority := removeExtraVoterPriority - float64(have%2)
action := AllocatorRemoveVoter
log.VEventf(ctx, 3, "%s - need=%d, have=%d, priority=%.2f", action, need, have, priority)
return action, priority
}
Expand Down
54 changes: 27 additions & 27 deletions pkg/kv/kvserver/allocator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4265,7 +4265,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorReplaceDead,
expectedAction: AllocatorReplaceDeadVoter,
},
// Need five replicas, one is on a dead store.
{
Expand Down Expand Up @@ -4304,7 +4304,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorReplaceDead,
expectedAction: AllocatorReplaceDeadVoter,
},
// Need three replicas, have two.
{
Expand All @@ -4328,7 +4328,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorAdd,
expectedAction: AllocatorAddVoter,
},
// Need five replicas, have four, one is on a dead store.
{
Expand Down Expand Up @@ -4362,7 +4362,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorAdd,
expectedAction: AllocatorAddVoter,
},
// Need five replicas, have four.
{
Expand Down Expand Up @@ -4396,7 +4396,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorAdd,
expectedAction: AllocatorAddVoter,
},
// Need three replicas, have four, one is on a dead store.
{
Expand Down Expand Up @@ -4430,7 +4430,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorRemoveDead,
expectedAction: AllocatorRemoveDeadVoter,
},
// Need five replicas, have six, one is on a dead store.
{
Expand Down Expand Up @@ -4474,7 +4474,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorRemoveDead,
expectedAction: AllocatorRemoveDeadVoter,
},
// Need three replicas, have five, one is on a dead store.
{
Expand Down Expand Up @@ -4513,7 +4513,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorRemoveDead,
expectedAction: AllocatorRemoveDeadVoter,
},
// Need three replicas, have four.
{
Expand Down Expand Up @@ -4547,7 +4547,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorRemove,
expectedAction: AllocatorRemoveVoter,
},
// Need three replicas, have five.
{
Expand Down Expand Up @@ -4586,7 +4586,7 @@ func TestAllocatorComputeAction(t *testing.T) {
},
},
},
expectedAction: AllocatorRemove,
expectedAction: AllocatorRemoveVoter,
},
// Need three replicas, two are on dead stores. Should
// be a noop because there aren't enough live replicas for
Expand Down Expand Up @@ -4756,14 +4756,14 @@ func TestAllocatorComputeActionRemoveDead(t *testing.T) {
desc: threeReplDesc,
live: []roachpb.StoreID{1, 2},
dead: []roachpb.StoreID{3},
expectedAction: AllocatorReplaceDead,
expectedAction: AllocatorReplaceDeadVoter,
},
// Needs three replicas, one is dead, but there is a replacement.
{
desc: threeReplDesc,
live: []roachpb.StoreID{1, 2, 4},
dead: []roachpb.StoreID{3},
expectedAction: AllocatorReplaceDead,
expectedAction: AllocatorReplaceDeadVoter,
},
// Needs three replicas, two are dead (i.e. the range lacks a quorum).
{
Expand All @@ -4777,7 +4777,7 @@ func TestAllocatorComputeActionRemoveDead(t *testing.T) {
desc: fourReplDesc,
live: []roachpb.StoreID{1, 2, 4},
dead: []roachpb.StoreID{3},
expectedAction: AllocatorRemoveDead,
expectedAction: AllocatorRemoveDeadVoter,
},
// Needs three replicas, has four, two are dead (i.e. the range lacks a quorum).
{
Expand Down Expand Up @@ -4840,7 +4840,7 @@ func TestAllocatorComputeActionDecommission(t *testing.T) {
},
},
},
expectedAction: AllocatorReplaceDecommissioning,
expectedAction: AllocatorReplaceDecommissioningVoter,
live: []roachpb.StoreID{1, 2},
dead: nil,
decommissioning: []roachpb.StoreID{3},
Expand Down Expand Up @@ -4870,7 +4870,7 @@ func TestAllocatorComputeActionDecommission(t *testing.T) {
},
},
},
expectedAction: AllocatorReplaceDead,
expectedAction: AllocatorReplaceDeadVoter,
live: []roachpb.StoreID{1},
dead: []roachpb.StoreID{2},
decommissioning: []roachpb.StoreID{3},
Expand Down Expand Up @@ -4905,7 +4905,7 @@ func TestAllocatorComputeActionDecommission(t *testing.T) {
},
},
},
expectedAction: AllocatorRemoveDead,
expectedAction: AllocatorRemoveDeadVoter,
live: []roachpb.StoreID{1, 4},
dead: []roachpb.StoreID{2},
decommissioning: []roachpb.StoreID{3},
Expand Down Expand Up @@ -4940,7 +4940,7 @@ func TestAllocatorComputeActionDecommission(t *testing.T) {
},
},
},
expectedAction: AllocatorRemoveDead,
expectedAction: AllocatorRemoveDeadVoter,
live: []roachpb.StoreID{1, 4},
dead: nil,
decommissioning: []roachpb.StoreID{3},
Expand Down Expand Up @@ -4970,7 +4970,7 @@ func TestAllocatorComputeActionDecommission(t *testing.T) {
},
},
},
expectedAction: AllocatorReplaceDecommissioning,
expectedAction: AllocatorReplaceDecommissioningVoter,
live: nil,
dead: nil,
decommissioning: []roachpb.StoreID{1, 2, 3},
Expand Down Expand Up @@ -5004,7 +5004,7 @@ func TestAllocatorComputeActionDecommission(t *testing.T) {
},
},
},
expectedAction: AllocatorRemoveDecommissioning,
expectedAction: AllocatorRemoveDecommissioningVoter,
live: []roachpb.StoreID{4},
dead: nil,
decommissioning: []roachpb.StoreID{1, 2, 3},
Expand Down Expand Up @@ -5082,7 +5082,7 @@ func TestAllocatorComputeActionDynamicNumReplicas(t *testing.T) {
// drop past 3, so 3 it is.
storeList: []roachpb.StoreID{1, 2, 3, 4},
expectedNumReplicas: 3,
expectedAction: AllocatorRemoveDecommissioning,
expectedAction: AllocatorRemoveDecommissioningVoter,
live: []roachpb.StoreID{4},
unavailable: nil,
dead: nil,
Expand All @@ -5092,7 +5092,7 @@ func TestAllocatorComputeActionDynamicNumReplicas(t *testing.T) {
// Ditto.
storeList: []roachpb.StoreID{1, 2, 3},
expectedNumReplicas: 3,
expectedAction: AllocatorReplaceDecommissioning,
expectedAction: AllocatorReplaceDecommissioningVoter,
live: []roachpb.StoreID{4, 5},
unavailable: nil,
dead: nil,
Expand All @@ -5105,7 +5105,7 @@ func TestAllocatorComputeActionDynamicNumReplicas(t *testing.T) {
// from the range at hand, rather than trying to replace it.
storeList: []roachpb.StoreID{1, 2, 3, 4},
expectedNumReplicas: 3,
expectedAction: AllocatorRemoveDead,
expectedAction: AllocatorRemoveDeadVoter,
live: []roachpb.StoreID{1, 2, 3, 5},
unavailable: nil,
dead: []roachpb.StoreID{4},
Expand All @@ -5118,7 +5118,7 @@ func TestAllocatorComputeActionDynamicNumReplicas(t *testing.T) {
// important than replacing the dead one.
storeList: []roachpb.StoreID{1, 4},
expectedNumReplicas: 3,
expectedAction: AllocatorAdd,
expectedAction: AllocatorAddVoter,
live: []roachpb.StoreID{1, 2, 3, 5},
unavailable: nil,
dead: []roachpb.StoreID{4},
Expand All @@ -5140,7 +5140,7 @@ func TestAllocatorComputeActionDynamicNumReplicas(t *testing.T) {
// it is and we are under-replicaed.
storeList: []roachpb.StoreID{1, 2},
expectedNumReplicas: 3,
expectedAction: AllocatorAdd,
expectedAction: AllocatorAddVoter,
live: []roachpb.StoreID{1, 2},
unavailable: nil,
dead: nil,
Expand All @@ -5160,7 +5160,7 @@ func TestAllocatorComputeActionDynamicNumReplicas(t *testing.T) {
// Three again, on account of avoiding the even four.
storeList: []roachpb.StoreID{1, 2, 3, 4},
expectedNumReplicas: 3,
expectedAction: AllocatorRemove,
expectedAction: AllocatorRemoveVoter,
live: []roachpb.StoreID{1, 2, 3, 4},
unavailable: nil,
dead: nil,
Expand Down Expand Up @@ -5214,7 +5214,7 @@ func TestAllocatorComputeActionDynamicNumReplicas(t *testing.T) {
// slice in these tests).
storeList: []roachpb.StoreID{1, 2, 3, 4, 5},
expectedNumReplicas: 5,
expectedAction: AllocatorReplaceDead,
expectedAction: AllocatorReplaceDeadVoter,
live: []roachpb.StoreID{1, 2, 3},
unavailable: []roachpb.StoreID{4},
dead: []roachpb.StoreID{5},
Expand All @@ -5225,7 +5225,7 @@ func TestAllocatorComputeActionDynamicNumReplicas(t *testing.T) {
// the most important thing is removing a decommissioning replica.
storeList: []roachpb.StoreID{1, 2, 3, 4, 5},
expectedNumReplicas: 3,
expectedAction: AllocatorRemoveDecommissioning,
expectedAction: AllocatorRemoveDecommissioningVoter,
live: []roachpb.StoreID{1, 2, 3},
unavailable: []roachpb.StoreID{4},
dead: nil,
Expand Down
Loading

0 comments on commit 3222cc9

Please sign in to comment.