cockroachdb / cockroachdb/cockroach

kvserver: follower read below a snapshot's GC threshold is served from collected data

Open
#173,315 0 comments 0 reactions 0 assignees View on GitHub
A-kv-replication branch-master C-bug O-agent T-kv-replication
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.