raft: don't apply entries when applying snapshot #14721
Conversation
Codecov Report
@@ Coverage Diff @@
## main #14721 +/- ##
=======================================
Coverage 76.04% 76.04%
=======================================
Files 457 457
Lines 37350 37350
=======================================
Hits 28403 28403
Misses 7162 7162
Partials 1785 1785
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Give me a ping when it's rebased, I'd like to glance at the diff again before merge. Thanks! |
unstable.snapshot is never an empty snapshot. This check made it look like it could be. Signed-off-by: Nathan VanBenschoten <[email protected]>
This commit removes the ability to apply log entries at the same time as applying a snapshot. Doing so it possible, but it leads to complex code and raises questions about what should be applied first. It also raises additional complexity when we start allowing concurrent, asynchronous log appends and log application. It's easiest to just disallow this. Signed-off-by: Nathan VanBenschoten <[email protected]>
nvanbenschoten
force-pushed
the
nvanbenschoten/noCommittedOnSnap
branch
from
a3c7480 to
539a841
Compare
|
Rebased on main now that #14719 is merged. PTAL! |
| @@ -181,9 +181,13 @@ func (l *raftLog) unstableEntries() []pb.Entry { | |||
| // If applied is smaller than the index of snapshot, it returns all committed | |||
| if l.committed+1 > off { | ||
| ents, err := l.slice(off, l.committed+1, l.maxNextCommittedEntsSize) | ||
| if l.hasPendingSnapshot() { | ||
| // See comment in hasNextCommittedEnts. |
There was a problem hiding this comment.
You could even structure this method as
- call hasNextCommittedEnts; if false, return early.
- otherwise, do the slice.
That way there's no duplication in logic between the two and one need not worry about them diverging via a future change.
This commit removes the ability to apply log entries at the same time as applying a snapshot. Doing so it possible, but it leads to complex code and raises questions about what should be applied first. It also raises additional complexity when we start allowing concurrent, asynchronous log appends and log application. It's easiest to just disallow this.
Signed-off-by: Nathan VanBenschoten [email protected]
Extracted from #14627.
cc. @tbg
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.