cockroachdb / cockroachdb/cockroach

kvserver: rangefeed registration can panic if the init resolved-ts scan fails

Open
#172,416 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**

`registerWithRangefeedRaftMuLocked` assumes that a failed `Register` on a freshly created processor can only mean the stopper is quiescing, and panics otherwise ([replica_rangefeed.go#L553-L576](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/replica_rangefeed.go#L553-L576)). But the processor can also stop itself when the async initial resolved-timestamp scan fails, which makes the panic reachable on an ordinary storage read error:

1. `registerWithRangefeedRaftMuLocked` creates a new processor. `p.Start` ([scheduled_processor.go#L114](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/rangefeed/scheduled_processor.go#L114)) constructs the intent scanner synchronously, launches `initResolvedTSScan.Run` as an async task, and returns nil.
2. The init scan's `LockTableIterator` hits a read error (bad sector, block corruption).
3. `initResolvedTSScan.Run` ([task.go#L58-L70](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/rangefeed/task.go#L58-L70)) calls `p.StopWithErr(err)`, which enqueues a stop request; the scheduler processes it and sets `p.stopping`.
4. Meanwhile the replica goroutine (still holding raftMu) calls `p.Register`. The processor request queue is FIFO and the stop request was enqueued first, so the register closure observes `p.stopping.Load() == true` and returns false.
5. The stopper is not quiescing, so the `default` branch panics — while holding raftMu — killing the node.

If the register request wins the race instead, the registration succeeds and is then disconnected with the scan error, a clean retryable outcome. Only the stop-first ordering panics. The window is microseconds wide, which is presumably why this has not been seen in the wild — but a persistent lock-table read error is retried by reconnecting rangefeed clients in a tight loop, re-rolling the race on every attempt.

**Expected behavior**

A failed registration on a stopped processor should produce a retryable error to the client (as the register-wins ordering already does), not a node crash.

**Suggested fix**

In the `reg == false` branch, when the stopper is not quiescing, return an error instead of panicking. A stopped processor at this point always has a legitimate cause (quiescence or init-scan failure).

**Additional context**

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

Jira issue: CRDB-65652

Contributor guide

Open the contributing guide

Research direction

Start in pkg/kv/kvserver/replica_rangefeed.go at registerWithRangefeedRaftMuLocked, then trace Start in rangefeed/scheduled_processor.go and Run in rangefeed/task.go. Follow the stop-first ordering after an init resolved-timestamp scan error; done means a failed registration returns a retryable client error instead of panicking when the stopper is not quiescing.

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
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.