cockroachdb / cockroachdb/cockroach
kvserver: rangefeed registration can panic if the init resolved-ts scan fails
- 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
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