cockroachdb / cockroachdb/cockroach

kvserver/rangefeed: bufferedRegistration.publish can strand an allocation in blockWhenFull mode

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

Description

**Describe the problem**

In `blockWhenFull` mode, `bufferedRegistration.publish` can complete its blocking channel send after `drainAllocations` has already run, leaving the event's budget allocation charged forever ([buffered_registration.go#L135-L146](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/rangefeed/buffered_registration.go#L135-L146)):

1. `publish` finds `br.buf` full, releases `br.mu`, and is about to execute the blocking `br.buf <- e` (alloc already `Use`'d) but is not yet parked on the channel.
2. The output loop hits a stream error; `runOutputLoop` calls `Disconnect`, then the deferred `drainAllocations` does its single non-blocking pass, empties `br.buf`, and returns.
3. The blocked `publish` now executes `br.buf <- e`; the channel has free capacity, so the send succeeds.
4. Nothing ever reads `br.buf` again. The allocation is never released; the feed budget stays charged until processor shutdown.

If the sender is already parked when `drainAllocations` receives, the channel handoff moves its element into the buffer and the drain picks it up — the leak requires the sender to not yet be parked, an ordinary scheduling race.

`blockWhenFull` is set when `EventChanTimeout == 0`. The code comments treat this as test-only, but `COCKROACH_RANGEFEED_SEND_TIMEOUT=0` is a documented production configuration ([replica_rangefeed.go#L103-L109](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/replica_rangefeed.go#L103-L109): clients are never disconnected due to slow consumption).

**Suggested fix**

After re-acquiring `br.mu` in the successful-send arm, check `br.mu.disconnected` and re-drain; or re-check `disconnected` before the blocking send and have `Disconnect` re-drain after setting the flag so a late send is always observed by a drain.

**Additional context**

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

Jira issue: CRDB-65655

Jira issue: CRDB-65790

Contributor guide

Open the contributing guide

Research direction

Start in pkg/kv/kvserver/rangefeed/buffered_registration.go at bufferedRegistration.publish (lines 135-146), then trace runOutputLoop, Disconnect, and drainAllocations. Reproduce or test the disconnect/send scheduling race described; done means a late successful send cannot leave its event allocation charged after the buffer is drained, including EventChanTimeout=0.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.