cockroachdb / cockroachdb/cockroach

kvserver: rangefeed lag observer cancel timer survives processor teardown

Open
#172,417 0 comments 0 reactions 0 assignees View on GitHub
A-kv-rangefeed branch-master C-bug O-agent P-2 T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**

The lag observer's `exceedsCancelLagStartTime` is only reset by an *observation* with lag below the cancel threshold ([replica_rangefeed_lag_observer.go#L114-L123](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/replica_rangefeed_lag_observer.go#L114-L123)), and observations only happen while a processor is attached — `handleClosedTimestampUpdateRaftMuLocked` returns before observing when there is no processor ([replica_rangefeed.go#L881-L884](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/replica_rangefeed.go#L881-L884)). But the observer lives on the Replica (created once in `replica_init.go`), not on the processor, so the timer silently spans distinct lag episodes and distinct rangefeed generations:

1. Feed A is active. At t0 the closed-timestamp lag exceeds the cancel threshold; the observer sets `exceedsCancelLagStartTime = t0`.
2. At t0+10s feed A disconnects for an unrelated retryable reason. The processor is removed; observer state is untouched.
3. The range recovers at t0+20s. With no processor attached, the below-threshold lag is never observed and the timer never resets.
4. Hours later a new lag episode begins and feed B registers. Registration immediately observes lag above the threshold with the stale t0 start time: `now - t0` far exceeds `cancelLagMinDuration`, so the observation reports `exceedsCancelLagThreshold = true`.
5. The nudge succeeds and feed B is cancelled with `REASON_RANGEFEED_CLOSED` on essentially its first closed-timestamp update — violating the contract that lag must be sustained for at least `kv.rangefeed.lagging_closed_timestamp_cancel_min_lagging_duration`.

The impact is a retryable error and replanning churn rather than data loss, but it is exactly the false-positive cancel the duration gate was designed to prevent.

**Related hardening in the same mechanism**

The async nudge closure ([replica_rangefeed.go#L913-L950](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/replica_rangefeed.go#L913-L950)) captures its cancel/nudge decision at observation time, may then wait on the nudge semaphore plus up to 5s in `ensureClosedTimestampStarted`, and disconnects without re-checking the current lag or whether the observed processor is still the one attached. It can therefore cancel a recovered or replaced processor, compounding the stale-timer problem above. Re-deriving the lag inside the closure before disconnecting would close this.

**Expected behavior**

The sustained-lag timer should track a single processor's lag episode. A new rangefeed should never inherit cancel-eligibility from a lag episode that ended before it existed.

**Suggested fix**

Reset the observer whenever the processor is set/unset on the replica (`setRangefeedProcessor` / `unsetRangefeedProcessorLocked`), or move the observer state to be per-processor.

**Additional context**

Found during an agent-assisted correctness audit of the rangefeed subsystem. Code links are pinned to master @ a7e1178.

Jira issue: CRDB-65653

Contributor guide

Open the contributing guide

Research direction

Start in pkg/kv/kvserver/replica_rangefeed_lag_observer.go and inspect setRangefeedProcessor, unsetRangefeedProcessorLocked, and the observation paths in replica_rangefeed.go. Trace processor teardown and registration, then run the relevant kvserver rangefeed tests and add coverage showing that a new processor cannot inherit stale lag state or be cancelled after recovery or replacement. Done means the cancel duration applies only to the current processor's sustained lag episode.

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
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.