Skip to content

Commit

Permalink
raft: don't emit unstable CommittedEntries
Browse files Browse the repository at this point in the history
See #14370.

When run in a single-voter configuration, prior to this PR
raft would emit `HardState`s that would emit a proposed `Entry`
simultaneously in `CommittedEntries` and `Entries`.

To be correct, this requires users of the raft library to enforce an
ordering between appending to the log and notifying the client about
`CommittedEntries` also present in `Entries`. This was easy to miss.

Walk back this behavior to arrive at a simpler contract: what's
emitted in `CommittedEntries` is truly committed, i.e. present
in stable storage on a quorum of voters.

This in turn pessimizes the single-voter case: rather than fully
handling an `Entry` in just one `Ready`, now two are required,
and in particular one has to do extra work to save on allocations.

We accept this as a good tradeoff, since raft primarily serves
multi-voter configurations.
  • Loading branch information
tbg committed Sep 1, 2022
1 parent eece843 commit a13a33f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
8 changes: 5 additions & 3 deletions raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,11 @@ func (r *raft) advance(rd Ready) {
if len(rd.Entries) > 0 {
e := rd.Entries[len(rd.Entries)-1]
r.raftLog.stableTo(e.Index, e.Term)
// If this is a single-voter configuration we need to simulate the leader
// sending an MsgAppResp to itself.
if r.id == r.lead && r.prs.IsSingleton() && r.prs.Progress[r.id].MaybeUpdate(e.Index) {
r.maybeCommit()
}
}
if !IsEmptySnap(rd.Snapshot) {
r.raftLog.stableSnapTo(rd.Snapshot.Metadata.Index)
Expand Down Expand Up @@ -635,9 +640,6 @@ func (r *raft) appendEntry(es ...pb.Entry) (accepted bool) {
}
// use latest "last" index after truncate/append
li = r.raftLog.append(es...)
r.prs.Progress[r.id].MaybeUpdate(li)
// Regardless of maybeCommit's return, our caller will call bcastAppend.
r.maybeCommit()
return true
}

Expand Down
5 changes: 4 additions & 1 deletion raft/testdata/single_node.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ stabilize
> 1 handling Ready
Ready MustSync=true:
Lead:1 State:StateLeader
HardState Term:1 Vote:1 Commit:4
HardState Term:1 Vote:1 Commit:3
Entries:
1/4 EntryNormal ""
> 1 handling Ready
Ready MustSync=false:
HardState Term:1 Vote:1 Commit:4
CommittedEntries:
1/4 EntryNormal ""

0 comments on commit a13a33f

Please sign in to comment.