fix(ci): Upload a shard's results three times, not once - #2854
Merged
Conversation
Shard 16 of run 31951756102 was three minutes into a 196-minute check budget when its runner went away: ``` the runner service is stopped, or a manually started runner is canceled. ``` Nothing the driver did: 143 is SIGTERM, and the other 39 shards of that wave carried on. The runner was reclaimed. What made it expensive is ours. The driver has written its results as it goes since #2836 -- one manifest line per package, appended, precisely so that a shard killed part way through still accounts for what it finished. But the upload was a single step after the checks, and `if: always()` cannot save a step on a runner that is no longer there: it was skipped, and all 87 packages came back `missing` after an hour of runner time. The checks now run in three slices, each followed by an upload. The slices share the results directory and overwrite one artifact name, so the newest upload to survive is a superset of the ones before it, and the loss is bounded to a third of the shard. The packages are dealt round robin rather than in blocks. `runnable` is heaviest first, so a contiguous cut would put every long check in the first slice and leave the last one nothing but cheap ones. Three things had to become slice-aware: * the end-of-run "whatever the loop never reached" pass reads the manifest back and skips packages already accounted for, so slice 2 cannot bury slice 1's results under a `deferred` line that a later line wins with. It still writes `deferred` for *later* slices' packages, which is the truth at that moment and better than the `missing` the collector reconciles into; * `timing.json` accumulates `checks` and `check_seconds` across slices, since the collector fits the cost model from them and a third of a shard's checks next to a whole job's `script_seconds` would price every shard at a third of what it costs; * the per-slice summary covers that slice's packages, not the whole shard's, whose other two thirds are still at their initial `deferred` in this process. `CHECK_SLICE` is unset outside the workflow, and then nothing slices. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D1xpHRV7yVfgtJg4vp9P7z
krlmlr
force-pushed
the
claude/revdep2-slice-uploads-8mj8p1
branch
from
August 17, 2026 12:47
e37fce4 to
c443f67
Compare
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.
What happened to shard 16
Not a driver failure. The UI reports:
and the job log ends:
Three minutes into a 196-minute check budget, while the other 39 shards of that wave carried on. #2852 addresses the cause side — a resource sampler in the shard so the next one says which of CPU, memory or network it was, and
nice/ioniceon the checks so the runner agent is never the process that loses. This PR addresses what it cost.What made it expensive, which is ours
The driver has written its results as it goes since #2836 — one manifest line per package, appended, precisely so that a shard killed part way through still accounts for what it finished. But the upload was a single step after the checks, and
if: always()cannot save a step on a runner that is no longer answering. It was skipped:All 87 packages came back
missing, after an hour of runner time (8 min setup, 5 min install, 4 min checking).The change
The checks run in three slices, each followed by an upload. The slices share the results directory and overwrite one artifact name, so the newest upload to survive is a superset of the ones before it, and the loss is bounded to a third of the shard.
Packages are dealt round robin, not in contiguous blocks:
runnableis ordered heaviest-first, so a block cut would put every long check in slice 1 and leave slice 3 nothing but cheap ones — and the deadline, which stops the shard when the next check will not fit, would then bite unevenly.Three things had to become slice-aware:
deferredline — and a later line wins in the collector. It still writesdeferredfor later slices' packages, which is the truth at that moment and better than themissingthe collector would otherwise reconcile them into.timing.jsonaccumulateschecksandcheck_secondsacross slices. The collector fits the cost model from these, and a third of a shard's checks next to a whole job'sscript_secondswould price every shard at a third of what it costs — exactly the kind of underestimate that tips a plan into extra waves.deferredin this process.CHECK_SLICEis unset outside the workflow, and then nothing slices — a localPHASE=checkinvocation behaves exactly as before.Rebased onto #2849, #2852 and #2853
The step list is the merge of this PR's slices with #2852's sampler and consumption report, in the order that keeps both working:
The consumption report stays immediately before the final upload, so its last sample and OOM verdict still ride in the artifact; every upload gained
overwrite: true, which the repeated name now requires. In the README, the three rows that had grown to describe the same event — #2852's starvation row, and two of mine — are one starvation row plus one merged "a shard job dies hard" row.shard.Rmerged cleanly and carries all three changes:CHECK_SLICEhere,ensure_check_sysreqs()from #2853,depmissingfrom #2849.Verified
1/3→ 4,2/3→ 3,3/3→ 3; union equals the input, no overlaps.pkgA=okandpkgB=deferredon disk and slice 2 having checkedpkgB, the unreached pass writes nothing,pkgAis protected, and last-line-wins leavespkgA=ok pkgB=ok.needs_recheck()isTRUEfor bothdeferredand the newly mergeddepmissing,FALSEforok— so a slice never reached is retried either way.revdep2.yamlparses and the step order above is what it yields; all five R scripts parse and areair-formatted; all three shell scripts passbash -n.Not fixed here
Nothing prevents the runner going away. What this bounds is the cost — and
retry-runstill picks up whatever a slice never reached.🤖 Generated with Claude Code
https://claude.ai/code/session_01D1xpHRV7yVfgtJg4vp9P7z