perf: minimize streamMu lock scope in sendRequestDefault - #10
Merged
Conversation
AppendRows on a managed writer's DefaultStream can block when the inflight queue (Max_Queue_Requests / Max_Queue_Bytes) is saturated. Holding streamMu for the entire duration serialized all concurrent flush calls on the same output instance, directly contributing to the 30s latency spikes observed under peak input load. Changes: - Add getStream(), appendResult(), leastLoadedStreamIndex() methods on outputConfig to encapsulate the lock/unlock boilerplate and narrow each critical section to slice access only. - Refactor sendRequestDefault to call AppendRows outside streamMu, using getStream() to fetch the stream pointer and appendResult() to enqueue the result after the call returns. - Refactor flushChunk to use leastLoadedStreamIndex() (no behavior change, removes the explicit Lock/Unlock pair). AppendRows on managedwriter.ManagedStream is goroutine-safe; the lock is no longer needed to protect the call itself. For the at-least-once (DefaultStream) path there is no stream rebuilding, so the *streamConfig pointer obtained under the lock remains valid after the lock is released.
yuzone
force-pushed
the
perf/minimize-stream-mu-lock-scope
branch
from
April 3, 2026 09:45
1503a7f to
8dc75fb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Previously,
sendRequestDefaultheldstreamMufor the entire durationof
AppendRows, blocking all concurrent flushes whenever the inflightqueue (Max_Queue_Requests / Max_Queue_Bytes) was saturated.
This PR narrows the critical section so that the lock is acquired only
for the slice access and result append, allowing
AppendRowsto executeconcurrently without holding the lock.
Changes
outputConfig:getStream(index)— acquiresstreamMuonly to read the stream pointerappendResult(stream, result)— acquiresstreamMuonly to append the resultleastLoadedStreamIndex()— acquiresstreamMuonly to find the least-loaded streamsendRequestDefaultandflushChunkwith these helpers