cockroachdb / cockroachdb/cockroach
kv/bulk: SSTBatcher cleanup re-panics during unwinding and leaks a limiter slot
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Summary:**
Two cleanup paths in `pkg/kv/bulk` do not survive an unwind, and one of them re-panics from a deferred `Close` during an unwind, replacing the original panic payload. Part of #174944.
**Findings:**
- `bulk.SSTBatcher.syncFlush`: `b.asyncAddSSTs.Wait()` runs before `b.cancelFlush()` and `b.flushSpan.Finish()`, none deferred. `ctxgroup.Wait()` re-panics a stored worker payload and never clears it, and this `Close` path is itself reached from deferred calls during unwinding (for example `defer batcher.Close(ctx)` in `restoreDataProcessor`). The re-panic replaces the original panic, so the diagnostic for the initiating failure is lost and the remaining cleanup is skipped. The same shape exists in `cloud.backgroundPipe.Close`.
- `bulk.SSTBatcher.maybeDelay`: the limiter reservation from `b.limiter.Begin(ctx)` is released by a `defer` that lives only inside a goroutine started later, so a panic in the intervening window leaks a slot in the node-shared bulk-send limiter. Benign at the default setting (`bulkio.ingest.sender_concurrency_limit=0`) but not when it is configured.
**Next Steps:**
- [ ] Convert a worker panic into an error inside `syncFlush` and `backgroundPipe.Close` rather than re-throwing it; clear the stored payload
- [ ] Acquire and defer the limiter reservation in the same frame
Epic: none
Jira issue: CRDB-68126
Contributor guide
Research direction
Start in pkg/kv/bulk at SSTBatcher.syncFlush and maybeDelay, then compare the cleanup behavior with cloud.backgroundPipe.Close and ctxgroup.Wait. Verify the panic payload is converted to an error and cleared while all cleanup still runs, and that the limiter reservation is acquired and deferred in the same frame.
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
- 52/100