Skip to content

Commit 10fc938

Browse files
committed
kvserver: campaign when leader is demoted to learner
This patch campaigns when the leader demotes itself to a learner during an atomic conf change, instead of waiting until it is completely removed from the range. Relying on the old leader to commit the final conf change while it's a learner appears unfortunate, and is not compatible with an upcoming etcd/raft change that makes the learner step down in this case. This is backwards compatible with 23.1, because the same follower replica is responsible for campaigning, and it does not matter if it campaigns during the demotion or removal -- in particular because it forces an immediate election via `forceCampaignLocked()` which immediately bumps the term. Epic: none Release note: None
1 parent 9cffd6e commit 10fc938

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

pkg/kv/kvserver/replica_raft.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2603,9 +2603,9 @@ func ComputeRaftLogSize(
26032603
}
26042604

26052605
// shouldCampaignAfterConfChange returns true if the current replica should
2606-
// campaign after a conf change. If the leader replica is removed, the
2607-
// leaseholder should campaign. We don't want to campaign on multiple replicas,
2608-
// since that would cause ties.
2606+
// campaign after a conf change. If the leader replica is demoted or removed,
2607+
// the leaseholder should campaign. We don't want to campaign on multiple
2608+
// replicas, since that would cause ties.
26092609
//
26102610
// If there is no current leaseholder we'll have to wait out the election
26112611
// timeout before someone campaigns, but that's ok -- either we'll have to wait
@@ -2622,7 +2622,7 @@ func shouldCampaignAfterConfChange(
26222622
raftStatus raft.BasicStatus,
26232623
leaseStatus kvserverpb.LeaseStatus,
26242624
) bool {
2625-
if raftStatus.Lead == 0 {
2625+
if raftStatus.Lead == raft.None {
26262626
// Leader unknown. We can't know if it was removed by the conf change, and
26272627
// because we force an election without prevote we don't want to risk
26282628
// throwing spurious elections.
@@ -2637,9 +2637,11 @@ func shouldCampaignAfterConfChange(
26372637
// don't expect to hit this, but let's be defensive.
26382638
return false
26392639
}
2640-
if _, ok := desc.GetReplicaDescriptorByID(roachpb.ReplicaID(raftStatus.Lead)); ok {
2641-
// The leader is still in the descriptor.
2642-
return false
2640+
if replDesc, ok := desc.GetReplicaDescriptorByID(roachpb.ReplicaID(raftStatus.Lead)); ok {
2641+
if replDesc.IsAnyVoter() {
2642+
// The leader is still a voter in the descriptor.
2643+
return false
2644+
}
26432645
}
26442646
// Prior to 23.2, the first voter in the descriptor campaigned, so we do
26452647
// the same in mixed-version clusters to avoid ties.

0 commit comments

Comments
 (0)