Skip to content

Commit

Permalink
raft: consolidate the commited entry rewrite check
Browse files Browse the repository at this point in the history
The append function call right after this does the same check, we don't
need to do this in two places. TestLogMaybeAppend exercises these
panics, and confirms the behaviour is the same after the first panic is
removed.

Epic: none
Release note: none
  • Loading branch information
pav-kv committed Jun 27, 2024
1 parent cdfba20 commit efe2947
Showing 1 changed file with 3 additions and 6 deletions.
9 changes: 3 additions & 6 deletions pkg/raft/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,6 @@ func (l *raftLog) maybeAppend(a logSlice, committed uint64) (lastnewi uint64, ok
if !ok {
return 0, false
}
lastAppendedIndex := a.lastIndex()
if match.index < lastAppendedIndex && match.index < l.committed {
l.logger.Panicf("entry %d is already committed [committed(%d)]", match.index+1, l.committed)
}

// Fast-forward to the first mismatching or missing entry.
// NB: prev.index <= match.index <= a.lastIndex(), so the sub-slicing is safe.
Expand All @@ -124,6 +120,7 @@ func (l *raftLog) maybeAppend(a logSlice, committed uint64) (lastnewi uint64, ok
// bookkeeping in the unstable structure.
l.append(a.entries...)
// TODO(pav-kv): decouple commits from appends.
lastAppendedIndex := a.lastIndex()
l.commitTo(min(committed, lastAppendedIndex))
return lastAppendedIndex, true
}
Expand All @@ -132,8 +129,8 @@ func (l *raftLog) append(ents ...pb.Entry) uint64 {
if len(ents) == 0 {
return l.lastIndex()
}
if after := ents[0].Index - 1; after < l.committed {
l.logger.Panicf("after(%d) is out of range [committed(%d)]", after, l.committed)
if first := ents[0].Index; first <= l.committed {
l.logger.Panicf("entry %d is already committed [committed(%d)]", first, l.committed)
}
l.unstable.truncateAndAppend(ents)
return l.lastIndex()
Expand Down

0 comments on commit efe2947

Please sign in to comment.