Skip to content
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.

Sign up for GitHub

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

add StepDownOnRemoval #79

Merged
merged 1 commit into from
Jul 17, 2023
Merged

add StepDownOnRemoval #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions raft.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,13 @@ type Config struct {
// This option may be removed once false positives are no longer possible.
// See: https://github.com/etcd-io/raft/issues/80
DisableConfChangeValidation bool

// StepDownOnRemoval makes the leader step down when it is removed from the
// group or demoted to a learner.
//
// This behavior will become unconditional in the future. See:
// https://github.com/etcd-io/raft/issues/83
StepDownOnRemoval bool
}

func (c *Config) validate() error {
Expand Down Expand Up @@ -408,6 +415,7 @@ type raft struct {
// when raft changes its state to follower or candidate.
randomizedElectionTimeout int
disableProposalForwarding bool
stepDownOnRemoval bool

tick func()
step stepFunc
Expand Down Expand Up @@ -447,6 +455,7 @@ func newRaft(c *Config) *raft {
readOnly: newReadOnly(c.ReadOnlyOption),
disableProposalForwarding: c.DisableProposalForwarding,
disableConfChangeValidation: c.DisableConfChangeValidation,
stepDownOnRemoval: c.StepDownOnRemoval,
}

cfg, prs, err := confchange.Restore(confchange.Changer{
Expand Down Expand Up @@ -1918,15 +1927,17 @@ 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, step down.
// This node is leader and was removed or demoted, step down if requested.
//
// We prevent demotions at the time writing but hypothetically we handle
// them the same way as removing the leader.
//
// 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)
if r.stepDownOnRemoval {
r.becomeFollower(r.Term, None)
}
return cs
}

Expand Down
2 changes: 2 additions & 0 deletions rafttest/interaction_env_handler_add_nodes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ func (env *InteractionEnv) handleAddNodes(t *testing.T, d datadriven.TestData) e
default:
return fmt.Errorf("invalid read-only option %q", arg.Vals[i])
}
case "step-down-on-removal":
arg.Scan(t, i, &cfg.StepDownOnRemoval)
}
}
}
Expand Down
87 changes: 74 additions & 13 deletions testdata/confchange_v1_remove_leader.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,10 @@ propose 1 bar
----
ok

# 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.
# 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.
stabilize 1
----
> 1 handling Ready
Expand All @@ -104,14 +106,10 @@ 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: StateFollower (Non-Voter) Term:1 Lead:0
1: StateLeader (Non-Voter) Term:1 Lead:1
2: StateFollower (Voter) Term:1 Lead:1
3: StateFollower (Voter) Term:1 Lead:1

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

# ...because the old leader n1 ignores the append responses.
# ... which thankfully is what we see on the leader.
stabilize 1
----
> 1 receiving messages
Expand Down Expand Up @@ -176,14 +174,77 @@ 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

# n1 can no longer propose.
# 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...
propose 1 baz
----
INFO 1 no leader at term 1; dropping proposal
raft proposal dropped

# Nor can it campaign to become leader.
campaign 1
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
----
WARN 1 is unpromotable and can not campaign
> 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.
# Expected behavior: a new leader is elected after an election timeout.
raft-state
----
1: StateLeader (Non-Voter) Term:1 Lead:1
2: StateFollower (Voter) Term:1 Lead:1
3: StateFollower (Voter) Term:1 Lead:1
190 changes: 190 additions & 0 deletions testdata/confchange_v1_remove_leader_stepdown.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
# We'll turn this back on after the boilerplate.
log-level none
----
ok

# Run a V1 membership change that removes the leader, asking it
# to step down on removal.
# Bootstrap n1, n2, n3.
add-nodes 3 voters=(1,2,3) index=2 step-down-on-removal=true
----
ok

campaign 1
----
ok

stabilize
----
ok

log-level debug
----
ok

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

# Start removing n1.
propose-conf-change 1 v1=true
r1
----
ok

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

# Propose an extra entry which will be sent out together with the conf change.
propose 1 foo
----
ok

# Send out the corresponding appends.
process-ready 1
----
Ready MustSync=true:
Entries:
1/4 EntryConfChange r1
1/5 EntryNormal "foo"
Messages:
1->2 MsgApp Term:1 Log:1/3 Commit:3 Entries:[1/4 EntryConfChange r1]
1->3 MsgApp Term:1 Log:1/3 Commit:3 Entries:[1/4 EntryConfChange r1]
1->2 MsgApp Term:1 Log:1/4 Commit:3 Entries:[1/5 EntryNormal "foo"]
1->3 MsgApp Term:1 Log:1/4 Commit:3 Entries:[1/5 EntryNormal "foo"]

# Send response from n2 (which is enough to commit the entries so far next time
# n1 runs).
stabilize 2
----
> 2 receiving messages
1->2 MsgApp Term:1 Log:1/3 Commit:3 Entries:[1/4 EntryConfChange r1]
1->2 MsgApp Term:1 Log:1/4 Commit:3 Entries:[1/5 EntryNormal "foo"]
> 2 handling Ready
Ready MustSync=true:
Entries:
1/4 EntryConfChange r1
1/5 EntryNormal "foo"
Messages:
2->1 MsgAppResp Term:1 Log:0/4
2->1 MsgAppResp Term:1 Log:0/5

# Put another entry in n1's log.
propose 1 bar
----
ok

# 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
Ready MustSync=true:
Entries:
1/6 EntryNormal "bar"
Messages:
1->2 MsgApp Term:1 Log:1/5 Commit:3 Entries:[1/6 EntryNormal "bar"]
1->3 MsgApp Term:1 Log:1/5 Commit:3 Entries:[1/6 EntryNormal "bar"]
> 1 receiving messages
2->1 MsgAppResp Term:1 Log:0/4
2->1 MsgAppResp Term:1 Log:0/5
> 1 handling Ready
Ready MustSync=false:
HardState Term:1 Vote:1 Commit:5
CommittedEntries:
1/4 EntryConfChange r1
1/5 EntryNormal "foo"
Messages:
1->2 MsgApp Term:1 Log:1/6 Commit:4
1->3 MsgApp Term:1 Log:1/6 Commit:4
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: StateFollower (Non-Voter) Term:1 Lead:0
2: StateFollower (Voter) Term:1 Lead:1
3: StateFollower (Voter) Term:1 Lead:1

# n2 responds, n3 doesn't yet. Quorum for 'bar' should not be reached...
stabilize 2
----
> 2 receiving messages
1->2 MsgApp Term:1 Log:1/5 Commit:3 Entries:[1/6 EntryNormal "bar"]
1->2 MsgApp Term:1 Log:1/6 Commit:4
1->2 MsgApp Term:1 Log:1/6 Commit:5
> 2 handling Ready
Ready MustSync=true:
HardState Term:1 Vote:1 Commit:5
Entries:
1/6 EntryNormal "bar"
CommittedEntries:
1/4 EntryConfChange r1
1/5 EntryNormal "foo"
Messages:
2->1 MsgAppResp Term:1 Log:0/6
2->1 MsgAppResp Term:1 Log:0/6
2->1 MsgAppResp Term:1 Log:0/6
INFO 2 switched to configuration voters=(2 3)

# ...because the old leader n1 ignores the append responses.
stabilize 1
----
> 1 receiving messages
2->1 MsgAppResp Term:1 Log:0/6
2->1 MsgAppResp Term:1 Log:0/6
2->1 MsgAppResp Term:1 Log:0/6

# When n3 responds, quorum is reached and everything falls into place.
stabilize
----
> 3 receiving messages
1->3 MsgApp Term:1 Log:1/3 Commit:3 Entries:[1/4 EntryConfChange r1]
1->3 MsgApp Term:1 Log:1/4 Commit:3 Entries:[1/5 EntryNormal "foo"]
1->3 MsgApp Term:1 Log:1/5 Commit:3 Entries:[1/6 EntryNormal "bar"]
1->3 MsgApp Term:1 Log:1/6 Commit:4
1->3 MsgApp Term:1 Log:1/6 Commit:5
> 3 handling Ready
Ready MustSync=true:
HardState Term:1 Vote:1 Commit:5
Entries:
1/4 EntryConfChange r1
1/5 EntryNormal "foo"
1/6 EntryNormal "bar"
CommittedEntries:
1/4 EntryConfChange r1
1/5 EntryNormal "foo"
Messages:
3->1 MsgAppResp Term:1 Log:0/4
3->1 MsgAppResp Term:1 Log:0/5
3->1 MsgAppResp Term:1 Log:0/6
3->1 MsgAppResp Term:1 Log:0/6
3->1 MsgAppResp Term:1 Log:0/6
INFO 3 switched to configuration voters=(2 3)
> 1 receiving messages
3->1 MsgAppResp Term:1 Log:0/4
3->1 MsgAppResp Term:1 Log:0/5
3->1 MsgAppResp Term:1 Log:0/6
3->1 MsgAppResp Term:1 Log:0/6
3->1 MsgAppResp Term:1 Log:0/6

# n1 can no longer propose.
propose 1 baz
----
INFO 1 no leader at term 1; dropping proposal
raft proposal dropped

# Nor can it campaign to become leader.
campaign 1
----
WARN 1 is unpromotable and can not campaign
4 changes: 0 additions & 4 deletions testdata/confchange_v2_replace_leader.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ raft-state
2: StateFollower (Voter) Term:1 Lead:1
3: StateFollower (Voter) Term:1 Lead:1

log-level info
----
ok

# create n4
add-nodes 1
----
Expand Down
Loading