cockroachdb / cockroachdb/cockroach

changefeedccl: resource leaks and potential Flush hang in runBatchingWorker error paths

Open
#166,624 1 comment 0 reactions 0 assignees View on GitHub
branch-master C-bug O-agent T-testeng
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

## Summary

`batchingSink.runBatchingWorker` has three related issues on error paths:

1. **Pool object and alloc leaks on error-skip paths.** When `termErr` is set (line 483) or `topicNamer.Name()` fails (line 493), the `continue` statement skips both `freeRowEvent(r)` and `r.alloc.Release(ctx)`. While alloc leaks are tolerated by design during errors, the `rowEvent` pool object leak is unintentional.

2. **Potential Flush hang on partial flushAll failure.** When `flushAll()` partially fails (first topic succeeds, second errors), `sinkFlushWaiter` is set but never closed. The `Flush` caller hangs until context cancellation.

3. **inflight counter drift.** `inflight` is incremented at line 487 before `topicNamer.Name()` is called. On Name() error, the event is never sent to IO, creating a permanent +1 drift. Currently masked by `termErr` handling but fragile.

## Locations

- `pkg/ccl/changefeedccl/batching_sink.go:483-485` (termErr skip path)
- `pkg/ccl/changefeedccl/batching_sink.go:487-496` (Name() error path)
- `pkg/ccl/changefeedccl/batching_sink.go:529-537` (flushReq handling)

## Suggested Fix

- Release alloc and free event on termErr skip
- Move inflight increment after Name() check
- Close sinkFlushWaiter after flushAll error

_This issue was found via automated deep static analysis._

Jira issue: CRDB-62036

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.