ci: seed a pinned file with a shard's share, not with the count it defeats - #1040
Merged
fujibee merged 1 commit intoSep 5, 2026
Merged
Conversation
…feats 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
fujibee
merged commit Sep 5, 2026
3569a4f
into
integration/terminal-driver-v1
38 of 42 checks passed
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
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.
PINNED_APARTexists because these files'@testcount does not describe theircost — 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
9makes its shard read as very nearly empty, and theordinary 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
33850939213attempt 4, on the terminal-driver tree. Per-file secondsderived from the run's own timestamps (bats runs serially here, so the test
numbering is time-ordered).
test_remote_engine_start_refusal.bats— 630s over 9@testblocks, 70 s/test, 36% of the shardtest_remote_status_liveness.bats— 347s, 11 s/testAgainst 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.
Change
A pinned file is seeded at one shard's average share, 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, same
partition code:
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
sleepbecomes a poll loop, with nothing to say so.The share is derived from the tree on every invocation.
The over-reservation risk is stated in the code: 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 the shard total, and pinning a majority would
invert it — nothing detects that case, so the note says to keep the list short.
Tests
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. Full
sharding suite: 12/12.
Refs #1014