cockroachdb / cockroachdb/cockroach
kvclient: request state abandoned when a panic unwinds through DistSender.Send and Streamer.Close
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Summary:**
Two paths in `pkg/kv/kvclient` leave state behind when a panic unwinds through them. Go runs deferred functions during unwinding, so the gap here is cleanup written as plain statements. Part of #174944.
**Findings:**
- `kvcoord.DistSender.Send`: `responseChs` — on the panic path the drain is deliberately skipped, immediately below a comment explaining that draining is required because the `client.Sender()` contract forbids holding onto any part of a request after `Send` returns. Per-range goroutines therefore keep reading and mutating `ba` / `ba.Requests` after the caller has moved on and may have recycled or re-accounted those buffers.
- `kvstreamer.Streamer.Close`: `metrics.OperatorsCount` — the gauge decrement and `*s = Streamer{}` are plain statements after `results.close` and `truncationHelper.Release`, so a panic in either leaves the node-level gauge permanently inflated. Observability only; no job-relevant gauge feeds a control decision.
**Next Steps:**
- [ ] `DistSender.Send`: on the panic path, either drain with a bounded timeout or deep-copy what the async paths reference
- [ ] `Streamer.Close`: defer the gauge decrement
Epic: none
Jira issue: CRDB-68117
Contributor guide
Research direction
Start in pkg/kv/kvclient by reading kvcoord.DistSender.Send, especially the panic-path handling around responseChs, and kvstreamer.Streamer.Close around results.close, truncationHelper.Release, and metrics.OperatorsCount. Determine whether bounded draining or deep-copying safely satisfies the Sender contract, then verify that panic unwinding leaves no request state behind and the gauge is decremented.
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
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100