[DeviousCardi]

Summary

Fixes #7759.

When useStreamConnection is true, MakeIngesterClient creates a grpc.ClientConn, then calls Run() to start INGESTER_CLIENT_STREAM_WORKER_COUNT (100) push-stream workers. If Run() returns an error, the function returned it without closing conn or cancelling the stream context — leaking the connection and any goroutines from workers that had already started successfully. In production this showed up as orphaned ClientConns reconnecting forever to unreachable ingester addresses after rollouts (observed via addrConn.resetTransportAndUnlock goroutine counts and steady SYN traffic to dead pod IPs).

Two related issues in the same Run() were fixed alongside the primary leak:

• Orphaned workers on partial failure: as soon as one worker's PushStream() fails, streamCancel() is now called immediately so sibling workers that already succeeded stop rather than continuing to run against a client that's about to be discarded.
• Data race: workerErr was written from all 100 worker goroutines with no synchronization. Replaced with a buffered chan error (sized to the worker count) so the first error is collected safely.

Changes

• MakeIngesterClient: close conn and cancel streamCtx before returning an error from Run().
• Run(): collect worker errors via a channel instead of an unsynchronized shared variable; cancel streamCtx as soon as any worker fails, so siblings stop promptly.
• Added a CHANGELOG.md entry.

Test plan

• go build ./pkg/ingester/client/...
• go test ./pkg/ingester/client/... -race — including two new tests: a unit test with a mock scheduler exercising partial-failure cancellation, and an end-to-end test that reproduces the issue's own scenario (unreachable address) and asserts via runtime.Stack scanning that no resetTransportAndUnlock/newClientStreamWithParams goroutines survive after MakeIngesterClient returns an error.
• Ran the new tests 20x under -race with no flakiness.

🤖 Generated with Claude Code

Written by Codex.

[friedrichg]

Thanks!

[sandy2008]

The leak fix looks right: both new tests fail on the merge-base and pass here, also under -race. Two small things inline. FYI, #7800 targets the same issue (#7759).

[sandy2008]

Small ordering issue: since streamCancel() runs before errCh <- err, a sibling woken by the cancellation can enqueue its context canceled first, and Run then returns that instead of the real failure, contrary to the comment below. In a probe where one worker fails while the other 99 wait on ctx, that happened in about a quarter of the runs. Sending first is safe because errCh has room for every worker:

Suggested change

	streamCancel()
	errCh <- err
	errCh <- err
	streamCancel()

[sandy2008]

Since 1.22.0 was marked in progress (#7836), this line ends up in the ## 1.22.0 in progress section (I checked with a trial merge onto current master). Unless it's meant for a 1.22 backport, could you move it under ## master / unreleased?