cockroachdb / cockroachdb/cockroach

kv/bulk: SSTBatcher cleanup re-panics during unwinding and leaks a limiter slot

Open
#174,955 2 comments 0 reactions 0 assignees View on GitHub
A-disaster-recovery C-bug O-agent T-disaster-recovery
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.