cockroachdb / cockroachdb/cockroach
kvserver: AddSSTable crash recovery
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
### Issue
Most raft commands are applied to the state machine [atomically with](https://github.com/cockroachdb/cockroach/blob/2606875e8b0a8b9b0b75738d429a451b249f0dfb/pkg/kv/kvserver/replica_app_batch.go#L598-L604) bumping the `RaftAppliedIndex` (and [other](https://github.com/cockroachdb/cockroach/blob/2606875e8b0a8b9b0b75738d429a451b249f0dfb/pkg/kv/kvserver/stateloader/stateloader.go#L238-L242) things like closed timestamp). Except the `AddSSTable` commands: they are applied as Pebble [ingestion](https://github.com/cockroachdb/cockroach/blob/2606875e8b0a8b9b0b75738d429a451b249f0dfb/pkg/kv/kvserver/replica_proposal.go#L690-L748) first, and then separately there is a write batch.
Because of this non-atomicity, it’s possible that we ingest, then crash/restart, and end up with the SST applied but `RaftAppliedIndex` being stale (by more than one, since typical command applications don’t sync). After restart, the `Replica` is in an incorrect state (with "future" writes applied), until all commands are replayed up to this `AddSSTable`.
### Potential Fixes
Options:
1. Support atomic ingestion+batch in Pebble, and make `AddSSTable` application atomic. Considered complex.
2. Recover partially applied `AddSSTable` commands on restart. E.g. on applying `AddSSTable`, write/sync a batch saying that we’re about to apply a non-atomic command, and only then ingest the SST and write another batch indicating a finished command. On restart, detect a partially applied `AddSSTable` and finish it before the `Replica` is initialized.
3. Same as option 2, but no sync on the first batch. Requires support from Pebble (below).
4. There is another [approach](https://github.com/cockroachdb/cockroach/blob/546e170520fd6f5e61308371bf7eef1f6de84d6a/pkg/storage/replicas_storage.go#L507-L510) which requires broader “standalone” raft log replays on restarts. It wouldn’t incur an additional sync on `AddSSTable`, but is more complex.
5. Convert the `AddSSTable` command application into a multi-SST ingestion, i.e. convert the write batch (with `RaftAppliedIndex` etc) into an SST, and atomically ingest both.
6. Introduce a “flush barrier” on splits/merges. Say, when a range is split/merged, we write a no-op range key into Pebble, over the range’s whole “user” key space. Now, any AddSSTable into this range will cause a flush (because it overlaps this barrier), but at most once (only if it arrives before a flush). The next one will no longer overlap this barrier. If there are no AddSSTables (typical case), there is no extra sync/flush.
7. **(preferred)** The RSE project will introduce WAG [#149604], recording replica lifecycle events to the log engine before applying the state machine mutation; which can be replayed at restart. One of these instructions is "apply to index" for a range. It will allow nicely fixing this bug.
### Background / Details
Solution 2 requires an extra sync because Pebble currently doesn't guarantee "durability order" between an ingestion and a write batch that do not intersect by keys. Consider a scenario where we make a write, ingest, make another write; and, before we were able to sync the WAL, there is a crash. On restart, the two writes are dropped but the ingestion is applied.
It would be possible to avoid this sync (solution 3) if Pebble made ingestions behave exactly like a write batch w.r.t. durability: post-restart, there is a prefix of all writes/ingestions that is durable.
More details / discussion in a [thread](https://cockroachlabs.slack.com/archives/CAC6K3SLU/p1739209164941419).
Jira issue: CRDB-47622
Contributor guide
Research direction
Start with the AddSSTable application path in pkg/kv/kvserver/replica_proposal.go and the atomic state-machine updates in replica_app_batch.go, then review stateloader.go and the WAG proposal in issue #149604. Define the recovery behavior for a crash between SST ingestion and the RaftAppliedIndex update, and verify that restart no longer leaves future writes applied with a stale index.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100