Skip to content

Distributor: fix goroutine leak merging label results on canceled queries - #7933

Open
amalsaidov wants to merge 1 commit into
cortexproject:masterfrom
amalsaidov:fix/merge-slices-parallel-goroutine-leak
Open

amalsaidov wants to merge 1 commit into
cortexproject:masterfrom
amalsaidov:fix/merge-slices-parallel-goroutine-leak

Conversation

@amalsaidov

Copy link
Copy Markdown

What this PR does:

MergeSlicesParallel merges the per-ingester label name and label value slices concurrently. The distributor calls it with parallelism 8 on every LabelValues and LabelNames query, so the slices are split into several batches merged by separate goroutines.

The function keeps only the first error, over a one-slot channel, but it sent to that channel with a blocking send:

if e != nil {
    errCh <- e   // blocks once the channel is full
    wg.Done()
    return
}

When a query is canceled, more than one batch can see the canceled context and return an error at the same time (each batch checks ctx.Err() once it has merged CheckContextEveryNIterations = 128 elements). The first send fills the channel, the receiver takes that error and returns, and every other worker that also errored blocks forever on its send. The goroutine that waits on the WaitGroup before closing the channels then never returns either, so the whole set leaks for the lifetime of the process. Canceled or timed-out label queries are routine, so this leaks goroutines steadily in production.

The fix sends the error without blocking, so a worker that cannot report its error drops it and exits. The first error is still the one returned, and the successful path is unchanged.

The new regression test cancels the context up front and uses goleak to assert nothing is left behind. It fails before the change (two workers parked on chan send plus the WaitGroup waiter) and passes after.

Which issue(s) this PR fixes:

No issue filed; found while reading the label-query merge path.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags

AI-assisted; directed, reviewed and tested by the author (amalsaidov).

@amalsaidov
amalsaidov requested a review from a team as a code owner November 13, 2026 11:56
@amalsaidov
amalsaidov requested a review from alanprot November 13, 2026 11:56
…ries

MergeSlicesParallel splits the per-ingester label name and label value
slices into batches and merges them concurrently. It keeps only the first
error, over a one-slot channel, but it sent to that channel with a blocking
send. When a query is canceled, more than one batch can observe the canceled
context and return an error at the same time. The first send fills the
channel, the receiver takes that error and returns, and every other worker
blocks forever on its send. The goroutine that waits on the WaitGroup before
closing the channels then never returns either, so the whole set leaks for
the lifetime of the process.

Send the error without blocking so a worker that cannot report its error
just drops it and exits. The first error is still the one returned.

Add a regression test that cancels the context up front and uses goleak to
assert no goroutine is left behind.

Signed-off-by: amalsaidov <[email protected]>
@amalsaidov
amalsaidov force-pushed the fix/merge-slices-parallel-goroutine-leak branch from b63d74f to 7a37c90 Compare November 13, 2026 11:56

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant