hashicorp/raft

Difference between commit Index condition in official paper and implementation

hqt opened this issue · 1 comments

hqt commented

I read the official Raft paper for the condition to update the commitIndex at the leader

If there exists an N such that N > commitIndex, a majority of matchIndex[i] ≥ N, and log[N].term == currentTerm: set commitIndex = N

The main point here is: We must have the condition log[N].term == currentTerm, otherwise, there is a situation mentioned in the paper that we might have an issue

Screenshot 2021-02-15 at 2 59 16 AM

When I read the source code here, however it seems the condition isn't there. Can someone point me some light about this?

Howdy @hqt,
Thank you for this question! Well spotted, we had to think about this for a bit to see if it was correct.
First, a new commitment object is started each time a new leader is elected and a new term is started. With this, a new StartIndex is created which is the first index within that term. This means according to this statement, no entries that are less than the StartIndex will be considered committed.

raft/commitment.go

Lines 97 to 99 in f7242e7

if quorumMatchIndex > c.commitIndex && quorumMatchIndex >= c.startIndex {
c.commitIndex = quorumMatchIndex
asyncNotifyCh(c.commitCh)

The leader will only call Match() will only call once it has written the log to its own disk, and when it has received an acknowledgement from a follower for the AppendEntries RPC. Which means it knows that the log was committed to a quorum of nodes during its leadership term.

raft/replication.go

Lines 603 to 613 in f7242e7

func updateLastAppended(s *followerReplication, req *AppendEntriesRequest) {
// Mark any inflight logs as committed
if logs := req.Entries; len(logs) > 0 {
last := logs[len(logs)-1]
atomic.StoreUint64(&s.nextIndex, last.Index+1)
s.commitment.match(s.peer.ID, last.Index)
}
// Notify still leader
s.notifyAll(true)
}

I hope this answers your question :)