ci: batch unit test packages into one go test run per group - #16335
Merged
Conversation
run-unit-tests-group.sh invoked "go test" once per package, serially, so a group used 0.4 of the runner's 4 cores (98s of CPU over 242s of wall clock). These tests are latency-bound rather than CPU-bound, so serializing packages wastes almost the whole runner. Batch them into one invocation per race/no-race set instead and let "go test" schedule packages concurrently up to -p. Per-package retry, which is why the loop existed, is kept by parsing the "FAIL<tab>package" lines out of the output and re-running only those. Group time is now bounded by the slowest single package rather than by the number of packages in the group, so 10 groups no longer buy anything over 5: measured locally at GOMAXPROCS=4, group 0 of 5 went 231s -> 94s and the heaviest group (pkg/ingester) ran in 254s against a 485s serial sum. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two unrelated problems with how the repository uses the Actions cache. The restore keys embedded the full build image reference, digest included, in both the primary key and its only fallback prefix. The build image is rebuilt whenever its own tooling dependencies are bumped (8 times in the currently retained cache window: smithy-go, cuelang, klauspost/compress, ...), and none of those touch the Go toolchain, yet each one dropped all three caches to nothing on every branch until main warmed them again. Add a second fallback tier that drops the image reference. This is safe because Go keys build cache entries by compiler build ID, so entries produced by a different Go version are carried but never hit. compare-helm-with-jsonnet.yml and update-vendored-mimir-prometheus.yml called setup-go without "cache: false", unlike the two calls in test-build-deploy.yml. That produced 26 entries totalling 8.18GB, as much as every deliberate Go build cache combined. They are scoped to refs/pull/N/merge, so only another push to the same PR can ever restore them, and the module half is empty anyway because this repository vendors its dependencies. Since caches are reclaimed on last access, that footprint shortens how long the shared caches on main survive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A cache written by a pull request run is scoped to that PR's merge ref, and GitHub only lets re-runs of the same PR restore it: not main, not sibling PRs. Once the PR closes nothing can hit those entries again, so deleting them is lossless. Measured on 2026-08-10: 10.38GB across 165 entries were PR scoped, against 9.93GB across 32 on main, and 16 of the 21 PRs still holding caches were already merged or closed. Since caches are reclaimed on last access rather than on age, that dead weight directly shortens how long the shared caches on main survive. Caches a PR only reads from main are owned by refs/heads/main, a different scope, so they are never listed by the ref filter this uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pitasi
added a commit
that referenced
this pull request
Aug 11, 2026
## what gives `TestIngesterSharding` an integration group to itself and round-robins the remaining 110 tests over the other 19. ## why the integration split round-robins the sorted test list, which leaves it **3.13x imbalanced**. measured on run 31381351961, the 20 groups ranged from 90s to **703s** against a 224s average. `TestIngesterSharding` is the whole problem: at **290s**. pinning it takes the slowest group from 628s to 290s of test time. ## notes - this won't move wall clock on its own: #16335 and this one together are what get a run to ~11 min Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
2 participants
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.
what
runs each unit test group as a single
go testinvocation instead of one per package, drops the group count from 10 to 5, and fixes two problems with how we use the actions cache.why
run-unit-tests-group.shrango testonce per package, serially, so a group used 0.4 of the runner's 4 cores — 98s of CPU over 242s of wall clock. these tests are latency-bound, not CPU-bound, so serializing packages wastes almost the whole runner. batching them letsgo testschedule packages concurrently up to-p.group wall time is now bounded by the slowest single package rather than by how many packages are in the group, so 10 groups buy nothing over 5. locally at
GOMAXPROCS=4: group 0 of 5 went 231s -> 94s, and the heaviest group (the one withpkg/ingester) ran in 254s against a 485s serial sum.the cache fixes came out of checking whether we were actually getting anything from the runner cache:
compare-helm-with-jsonnet.ymlandupdate-vendored-mimir-prometheus.ymlcalledsetup-gowithoutcache: false, unlike the two calls in this file. that was 26 entries / 8.18GB, as much as every deliberate go build cache combined, all scoped torefs/pull/N/mergeso only another push to the same PR could ever restore them. the module half is empty anyway since we vendor.caches are reclaimed on last access, not on age, so that footprint was shortening how long the shared caches on main survive.
notes
FAIL\t<pkg>lines out of the output and re-running only those packages.test (0, 10)->test (0, 5)), so branch protection needs updating or merges will block.-parallel 16on top of this is worth another 25-30% but flakedTestIngester_compactBlocksToReduceInMemorySeries_Concurrency, so not here.t.Parallel().