cockroachdb / cockroachdb/cockroach
kvserver: follower read below a snapshot's GC threshold is served from collected data
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
A follower read whose timestamp is below the GC threshold of a snapshot the replica is applying can be served from data the snapshot's sender had already garbage collected. The read succeeds and silently returns an incomplete result, where it is owed a `BatchTimestampBeforeGCError`.
**Root cause**
Read-only requests pin engine state and then validate that pinned state by checking the read's timestamp against the replica's in-memory GC threshold — `Replica.checkExecutionCanProceedAfterStorageSnapshot` → [`checkTSAboveGCThreshold`](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/kv/kvserver/replica.go#L2233), reading [`r.shMu.state.GCThreshold`](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/kv/kvserver/replica.go#L2202). The check is deliberately post-pin: GC requests don't acquire latches at the timestamps they collect, so read traffic at or around the threshold is not serialized against them.
That ordering is only sound if the in-memory threshold is bumped *before* the data it describes is removed. The GC command path upholds it — `GCThreshold` is applied as a [pre-apply side effect](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/kv/kvserver/replica_app_batch.go#L401), and the comment there spells out the invariant readers depend on.
Snapshot application does not. The snapshot carries data the sender collected up to the snapshot's GC threshold, but the receiving replica publishes that threshold only after the data lands: the ingest commits in [`applySnapshotRaftMuLocked`](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/kv/kvserver/replica_raftstorage.go#L644), and the snapshot's `ReplicaState` — `GCThreshold` included — is swapped in [afterwards](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/kv/kvserver/replica_raftstorage.go#L760). In between, the engine holds the collected data while the in-memory threshold is still the old, lower value. A read with a timestamp between the two thresholds that pins engine state inside that window passes the check against the stale threshold and scans versions the snapshot no longer carries.
The window is entered whenever a replica catches up by snapshot from a sender that has collected more than it has — ordinary for a follower that was partitioned, slow, or restarted while the range was garbage collected.
Contributor guide
Research direction
Start in pkg/kv/kvserver/replica.go with checkExecutionCanProceedAfterStorageSnapshot and checkTSAboveGCThreshold, then trace snapshot handling through applySnapshotRaftMuLocked and ReplicaState publication in replica_raftstorage.go. Compare this with the GCThreshold pre-apply side effect in replica_app_batch.go. Done means a follower read below the incoming snapshot's GC threshold returns BatchTimestampBeforeGCError instead of an incomplete result.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100