Skip to content

ci: bound the ffmpeg apt fetch so a stalled mirror costs a retry, not the job - #3356

Merged
miguel-heygen merged 3 commits into
mainfrom
ci-bound-ffmpeg-apt
Aug 19, 2026
Merged

ci: bound the ffmpeg apt fetch so a stalled mirror costs a retry, not the job#3356
miguel-heygen merged 3 commits into
mainfrom
ci-bound-ffmpeg-apt

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Flagged by @jrusso1020 — the ffmpeg apt install stalls on hosted runners, and it reproduces on main's tip, so it isn't specific to any one PR.

What it costs

An unbounded apt-get inherits the whole job budget. The producer integration lane normally finishes in ~11 minutes against a 20 minute cap; on a stalled fetch it ran to the cap and failed.

The bill isn't one red check. On the run that prompted this, four checks went red off that single step:

Check Why
Producer: integration tests stalled at Install FFmpeg, 20m18s
Preview parity stalled at Install ffmpeg, 20m18s
Test fails closed — "Producer unit/integration tests did not succeed", 6s
preview-regression fails closed on the skipped Preview parity

So a mirror stall reads as a producer defect and a preview defect. Same fail-closed chain @jrusso1020 traced on #3306, different trigger.

Evidence it's the mirror and not the tests: the identical job passed in 10m40s on #3306 at the same time it hit the cap on #3305.

The fix

Each attempt is bounded (timeout 120 for update, timeout 300 for install) and retried three times, so a stalled mirror costs seconds instead of the job. The five workflows that installed ffmpeg now share one action instead of five copies of the command.

What this deliberately does not do

Both alternatives in the original suggestion were considered and rejected, and I'd rather say why than leave them looking unexplored:

  • Caching the binary — apt's ffmpeg is dynamically linked. Restoring just the binary onto a runner that never installed the package strips it from its shared libraries. That fails only on cache hits, which is the worst shape for a CI flake fix.
  • A static build (ffmpeg-static, already a dependency) — this would change the ffmpeg binary underneath the producer's output comparisons. prepare-ffmpeg-bin already goes out of its way to avoid that CDN download, so the repo has evidently decided system ffmpeg is the source. A network-stall fix isn't the place to reopen it.

If the stalls continue past this, the next honest step is caching the .deb set (which keeps the dynamic links intact), not the binary.

Verification

YAML parses on all four changed workflows and the new action; the install script passes bash -n. Beyond that this is only exercised by CI running it — the checks on this PR are the test.

… the job

Hosted runners intermittently stall on an apt mirror, and an unbounded
apt-get inherits the whole job budget. The producer integration lane normally
finishes in ~11 minutes against a 20 minute cap; on a stalled fetch it ran to
the cap and failed. Same step, same shape, reproduces on main's tip — it is not
specific to any one PR.

The cost is not one red check. On the run that prompted this, four went red off
that single step: the two jobs that install ffmpeg, plus a Test gate and a
preview-regression gate that both fail closed when their dependency does not
succeed. So a mirror stall reads as a producer defect and a preview defect.

Each attempt is now bounded and retried three times, and the five workflows
that installed ffmpeg share one action instead of five copies of the command.
Deliberately still apt: caching the binary would strip it from the shared
libraries it links against, and switching to a static build would change the
ffmpeg under the producer's output comparisons. Neither belongs in a fix for a
network stall.
Converting the step to the shared action left the trailing `ffmpeg -version`
line behind, and YAML folded it into the `uses:` value — so the runner looked
for an action at a path with the command appended and failed all four perf
shards.

It parsed cleanly, which is why validating with a YAML load did not catch it:
`uses: ./path\n  ffmpeg -version` is a legal folded scalar. The check that
does catch it asserts every local `uses:` resolves to a directory containing
an action file, which is now what I ran. The action prints the version itself.
The first version wrapped apt in `timeout` and retried. A passing run showed
why that is the wrong shape: the mirror is slow rather than hung — the install
spent ~15 minutes pulling packages from azure.archive.ubuntu.com and finished
successfully. Killing it at 300s discarded a download that was making progress
and started over, so the retry turned a slow mirror into a slower one, and the
worst case of three attempts exceeded the job's own 20 minute cap.

Bound the connection instead. Acquire::Retries re-fetches the one package whose
connection stalled while keeping everything already downloaded, and
Acquire::http::Timeout caps how long any single connection may sit idle. That
addresses the stall the original report described without punishing the slow
case that is far more common.
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Correcting my own fix before it lands — a passing run showed the premise was wrong.

Producer: integration tests went green at 17m52s, and the ffmpeg step alone took 948 seconds. The log says what actually happened:

21:55:04  step starts
21:56:04  apt-get update completes (~60s, fine)
...       install pulls packages from azure.archive.ubuntu.com
22:10:53  ffmpeg version printed — success

So the mirror is slow, not hung. My timeout 300 killed a download that was making progress and started it over, which is why it took three attempts to get through. Two consequences I'd rather not merge:

  1. A wall-clock kill discards partial progress, so on a slow mirror the retry makes it slower, not faster.
  2. Worst case was 3 × (120 + 300) = 21 minutes against the job's own 20 minute cap — the fix could still burn the whole job, which is the exact failure it was meant to prevent.

The bound belongs on the connection rather than the command:

-o Acquire::Retries=3
-o Acquire::http::Timeout=30

Acquire::Retries re-fetches only the package whose connection stalled and keeps everything already downloaded; Acquire::http::Timeout caps how long any single connection may sit idle. That still addresses the stall @jrusso1020 originally described, without punishing the slow case that turns out to be far more common.

Worth recording that the evidence for this came from a green run. The check passed, so nothing would have flagged it — the 948-second step duration is the only thing that gave it away, and I only looked because 17m52s against a 20m cap seemed too close for a job that normally takes 11.

Also carried from the earlier push: converting player-perf.yml left a trailing ffmpeg -version line that YAML folded into the uses: value, so the runner looked for an action at a path with the command appended. My "does the YAML parse" check could not catch that — it is valid YAML. The check that catches it asserts every local uses: resolves to a directory containing an action file, and that now runs across all four workflows.

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Green at 60 checks, and the timings show the corrected shape was the right call:

ffmpeg approach Producer: integration Preview parity
unbounded apt (before this PR) 20m18s — failed at the 20m cap 20m18s — failed
timeout + retry (first version here) 17m52s — passed, 948s in the ffmpeg step 3m48s
connection-bounded (Acquire::Retries + http::Timeout) 2m29s 1m34s

The middle row is the interesting one: it was green, which is why nothing flagged it. The wall-clock kill was discarding a download that was making progress and restarting it, so three attempts were needed to get through — 948 seconds for an install that takes about forty when the mirror behaves. Worst case would also have been 21 minutes against a 20 minute job cap, so the fix could still have burned the job it existed to protect.

Bounding the connection instead lets apt re-fetch only the package that stalled and keep everything already downloaded. Same protection against the hang @jrusso1020 reported, without punishing the far more common slow case.

Merging on Miguel's go-ahead. Happy to revisit if you'd rather it were shaped differently — you raised the original report, so the shape should be one you'd defend.

@miguel-heygen
miguel-heygen merged commit 7e96e60 into main Aug 19, 2026
62 checks passed
@miguel-heygen
miguel-heygen deleted the ci-bound-ffmpeg-apt branch August 19, 2026 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant