cmd: bound the per-runner check fan-out#2577
Draft
bendrucker wants to merge 1 commit into
Draft
Conversation
The per-runner check fan-out launched an unbounded goroutine per module-call runner, so peak concurrency tracked the runner count. Under a deep check that guards a remote service, hundreds of module calls turn into hundreds of simultaneous requests (the #1113 storm). Resolve a single worker count (--max-workers or NumCPU, or 1 under --no-parallel-runners) and cap the fan-out with errgroup.SetLimit. The same count now governs the recursive directory pool, so resolution lives in one place. errgroup also waits for every check before returning, so a failing check no longer leaves goroutines running after Clean. At 1000 module calls the end-to-end benchmark caps peak concurrency at the worker count instead of ~900. Wall-clock rises with the bound, which is the intended trade of throughput for bounded resource use. Claude-Session: https://claude.ai/code/session_01SFtKL5iaFWPVDqdcyvPiCJ
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
1 participant
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.
The per-runner check fan-out launched one goroutine per module-call runner with no
bound, so peak concurrency equaled the number of runners. When a check guards a remote
service, as the deep_check case in #1113 does, hundreds of module calls become hundreds of
simultaneous requests. That is what produced the STS throttling and false
no such hostfailures.
This caps the fan-out. A single worker count is resolved from
--max-workers(or thenumber of CPUs, or 1 under
--no-parallel-runners) and applied witherrgroup.SetLimit.The recursive directory pool already derived its bound the same way, so both now draw from
one helper and the two concurrency layers share a budget.
errgroup.Waitalso blocksuntil every check finishes, so a failing check no longer leaves goroutines touching a
runner after
Clean.No user-facing flags change.
--max-workersand--no-parallel-runnerskeep theirmeaning, and
--max-workersnow bounds the per-runner path too.Measured with the end-to-end fan-out benchmark from #2573, at 1000 module calls this caps
peak concurrency at the worker count instead of ~900. Wall-clock rises under the bound,
which is the intent: it trades throughput for bounded resource use, the safety property the
storm needs.
Stacked on #2576.