cockroachdb / cockroachdb/cockroach

raft: incorrect term on delegated snapshots

Open
#127,349 1 comment 0 reactions 1 assignee Claimed by @pav-kv View on GitHub
A-kv-replication branch-master C-bug T-kv
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.