etcd-io / raft

Raft library for maintaining a replicated state machine
Apache License 2.0
666 stars 164 forks source link

Clean up and improve snapshot handling #124

Open erikgrinaker opened 10 months ago

erikgrinaker commented 10 months ago

In #110, we made some minor changes to snapshot handling, which removed the main use of Progress.PendingSnapshot. As described in https://github.com/etcd-io/raft/pull/110#discussion_r1405420442, we should now be able to remove PendingSnapshot completely, and take the opportunity to clean up some of the related state transitions.

Also, as described in https://github.com/cockroachdb/cockroach/issues/87583 and https://github.com/etcd-io/raft/pull/110#discussion_r1442776945, we should modify ReportSnapshot() and MsgSnapStatus to take the index of the applied snapshot -- in the common case, this will be the same as that in MsgSnap, but complex systems (e.g. CockroachDB) may generate and apply snapshots at a different index than that requested by Raft (e.g. to send snapshots from a follower in the same region), and reporting back the actual index would simplify the logic and e.g. allow updating Match and transitioning directly to StateReplicate in this case.

These are all breaking changes to public APIs, so would have to be done for 4.0. There are possibly other related cleanups that could be made as well.

@pav-kv @ahrtr Please add any further ideas and context as needed.