Found during review of #29.
Two related items in runParallelExec:
-
Worker pool idiom. A goroutine is spawned per pod before the semaphore is acquired, so with thousands of pods all goroutines exist up front and block on the channel. golang.org/x/sync/errgroup with SetLimit(limit) bounds goroutine creation itself and replaces the manual WaitGroup + semaphore + results channel plumbing. (Note: since Go 1.22 loop variables are per-iteration, so passing pod.Name, pod.Namespace as goroutine parameters is no longer needed to avoid capture bugs.)
-
Testability. The concurrency machinery is the code most likely to break, but it is untested because execCommand is hard-wired into runParallelExec. Injecting an exec function (e.g. func(podName, namespace string) PodResult) would allow tests asserting that the -j cap is honored and that all results come back. newExecutor is also untested.
Found during review of #29.
Two related items in
runParallelExec:Worker pool idiom. A goroutine is spawned per pod before the semaphore is acquired, so with thousands of pods all goroutines exist up front and block on the channel.
golang.org/x/sync/errgroupwithSetLimit(limit)bounds goroutine creation itself and replaces the manualWaitGroup+ semaphore + results channel plumbing. (Note: since Go 1.22 loop variables are per-iteration, so passingpod.Name, pod.Namespaceas goroutine parameters is no longer needed to avoid capture bugs.)Testability. The concurrency machinery is the code most likely to break, but it is untested because
execCommandis hard-wired intorunParallelExec. Injecting an exec function (e.g.func(podName, namespace string) PodResult) would allow tests asserting that the-jcap is honored and that all results come back.newExecutoris also untested.