cmd: bound per-runner check concurrency

[bendrucker]

Summary

Per-runner check parallelism had no concurrency limit. tflint runs each ruleset's Check once per runner (the root module plus one runner per module call), and dispatched the module-call checks as one goroutine each with no bound. The only control was the boolean --no-parallel-runners, which turns the fan-out fully off. --max-workers did not apply. A configuration with many module calls therefore ran that many plugin Check RPCs at once, and the only way to reduce the load was to serialize every runner.

This bounds the per-runner fan-out by the --max-workers value (default: number of CPUs), so parallelism has a ceiling instead of an on/off switch.

Motivation

Check is plugin code, and a plugin may do network or other rate-limited work per runner. When it does, unbounded fan-out becomes a load problem rather than a speedup.

This surfaced in terraform-linters/tflint-ruleset-aws#1113. With deep_check and ~60 provider "aws" instances, each runner opened many concurrent STS connections, and the runner count multiplied that into a connection storm that exhausted DNS and sockets. --max-workers had no effect. Only --no-parallel-runners avoided the errors, at a large speed cost. The plugin side now caches clients across runners tflint-ruleset-aws#1114, but the core fan-out stayed unbounded for any plugin that does per-runner work.

Two existing reports look like the same unbounded concurrency from other angles:

• Race condition on circular dependency check when using parallel workers #2094: sporadic false circular reference found errors under parallel runners, gone with --no-parallel-runners.
• Failed to check ruleset; timeout waiting for connection info #2544: timeout waiting for connection info under heavy load, with --no-parallel-runners tried as a workaround.

Change

The per-runner loop in inspectModule now acquires from a semaphore sized by a worker count before launching each check. The worker count comes from runnerWorkers(): --max-workers when set, otherwise the CPU count, or 1 under --no-parallel-runners. The old NoParallelRunners branch collapses into the N=1 case of the same path.

The --max-workers resolution that recursive inspection already used moves to a shared Options.maxWorkers(), so one definition now governs concurrency on both paths. The flag description now says it bounds both directory and per-runner workers.

--no-parallel-runners is unchanged in behavior: it still forces fully serial checks.

Flag

This reuses --max-workers rather than adding a flag, so a single, familiar control governs concurrency everywhere. If per-directory and per-runner concurrency turn out to need independent tuning, a dedicated flag (for example --max-runner-workers) could be layered on later without changing this default.

One interaction to note: under --recursive, --max-workers already bounds the directory subprocess pool, and it is not currently propagated to those subprocesses. So each subprocess resolves its own per-runner bound, and the effective ceiling is roughly --max-workers directories times --max-workers runners. That is bounded and far below today's unbounded fan-out, but we could also propagate the value to workers.

Testing

Test_checkRunners_boundsConcurrency blocks the checks mid-flight and samples the peak number running at once, asserting it never exceeds the worker count across serial, bounded, and worker-heavy cases. An unbounded implementation fails it. Test_checkRunners_returnsFirstError confirms the first error surfaces and every runner is still drained. Test_Options_runnerWorkers covers the worker-count resolution, including --no-parallel-runners overriding --max-workers.

Origin

Per-runner parallelism landed in #1944 and the recursive --max-workers pool in #2021. This reconciles the two so one concurrency model covers both paths.

[wata727]

In my experience, if a Check call is I/O-bound, setting the number of goroutines to be the same as the number of processes degrades performance.

The reason I didn't set an upper limit on the parallel-runner concurrency is because it depends on the number of module calls. My intuition told me there wouldn't be that many module calls, but it seems I was wrong.

How about starting with a sufficiently large fixed upper limit, like 10 or 20?