Skip to content

Commit

Permalink
raft: stop broadcasting MsgDeFortifyLeader once it is safe
Browse files Browse the repository at this point in the history 
This patch identifies the scenario where a leader can safely stop
de-fortifying its peers -- when a new leader has been elected and has
been able to commit an entry.

Informs #132581

Release note: None
  • Loading branch information
@arulajmani
arulajmani committed Oct 14, 2024
1 parent f5dd8bf commit e2287a7
Show file tree
Hide file tree
Showing 2 changed files with 314 additions and 5 deletions.
23 changes: 19 additions & 4 deletions pkg/raft/raft.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,16 @@ func (r *raft) sendDeFortify(to pb.PeerID) {
if to == r.id {
// We handle the case where the leader is trying to de-fortify itself
// specially. Doing so avoids a self-addressed message.
r.deFortify(r.id, r.fortificationTracker.Term())
switch {
case r.Term == r.fortificationTracker.Term():
r.deFortify(r.id, r.fortificationTracker.Term())
case r.Term > r.fortificationTracker.Term():
r.logger.Debugf("de-foritfying self at term %d is a no-op; current term %d",
r.fortificationTracker.Term(), r.Term,
)
case r.Term < r.fortificationTracker.Term():
panic("fortification tracker's term cannot be higher than raft groups")
}
return
}
r.send(pb.Message{To: to, Type: pb.MsgDeFortifyLeader, Term: r.fortificationTracker.Term()})
Expand Down Expand Up @@ -892,9 +901,15 @@ func (r *raft) bcastDeFortify() {
// MsgDeFortifyLeader to all peers or not.
func (r *raft) shouldBcastDeFortify() bool {
assertTrue(r.state != pb.StateLeader, "leaders should not be de-fortifying without stepping down")
// TODO(arul): expand this condition to ensure a new leader has committed an
// entry.
return r.fortificationTracker.FortificationEnabled() && r.fortificationTracker.CanDefortify()
hasNewLeaderCommittedEntry := false
committedTerm, err := r.raftLog.storage.Term(r.raftLog.committed)
if err == nil && committedTerm > r.fortificationTracker.Term() {
// NB: If there's an error getting the committed term, we must
// conservatively assume a new leader hasn't committed an entry. As a
// result, we'll still send out a MsgDeFortifyLeader, if it's safe to do so
hasNewLeaderCommittedEntry = true
}
return r.fortificationTracker.FortificationEnabled() && r.fortificationTracker.CanDefortify() && !hasNewLeaderCommittedEntry
}

// maybeUnpauseAndBcastAppend unpauses and attempts to send an MsgApp to all the
Expand Down
296 changes: 295 additions & 1 deletion pkg/raft/testdata/de_fortification_checkquorum.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,30 @@ raft-state
2: StateFollower (Voter) Term:1 Lead:1 LeadEpoch:0
3: StateFollower (Voter) Term:1 Lead:1 LeadEpoch:0

# And as a result, 2 is able to win the election.
# A new leader hasn't been elected yet, and by extension, no new entry has been
# committed yet. As a result, 1 should continue to broadcast a
# MsgDeFortifyLeader on every hertbeat tick.
tick-heartbeat 1
----
ok

stabilize
----
> 1 handling Ready
Ready MustSync=false:
Messages:
1->2 MsgDeFortifyLeader Term:1 Log:0/0
1->3 MsgDeFortifyLeader Term:1 Log:0/0
> 2 receiving messages
1->2 MsgDeFortifyLeader Term:1 Log:0/0
DEBUG 2 is not fortifying 1; de-fortification is a no-op
> 3 receiving messages
1->3 MsgDeFortifyLeader Term:1 Log:0/0
DEBUG 3 is not fortifying 1; de-fortification is a no-op


# But because all peers have de-fortified at this point, 2 is able to call and
# win the election.
campaign 2
----
INFO 2 is starting a new election at term 1
Expand All @@ -159,3 +182,274 @@ raft-state
1: StateFollower (Voter) Term:2 Lead:2 LeadEpoch:1
2: StateLeader (Voter) Term:2 Lead:2 LeadEpoch:1
3: StateFollower (Voter) Term:2 Lead:2 LeadEpoch:1

tick-heartbeat 1
----
ok

# Note that 1 does not send a MsgDeFortifyLeader now that 2 has committed an
# entry.
stabilize
----
ok

# Lastly, we'll create a scenario where 2 steps down because of check quorum and
# a new leader is elected as a result, but 2 doesn't hear about this. It should
# continue to broadcast MsgDeFortifyLeader until it hears about this (i.e. it
# learns of an entry committed by a new leader).

# Usual steps -- set randomized timeout for the benefit of the test, tick an
# election twice, and expire support to allow step down.
set-randomized-election-timeout 2 timeout=3
----
ok

withdraw-support 1 2
----
1 2 3
1 1 x 1
2 x 1 1
3 x 1 1

withdraw-support 3 2
----
1 2 3
1 1 x 1
2 x 1 1
3 x x 1

support-expired 2
----
ok

tick-election 2
----
INFO 2 leader at term 2 does not support itself in the liveness fabric
INFO 2 leader at term 2 does not support itself in the liveness fabric
DEBUG 2 does not have store liveness support from a quorum of peers
INFO 2 leader at term 2 does not support itself in the liveness fabric

tick-election 2
----
INFO 2 leader at term 2 does not support itself in the liveness fabric
INFO 2 leader at term 2 does not support itself in the liveness fabric
DEBUG 2 has not received messages from a quorum of peers in the last election timeout
DEBUG 2 does not have store liveness support from a quorum of peers
WARN 2 stepped down to follower since quorum is not active
INFO 2 became follower at term 2

# Tick 1 once to ensure it grants its prevote.
set-randomized-election-timeout 1 timeout=5
----
ok

tick-heartbeat 1
----
DEBUG 1 setting election elapsed to start from 3 ticks after store liveness support expired

campaign 3
----
INFO 3 is starting a new election at term 2
INFO 3 became pre-candidate at term 2
INFO 3 [logterm: 2, index: 12] sent MsgPreVote request to 1 at term 2
INFO 3 [logterm: 2, index: 12] sent MsgPreVote request to 2 at term 2

stabilize 3
----
> 3 handling Ready
Ready MustSync=true:
State:StatePreCandidate
HardState Term:2 Commit:12 Lead:0 LeadEpoch:0
Messages:
3->1 MsgPreVote Term:3 Log:2/12
3->2 MsgPreVote Term:3 Log:2/12
INFO 3 received MsgPreVoteResp from 3 at term 2
INFO 3 has received 1 MsgPreVoteResp votes and 0 vote rejections

deliver-msgs drop=(2)
----
dropped: 3->2 MsgPreVote Term:3 Log:2/12

stabilize 1 3
----
> 1 handling Ready
Ready MustSync=true:
HardState Term:2 Vote:2 Commit:12 Lead:2 LeadEpoch:0
> 1 receiving messages
3->1 MsgPreVote Term:3 Log:2/12
INFO 1 [logterm: 2, index: 12, vote: 2] cast MsgPreVote for 3 [logterm: 2, index: 12] at term 2
> 1 handling Ready
Ready MustSync=false:
Messages:
1->3 MsgPreVoteResp Term:3 Log:0/0
> 3 receiving messages
1->3 MsgPreVoteResp Term:3 Log:0/0
INFO 3 received MsgPreVoteResp from 1 at term 2
INFO 3 has received 2 MsgPreVoteResp votes and 0 vote rejections
INFO 3 became candidate at term 3
INFO 3 [logterm: 2, index: 12] sent MsgVote request to 1 at term 3
INFO 3 [logterm: 2, index: 12] sent MsgVote request to 2 at term 3
> 3 handling Ready
Ready MustSync=true:
State:StateCandidate
HardState Term:3 Vote:3 Commit:12 Lead:0 LeadEpoch:0
Messages:
3->1 MsgVote Term:3 Log:2/12
3->2 MsgVote Term:3 Log:2/12
INFO 3 received MsgVoteResp from 3 at term 3
INFO 3 has received 1 MsgVoteResp votes and 0 vote rejections
> 1 receiving messages
3->1 MsgVote Term:3 Log:2/12
INFO 1 [term: 2] received a MsgVote message with higher term from 3 [term: 3]
INFO 1 became follower at term 3
INFO 1 [logterm: 2, index: 12, vote: 0] cast MsgVote for 3 [logterm: 2, index: 12] at term 3
> 1 handling Ready
Ready MustSync=true:
HardState Term:3 Vote:3 Commit:12 Lead:0 LeadEpoch:0
Messages:
1->3 MsgVoteResp Term:3 Log:0/0
> 3 receiving messages
1->3 MsgVoteResp Term:3 Log:0/0
INFO 3 received MsgVoteResp from 1 at term 3
INFO 3 has received 2 MsgVoteResp votes and 0 vote rejections
INFO 3 became leader at term 3
> 3 handling Ready
Ready MustSync=true:
State:StateLeader
HardState Term:3 Vote:3 Commit:12 Lead:3 LeadEpoch:1
Entries:
3/13 EntryNormal ""
Messages:
3->1 MsgFortifyLeader Term:3 Log:0/0
3->2 MsgFortifyLeader Term:3 Log:0/0
3->1 MsgApp Term:3 Log:2/12 Commit:12 Entries:[3/13 EntryNormal ""]
3->2 MsgApp Term:3 Log:2/12 Commit:12 Entries:[3/13 EntryNormal ""]
> 1 receiving messages
3->1 MsgFortifyLeader Term:3 Log:0/0
3->1 MsgApp Term:3 Log:2/12 Commit:12 Entries:[3/13 EntryNormal ""]
> 1 handling Ready
Ready MustSync=true:
HardState Term:3 Vote:3 Commit:12 Lead:3 LeadEpoch:1
Entries:
3/13 EntryNormal ""
Messages:
1->3 MsgFortifyLeaderResp Term:3 Log:0/0 LeadEpoch:1
1->3 MsgAppResp Term:3 Log:0/13 Commit:12
> 3 receiving messages
1->3 MsgFortifyLeaderResp Term:3 Log:0/0 LeadEpoch:1
1->3 MsgAppResp Term:3 Log:0/13 Commit:12
> 3 handling Ready
Ready MustSync=true:
HardState Term:3 Vote:3 Commit:13 Lead:3 LeadEpoch:1
CommittedEntries:
3/13 EntryNormal ""
Messages:
3->1 MsgApp Term:3 Log:3/13 Commit:13
> 1 receiving messages
3->1 MsgApp Term:3 Log:3/13 Commit:13
> 1 handling Ready
Ready MustSync=true:
HardState Term:3 Vote:3 Commit:13 Lead:3 LeadEpoch:1
CommittedEntries:
3/13 EntryNormal ""
Messages:
1->3 MsgAppResp Term:3 Log:0/13 Commit:13
> 3 receiving messages
1->3 MsgAppResp Term:3 Log:0/13 Commit:13

deliver-msgs drop=(2)
----
dropped: 3->2 MsgVote Term:3 Log:2/12
dropped: 3->2 MsgFortifyLeader Term:3 Log:0/0
dropped: 3->2 MsgApp Term:3 Log:2/12 Commit:12 Entries:[3/13 EntryNormal ""]

# 3 has been elected leader and has committed an entry. 2 is obilivious to this.
raft-state
----
1: StateFollower (Voter) Term:3 Lead:3 LeadEpoch:1
2: StateFollower (Voter) Term:2 Lead:0 LeadEpoch:1
3: StateLeader (Voter) Term:3 Lead:3 LeadEpoch:1

raft-log 3
----
1/11 EntryNormal ""
2/12 EntryNormal ""
3/13 EntryNormal ""

raft-log 2
----
1/11 EntryNormal ""
2/12 EntryNormal ""

# And as a result, it continues to send out de-fortification requests.
tick-heartbeat 2
----
ok

stabilize
----
> 2 handling Ready
Ready MustSync=true:
State:StateFollower
HardState Term:2 Vote:2 Commit:12 Lead:2 LeadEpoch:0
Messages:
2->1 MsgDeFortifyLeader Term:2 Log:0/0
2->3 MsgDeFortifyLeader Term:2 Log:0/0
> 1 receiving messages
2->1 MsgDeFortifyLeader Term:2 Log:0/0
INFO 1 [term: 3] ignored a MsgDeFortifyLeader message with lower term from 2 [term: 2]
> 3 receiving messages
2->3 MsgDeFortifyLeader Term:2 Log:0/0
INFO 3 [term: 3] ignored a MsgDeFortifyLeader message with lower term from 2 [term: 2]

tick-heartbeat 3
----
ok

# Catch 2 up.
stabilize
----
> 3 handling Ready
Ready MustSync=false:
Messages:
3->2 MsgFortifyLeader Term:3 Log:0/0
3->2 MsgApp Term:3 Log:2/12 Commit:13 Entries:[3/13 EntryNormal ""]
> 2 receiving messages
3->2 MsgFortifyLeader Term:3 Log:0/0
INFO 2 [term: 2] received a MsgFortifyLeader message with higher term from 3 [term: 3]
INFO 2 became follower at term 3
3->2 MsgApp Term:3 Log:2/12 Commit:13 Entries:[3/13 EntryNormal ""]
> 2 handling Ready
Ready MustSync=true:
HardState Term:3 Commit:13 Lead:3 LeadEpoch:1
Entries:
3/13 EntryNormal ""
CommittedEntries:
3/13 EntryNormal ""
Messages:
2->3 MsgFortifyLeaderResp Term:3 Log:0/0 LeadEpoch:1
2->3 MsgAppResp Term:3 Log:0/13 Commit:13
> 3 receiving messages
2->3 MsgFortifyLeaderResp Term:3 Log:0/0 LeadEpoch:1
2->3 MsgAppResp Term:3 Log:0/13 Commit:13

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

raft-log 2
----
1/11 EntryNormal ""
2/12 EntryNormal ""
3/13 EntryNormal ""

# And now, 2 shouldn't send any de-fortification requests.
tick-heartbeat 2
----
ok

stabilize
----
ok

0 comments on commit e2287a7

Please sign in to comment.