Skip to content

Commit 6146578

Browse files
craig[bot]pav-kv
andcommitted
Merge #144116
144116: raft: do not set self-match on becoming leader r=iskettaneh a=pav-kv This fixes a "nearly" bug. The contract of `Progress.Match` is that the index is durable. When becoming leader, there is no guarantee that `raftLog.lastIndex()` is durable locally. We could have got a quorum of votes, and yet the local storage hasn't voted/synced yet. We only know that a log index is durable when handling a self-`MsgAppResp` or a storage append ack. This wasn't a real bug because `raftLog.lastIndex()` is an entry from previous terms, and the first entry at our leader's term comes after it. A leader can only commit entries from its own term (see `maybeCommit`), so this opportunistically set `Match` could not cause false-positive commits. But could be a bug if this `lastIndex` included the dummy entry, which is appended a few instructions down the line. There is no value in setting `Match` for the leader this way, and it appears cleaner that `Match` is initialized empty and only updated by `MsgAppResp`, symmetrically for all peers including self. For the leader, it will be first updated when the dummy entry makes a durable roundtrip to local storage. Epic: none Release note: none Co-authored-by: Pavel Kalinnikov <[email protected]>
2 parents 17e9fdd + 720709b commit 6146578

File tree

3 files changed

+13
-4
lines changed

3 files changed

+13
-4
lines changed

pkg/raft/raft.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,9 +1071,6 @@ func (r *raft) reset(term uint64) {
10711071
Inflights: tracker.NewInflights(r.maxInflight, r.maxInflightBytes),
10721072
IsLearner: pr.IsLearner,
10731073
}
1074-
if id == r.id {
1075-
pr.Match = r.raftLog.lastIndex()
1076-
}
10771074
})
10781075

10791076
r.pendingConfIndex = 0

pkg/raft/testdata/campaign.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,3 +133,15 @@ stabilize
133133
> 1 receiving messages
134134
2->1 MsgAppResp Term:1 Log:0/3 Commit:3
135135
3->1 MsgAppResp Term:1 Log:0/3 Commit:3
136+
137+
raft-state
138+
----
139+
1: StateLeader (Voter) Term:1 Lead:1 LeadEpoch:1
140+
2: StateFollower (Voter) Term:1 Lead:1 LeadEpoch:1
141+
3: StateFollower (Voter) Term:1 Lead:1 LeadEpoch:1
142+
143+
status 1
144+
----
145+
1: StateReplicate match=3 next=4 sentCommit=2 matchCommit=2
146+
2: StateReplicate match=3 next=4 sentCommit=3 matchCommit=3
147+
3: StateReplicate match=3 next=4 sentCommit=3 matchCommit=3

pkg/raft/tracker/progress.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import (
3838
// TODO(pav-kv): consolidate all flow control state changes here. Much of the
3939
// transitions in raft.go logically belong here.
4040
type Progress struct {
41-
// Match is the index up to which the follower's log is known to match the
41+
// Match is the index up to which the peer's log is known to durably match the
4242
// leader's.
4343
Match uint64
4444
// Next is the log index of the next entry to send to this follower. All

0 commit comments

Comments
 (0)