Skip to content

Commit 2584c41

Browse files
committed
raft: advance commit index safely
This change makes the commit index advancement in handleHeartbeat safe. Previously, a follower would attempt to update the commit index to whichever was sent in the MsgHeartbeat message. Out-of-bound indices would crash the node. It is always safe to advance a commit index if the follower's log is "in sync" with the leader, i.e. when its log is guaranteed to be a prefix of the leader's log. This becomes true when the first MsgApp append message succeeds. At the moment, the leader will never send a commit index that exceeds the follower's log size. However, this may change in future. This change is a defence-in-depth. Signed-off-by: Pavel Kalinnikov <[email protected]>
1 parent 026484c commit 2584c41

File tree

1 file changed

+13
-1
lines changed

1 file changed

+13
-1
lines changed

raft.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,11 @@ type raft struct {
375375

376376
// the leader id
377377
lead uint64
378+
// logSynced is true if this node's log is guaranteed to be a prefix of the
379+
// leader's log at this term. Always true for the leader. Always false for a
380+
// candidate. For a follower, becomes true the first time a MsgApp append to
381+
// the log succeeds.
382+
logSynced bool
378383
// leadTransferee is id of the leader transfer target when its value is not zero.
379384
// Follow the procedure defined in raft thesis 3.10.
380385
leadTransferee uint64
@@ -763,6 +768,7 @@ func (r *raft) reset(term uint64) {
763768
r.Vote = None
764769
}
765770
r.lead = None
771+
r.logSynced = false
766772

767773
r.electionElapsed = 0
768774
r.heartbeatElapsed = 0
@@ -908,6 +914,7 @@ func (r *raft) becomeLeader() {
908914
r.reset(r.Term)
909915
r.tick = r.tickHeartbeat
910916
r.lead = r.id
917+
r.logSynced = true // the leader's log is in sync with itself
911918
r.state = StateLeader
912919
// Followers enter replicate mode when they've been successfully probed
913920
// (perhaps after having received a snapshot as a result). The leader is
@@ -1735,6 +1742,7 @@ func (r *raft) handleAppendEntries(m pb.Message) {
17351742
return
17361743
}
17371744
if mlastIndex, ok := r.raftLog.maybeAppend(m.Index, m.LogTerm, m.Commit, m.Entries...); ok {
1745+
r.logSynced = true // from now on, the log is a prefix of the leader's log
17381746
r.send(pb.Message{To: m.From, Type: pb.MsgAppResp, Index: mlastIndex})
17391747
return
17401748
}
@@ -1770,7 +1778,11 @@ func (r *raft) handleAppendEntries(m pb.Message) {
17701778
}
17711779

17721780
func (r *raft) handleHeartbeat(m pb.Message) {
1773-
r.raftLog.commitTo(m.Commit)
1781+
// If our log is not a prefix of the leader's log, it is unsafe to advance the
1782+
// commit index, because the entries at this index may mismatch.
1783+
if r.logSynced {
1784+
r.raftLog.commitTo(min(m.Commit, r.raftLog.lastIndex()))
1785+
}
17741786
r.send(pb.Message{To: m.From, Type: pb.MsgHeartbeatResp, Context: m.Context})
17751787
}
17761788

0 commit comments

Comments
 (0)