ci(standalone): give the smoketest's apt step a budget that outlasts its retries - #454
Conversation
…its retries The `Install system dependencies` step in `Standalone Smoketest` wraps a three-attempt, mirror-rotating apt loop whose worst case is 3 x (120 update + 180 install) + 2 x 15 sleep = 930s, but the step's `timeout-minutes` was 10 (600s). Actions therefore killed the step partway through the third attempt — in exactly the sustained-mirror-outage case the rotation exists for — so the `::error::apt failed after 3 attempts across mirrors` line never printed and the log showed only a bare step timeout. Raised to 16 minutes, with the arithmetic written beside it so the next person changing the retry schedule sees what the budget has to cover. This is the same fix a1bd66a made for the `Install zsh` step in `Build & Test` (750s schedule under a 360s cap); that change deliberately left this one alone as pre-existing and out of its diff. No behavior change on a healthy run: a successful apt finishes in under a minute and the loop exits on first success.
Deploying mouseterm with
|
| Latest commit: |
19122fd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3057f728.mouseterm.pages.dev |
| Branch Preview URL: | https://ci-smoketest-apt-timeout-bud.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
The worst-case arithmetic in the new comment undercounts by one sleep: sleep 15 is inside the loop after every failed attempt, including the third, so all three run before the ::error:: and exit 1. Worst case is 3 x (120 + 180) + 3 x 15 = 945s, not 930s. The 16-minute cap still covers it (945 < 960), so the fix works — but this comment is the reference the next person changing the retry schedule reads, and it's the one place the count has to be right. At sleep 60 the same undercount would read 1020s against a real 1080s.
The alternative is to make the code match the comment instead — guard the last sleep ([ "$i" -lt 2 ] && sleep 15), which also drops 15s of pure waiting before a failure the loop has already decided on. I'd take the comment fix: a conditional whose payoff is 15 seconds in a failure path isn't worth the extra branch.
Nothing else on the diff. Same-shape check: release.yml's Install system dependencies (Linux) has no retry loop and no cap, so it has no schedule to outlast — the body's read is right there. Note that the body's "the two capped steps in ci.yml are now the only ones" describes the post-#453 state; on this PR's merged tree Install zsh doesn't exist yet.
… two) The loop's `sleep 15` sits after the `::warning::` inside the `for` with nothing skipping it on the last iteration, so it runs after the third failed attempt too, before the `::error::` and `exit 1`. Worst case is 3 x (120 + 180) + 3 x 15 = 945s, not 930s. The 16-minute cap already covers it (945 < 960), so no budget change — but this comment is what the next person changing the retry schedule reads, and the count has to be right there: at `sleep 60` the same undercount would read 1020s against 1080s.
2 participants
Standalone Smoketest'sInstall system dependenciesstep wraps a three-attempt, mirror-rotating apt loop whose worst case is3 x (120 update + 180 install) + 3 x 15 sleep = 945s, under atimeout-minutes: 10(600s) cap. Actions killed the step partway through attempt 3 — in exactly the sustained-mirror-outage case the rotation exists for — so::error::apt failed after 3 attempts across mirrorsnever printed and the log showed a bare step timeout instead. Raised to 16 minutes with the arithmetic written beside it, so the next person changing the retry schedule sees what the budget has to cover.This is the follow-up #453 deliberately left out of its diff:
a1bd66afixed the identical shape inBuild & Test'sInstall zshstep (a 765s schedule under a 360s cap) and noted this one as pre-existing and out of scope.Verification is by arithmetic — the failure only reproduces under a real mirror outage.
.github/workflows/ci.ymlstill parses (yaml.safe_load), and the smoketest job'sInstall system dependenciesstep readstimeout-minutes: 16.Why 16, and what was left alone
945s is 15.75 minutes; 16 is the next whole minute, matching how
a1bd66arounded up. No behavior change on a healthy run — a successful apt finishes in under a minute and the loop exits on first success, so the cap is only reachable when the step is already failing.The
sleep 15runs on every failed attempt including the third — nothing in theforskips it on the last iteration — so the count is three sleeps, not two. An earlier revision of this comment said 930s;19122fdcorrects it. The cap covers either figure (945 < 960), but the comment is the reference the next schedule change reads from, so the count has to be right there.release.yml's Linux apt step (Install system dependencies (Linux), lines 64-68) has no retry loop, notimeout, and notimeout-minutesat all, so it has no budget-vs-schedule mismatch to fix — a different change if it wants one. On this PR's merge baseInstall system dependenciesis the only capped step inci.yml;Install zsharrives with #453, and once both land both budgets exceed their schedules (zsh: 765s under 780s — that comment carries the same one-sleep undercount, noted on this thread rather than fixed here, since it belongs to #453's diff).