From 94fbe0dd64da9428b6dd5a4c9caf4e5b3043ca0f Mon Sep 17 00:00:00 2001 From: fujibee Date: Fri, 4 Sep 2026 16:15:58 -0700 Subject: [PATCH] ci: seed a pinned file with a shard's share, not with the count it defeats PINNED_APART exists because these files' @test count does not describe their cost -- a file that is mostly waiting carries a tiny count while dominating its shard's wall clock. The seeding then handed the balancer that same count: a 630s file reporting a load of 9 makes its shard read as very nearly empty, and the ordinary weighted pass fills it right back up. The pin separated the heavy files from each other and then let one of them be buried under a full share of everything else. Measured, macOS, run 33850939213 attempt 4, on the terminal-driver tree. Per-file seconds derived from the run's own timestamps (bats runs serially here, so the test numbering is time-ordered): shard 1 32.2 min holds test_remote_engine_start_refusal.bats -- 630s across 9 @test blocks, 70 s/test, 36% of the shard shard 2 17.1 min holds test_remote_status_liveness.bats -- 347s, 11 s/test shard 3 10.8 min shard 4 12.2 min Against a 30-minute cap, shard 1 was cancelled at the cap on four consecutive attempts -- 395 of 462 tests done, zero failures, still progressing 16 seconds before the cut -- while shards 3 and 4 sat idle for fifteen minutes. A pinned file is now seeded at one shard's average share instead, which is the honest reading of "we cannot weigh this one": it is treated as already worth about what a shard is supposed to hold. Same tree, same measured per-file seconds, the same partition code: 14.5 / 15.8 / 21.0 / 21.1 min worst case 21.1, nine minutes of headroom Deliberately not a table of per-file seconds. That alternative was weighed when the weights were first written and rejected because it goes stale silently -- wrong the moment a fixed `sleep` becomes a poll loop, with nothing to say so. The share is derived from the tree on every invocation. The existing pin test cannot catch this regression: seeded by count the two pinned files still land on different shards, so it stays green through exactly the failure above. The new test asserts the reservation itself -- a shard holding a pin carries measurably fewer tests than the average -- and was mutation-checked to fail when the seed is put back to the count, and to pass otherwise. Refs #1014 --- .github/scripts/shard-tests.sh | 40 +++++++++++++++++++++++++++++++++- tests/test_ci_sharding.bats | 37 +++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/.github/scripts/shard-tests.sh b/.github/scripts/shard-tests.sh index 13db2f598..765e3345d 100755 --- a/.github/scripts/shard-tests.sh +++ b/.github/scripts/shard-tests.sh @@ -148,6 +148,44 @@ while [ "$i" -lt "$total" ]; do i=$((i + 1)) done +# What a pinned file is seeded with, and why it is not its @test count. +# +# The pin exists because these files' @test count does not describe their cost +# (see above). Seeding `load` with that same count therefore hands the balancer +# the one number it was told not to trust: a 630s file that contains 9 @test +# blocks reports a load of 9, the shard reads as very nearly empty, and the +# ordinary weighted pass fills it right back up. Measured on the terminal-driver +# tree (macOS, run 33850939213 attempt 4): the four shards came out +# 32.2/17.1/10.8/12.2 minutes against a 30-minute cap, and the 32.2 was the +# shard holding the 630s pin plus a full share of everything else. It timed out +# four attempts in a row while two other shards sat idle for fifteen minutes. +# +# So a pinned file is seeded at ONE SHARD'S AVERAGE SHARE instead: it is treated +# as already worth about what a shard is supposed to hold, which is the honest +# reading of "we cannot weigh this one". Same tree, same measured per-file +# seconds: 14.5/15.8/21.0/21.1, a 21-minute worst case. +# +# This is deliberately not a table of per-file seconds. That alternative was +# considered when the weights were first written and rejected because it goes +# stale silently -- it would be wrong the moment a fixed `sleep` becomes a poll +# loop, with nothing to say so. The share is derived from the tree on every run +# and cannot drift away from it. +# +# The failure mode to watch is over-reservation: a pinned file that is actually +# light leaves its shard underfilled and pushes the others up. That is bounded +# while the pins are few relative to `total` -- at 2 pins in 4 shards the worst +# case is the remaining work over two shards, which the numbers above clear with +# room. Pinning a majority of the shards would invert this, and nothing here +# detects that; keep PINNED_APART short. +total_tests=0 +while IFS= read -r f; do + [ -n "$f" ] || continue + total_tests=$((total_tests + $(file_weight "$f"))) +done <