cockroachdb / cockroachdb/cockroach
raft: handle snapshots with outdated term
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
Before stepping `MsgSnap` to raft, we [bump](https://github.com/cockroachdb/cockroach/blob/c557fb59f6aec659d364e9002fc083c59c6392b6/pkg/kv/kvserver/replica_raft.go#L618-L631) its term to the receiver `Term` (to force through its term checks). This would have been fine (because the snapshot carries committed state and can always be handled), but raft assumes the snapshot was sent by the `MsgSnap.Term` leader, and updates the state accordingly [[1](https://github.com/cockroachdb/cockroach/blob/c557fb59f6aec659d364e9002fc083c59c6392b6/pkg/raft/raft.go#L1100-L1101), [2](https://github.com/cockroachdb/cockroach/blob/c557fb59f6aec659d364e9002fc083c59c6392b6/pkg/raft/raft.go#L1668)]. Since the `MsgSnap` term could have been bumped arbitrarily, these transitions can be incorrect: we will falsely believe that the snapshot originator is a leader at a different term.
The `lead` field is used for a bunch of things:
- if `lead != None`, we [can’t vote](https://github.com/cockroachdb/cockroach/blob/c557fb59f6aec659d364e9002fc083c59c6392b6/pkg/raft/raft.go#L1190)
- proposals are [forwarded](https://github.com/cockroachdb/cockroach/blob/c557fb59f6aec659d364e9002fc083c59c6392b6/pkg/raft/raft.go#L1649-L1657) to this lead, so can travel some and then be dropped / never reach the actual leader
- the lead field is [surfaced](https://github.com/cockroachdb/cockroach/blob/c557fb59f6aec659d364e9002fc083c59c6392b6/pkg/raft/raft.go#L493) through the `SoftState` to CRDB, and we probably have some logic relying on it being correct.
- Upd: the `lead` field is now in `HardState` and has a role in the correctness-sensitive liveness / fortification / leader leases protocols.
To fix this case, we need to remove this term bump, and allow raft to handle snapshots at outdated terms. A snapshot can always be applied because it carries committed state that is not reversible; except when it carries a committed state that we already have, but it’s easy to check.
Jira issue: CRDB-40401
Contributor guide
Assessment
This issue has not been assessed yet.