Skip to content

Commit

Permalink
Merge pull request #77 from erikgrinaker/remove-leader-step-down
Browse files Browse the repository at this point in the history
step down when leader removes itself
  • Loading branch information
ahrtr authored Jun 26, 2023
2 parents a10cd45 + 72d62a1 commit 4abd9e9
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 80 deletions.
14 changes: 7 additions & 7 deletions raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -1892,15 +1892,15 @@ func (r *raft) switchToConfig(cfg tracker.Config, prs tracker.ProgressMap) pb.Co
r.isLearner = ok && pr.IsLearner

if (!ok || r.isLearner) && r.state == StateLeader {
// This node is leader and was removed or demoted. We prevent demotions
// at the time writing but hypothetically we handle them the same way as
// removing the leader: stepping down into the next Term.
// This node is leader and was removed or demoted, step down.
//
// TODO(tbg): step down (for sanity) and ask follower with largest Match
// to TimeoutNow (to avoid interruption). This might still drop some
// proposals but it's better than nothing.
// We prevent demotions at the time writing but hypothetically we handle
// them the same way as removing the leader.
//
// TODO(tbg): test this branch. It is untested at the time of writing.
// TODO(tbg): ask follower with largest Match to TimeoutNow (to avoid
// interruption). This might still drop some proposals but it's better than
// nothing.
r.becomeFollower(r.Term, None)
return cs
}

Expand Down
86 changes: 13 additions & 73 deletions testdata/confchange_v1_remove_leader.txt
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,8 @@ propose 1 bar
----
ok

# n1 applies the conf change, so it has now removed itself. But it still has
# an uncommitted entry in the log. If the leader unconditionally counted itself
# as part of the commit quorum, we'd be in trouble. In the block below, we see
# it send out appends to the other nodes for the 'bar' entry.
# n1 applies the conf change, removing itself and stepping down. But it still
# has an uncommitted 'bar' entry in the log that it sends out appends for first.
stabilize 1
----
> 1 handling Ready
Expand All @@ -106,10 +104,14 @@ stabilize 1
1->2 MsgApp Term:1 Log:1/6 Commit:5
1->3 MsgApp Term:1 Log:1/6 Commit:5
INFO 1 switched to configuration voters=(2 3)
INFO 1 became follower at term 1
> 1 handling Ready
Ready MustSync=false:
Lead:0 State:StateFollower

raft-state
----
1: StateLeader (Non-Voter) Term:1 Lead:1
1: StateFollower (Non-Voter) Term:1 Lead:0
2: StateFollower (Voter) Term:1 Lead:1
3: StateFollower (Voter) Term:1 Lead:1

Expand All @@ -134,7 +136,7 @@ stabilize 2
2->1 MsgAppResp Term:1 Log:0/6
INFO 2 switched to configuration voters=(2 3)

# ... which thankfully is what we see on the leader.
# ...because the old leader n1 ignores the append responses.
stabilize 1
----
> 1 receiving messages
Expand Down Expand Up @@ -174,76 +176,14 @@ stabilize
3->1 MsgAppResp Term:1 Log:0/6
3->1 MsgAppResp Term:1 Log:0/6
3->1 MsgAppResp Term:1 Log:0/6
> 1 handling Ready
Ready MustSync=false:
HardState Term:1 Vote:1 Commit:6
CommittedEntries:
1/6 EntryNormal "bar"
Messages:
1->2 MsgApp Term:1 Log:1/6 Commit:6
1->3 MsgApp Term:1 Log:1/6 Commit:6
> 2 receiving messages
1->2 MsgApp Term:1 Log:1/6 Commit:6
> 3 receiving messages
1->3 MsgApp Term:1 Log:1/6 Commit:6
> 2 handling Ready
Ready MustSync=false:
HardState Term:1 Vote:1 Commit:6
CommittedEntries:
1/6 EntryNormal "bar"
Messages:
2->1 MsgAppResp Term:1 Log:0/6
> 3 handling Ready
Ready MustSync=false:
HardState Term:1 Vote:1 Commit:6
CommittedEntries:
1/6 EntryNormal "bar"
Messages:
3->1 MsgAppResp Term:1 Log:0/6
> 1 receiving messages
2->1 MsgAppResp Term:1 Log:0/6
3->1 MsgAppResp Term:1 Log:0/6

# However not all is well. n1 is still leader but unconditionally drops all
# proposals on the floor, so we're effectively stuck if it still heartbeats
# its followers...
# n1 can no longer propose.
propose 1 baz
----
INFO 1 no leader at term 1; dropping proposal
raft proposal dropped

tick-heartbeat 1
----
ok

# ... which, uh oh, it does.
# TODO(tbg): change behavior so that a leader that is removed immediately steps
# down, and initiates an optimistic handover.
stabilize
----
> 1 handling Ready
Ready MustSync=false:
Messages:
1->2 MsgHeartbeat Term:1 Log:0/0 Commit:6
1->3 MsgHeartbeat Term:1 Log:0/0 Commit:6
> 2 receiving messages
1->2 MsgHeartbeat Term:1 Log:0/0 Commit:6
> 3 receiving messages
1->3 MsgHeartbeat Term:1 Log:0/0 Commit:6
> 2 handling Ready
Ready MustSync=false:
Messages:
2->1 MsgHeartbeatResp Term:1 Log:0/0
> 3 handling Ready
Ready MustSync=false:
Messages:
3->1 MsgHeartbeatResp Term:1 Log:0/0
> 1 receiving messages
2->1 MsgHeartbeatResp Term:1 Log:0/0
3->1 MsgHeartbeatResp Term:1 Log:0/0

# Just confirming the issue above - leader does not automatically step down.
raft-state
# Nor can it campaign to become leader.
campaign 1
----
1: StateLeader (Non-Voter) Term:1 Lead:1
2: StateFollower (Voter) Term:1 Lead:1
3: StateFollower (Voter) Term:1 Lead:1
WARN 1 is unpromotable and can not campaign

0 comments on commit 4abd9e9

Please sign in to comment.