-
Notifications
You must be signed in to change notification settings - Fork 4.1k
storage: better testing of applySnapshot #7737
Description
From #7598 via @petermattis:
I was referring to applySnapshot in general. It looks like it is only tested indirectly, but I can't see a reason for that. It's not necessary to address in this PR, but we should file an issue to thoroughly test all of the applySnapshot possibilities (preemptive vs non-preemptive, existing HardState vs empty HardState, etc).
Also in this PR via @bdarnell:
etcd/raft doesn't appear to have a check like this [preventing regression of last log index]. I think they assume that snapshots only transfer the state machine and don't touch the logs. They do, however, drop any snapshots from older terms.
Dropping snapshots from older terms is an option, though we could accept them We should revisit this as we try to pass all snapshots through Raft (in which case we'll likely want to apply any snapshot Raft gives to us simply as not to interfere with it).