Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
106202: allocator: promote non-voter in 1-voter rebalance r=erikgrinaker a=kvoli

When there is only 1 voter and 1 non-voter, it was not possible to rebalance the voter replica onto the store which had the non-voter. When attempting to rebalance with only 1 voter, we fall back to adding the new replica first. Adding this new replica fails if the store already has a non-voting replica.

This patch updates the 1 voter case in  `ReplicationChangesForRebalance` to also consider whether the add target store already has a non-voting replica. If a non-voting replica is found, a promotion is now issued rather than just adding the voter.

Fixes: cockroachdb#104066
Epic: none

Release note: None

Co-authored-by: Austen McClernon <[email protected]>
  • Loading branch information
craig[bot] and kvoli committed Jul 10, 2023
2 parents 28fad87 + 9ed7027 commit e2af0b8
Showing 1 changed file with 20 additions and 4 deletions.
24 changes: 20 additions & 4 deletions pkg/kv/kvserver/allocator/plan/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func ReplicationChangesForRebalance(
addTarget, removeTarget roachpb.ReplicationTarget,
rebalanceTargetType allocatorimpl.TargetReplicaType,
) (chgs []kvpb.ReplicationChange, performingSwap bool, err error) {
rdesc, found := desc.GetReplicaDescriptor(addTarget.StoreID)
if rebalanceTargetType == allocatorimpl.VoterTarget && numExistingVoters == 1 {
// If there's only one replica, the removal target is the
// leaseholder and this is unsupported and will fail. However,
Expand All @@ -58,14 +59,29 @@ func ReplicationChangesForRebalance(
// when we know it's necessary, picking the smaller of two evils.
//
// See https://github.com/cockroachdb/cockroach/issues/40333.
chgs = []kvpb.ReplicationChange{
{ChangeType: roachpb.ADD_VOTER, Target: addTarget},
}
log.KvDistribution.Infof(ctx, "can't swap replica due to lease; falling back to add")

// Even when there is only 1 existing voter, there may be other replica
// types in the range. Check if the add target already has a replica, if so
// it must be a non-voter or the rebalance is invalid.
if found && rdesc.Type == roachpb.NON_VOTER {
// The receiving store already has a non-voting replica. Instead of just
// adding a voter to the receiving store, we *must* promote the non-voting
// replica to a voter.
chgs = kvpb.ReplicationChangesForPromotion(addTarget)
} else if !found {
chgs = []kvpb.ReplicationChange{
{ChangeType: roachpb.ADD_VOTER, Target: addTarget},
}
} else {
return nil, false, errors.AssertionFailedf(
"invalid rebalancing decision: trying to"+
" move voter to a store that already has a replica %s for the range", rdesc,
)
}
return chgs, false, err
}

rdesc, found := desc.GetReplicaDescriptor(addTarget.StoreID)
switch rebalanceTargetType {
case allocatorimpl.VoterTarget:
// Check if the target being added already has a non-voting replica.
Expand Down

0 comments on commit e2af0b8

Please sign in to comment.