cockroachdb / cockroachdb/cockroach
raft: incorrect term on delegated snapshots
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
After #105044, the delegate can generate a snapshot while being at a [newer term](https://github.com/cockroachdb/cockroach/blob/c557fb59f6aec659d364e9002fc083c59c6392b6/pkg/kv/kvserver/replica_command.go#L3134) than the coordinator, but in the `MsgSnap` message it puts the [Term of the coordinator](https://github.com/cockroachdb/cockroach/blob/c557fb59f6aec659d364e9002fc083c59c6392b6/pkg/kv/kvserver/replica_command.go#L3328), as if it was sent by that leader. This is problematic because the delegate can be already applying entries committed at later terms, and as a result the `MsgSnap` can carry `Term < snapshot.Metadata.Term`. On the receiver who applies this snapshot, it can break the raft log/state invariant: `HardState.Term >= lastEntryTerm >= RaftTruncatedState.Term`. Additionally, the receiver of the snapshot assumes [#127348] the sender of the message is the leader.
To fix this, we should seek to guarantee an invariant on `MsgSnap`:
```
Term >= the term at which the entries in the snapshot were committed.
```
One way to do this is to put the delegate's term instead of the originator. Because of #127348 though, the `MsgSnap` should be sent on behalf of a leader at this term. We can consult the `SoftState.Lead` field for this.
Another option is to reject snapshot delegations if our term differs. One problem with this is that the snapshot could have been initiated by a leaseholder who was not a leader. Looks like we need to consult `SoftState.Lead` in this case too.
Jira issue: CRDB-40402
Contributor guide
Assessment
This issue has not been assessed yet.