Skip to content

Cap hung rust-cache restores so they cannot stall the merge queue - #1730

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
ci-janitor/bound-rust-cache-restore
Oct 26, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
ci-janitor/bound-rust-cache-restore

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

At 15:24 UTC on 2026-10-26, the merge_group run for queue head #1719 (run 40348796795) hung in yarn-classic 1.22.22 (job). The job sat in the Cache cargo step (Swatinem/rust-cache restore) for over 14 minutes. Every earlier step had finished in under 30 s.

Root cause

The restore is a transient infrastructure call to the cache backend on Depot runners. It has no timeout of its own. rust-cache already treats a restore error as a cache miss, but a restore that never returns blocks the job until the job-level timeout.

Fix

Every Swatinem/rust-cache step in ci.yml (11 steps), plus the macOS vexctl actions/cache/restore, now has timeout-minutes: 5 and continue-on-error: true. The vexctl save step is skipped when that restore did not succeed, so it never saves under an empty key on main. A hung restore fails only that step after 5 min, and the job carries on with a cold build. The 5-minute cap is more than 10x the slowest healthy restore seen. No other step changes behaviour, and save-if is untouched.

scripts/tests/test_merge_queue_fail_fast.py had a contract test that rejected any continue-on-error in ci.yml. The test exists because a job-level key would let ci-ok pass despite a failed job. It still rejects job-level keys. It now also allows continue-on-error only on rust-cache or actions/cache/restore steps that carry a timeout, and requires every such restore in ci.yml to be capped. I checked that adding continue-on-error to Run clippy, or removing a cap, fails the test. merge-queue-fail-fast.py reads job conclusions, so its fail-fast logic is unaffected. The script's docstring now says "job-level".

The compatibility workflows (*-compatibility.yml, 12 more rust-cache steps) are not required checks and are left for a follow-up.

Proof

  • A YAML parse confirms every rust-cache step in ci.yml carries both keys.
  • actionlint reports 170 findings before and after the change, all pre-existing matrix-property warnings, so none are new. zizmor --offline gives 59 findings both before and after.
  • python3 -B -m unittest discover -s scripts/tests: 373 tests OK, which is the suite the lint-ecosystems job runs.
  • No test is added, removed or moved.

Required check names (ci-ok, clippy) are unchanged. A step that times out is reported as failed, but because of continue-on-error the job (and so ci-ok) still passes.

/code-review high findings not fixed

  • Partial extraction if the cap fires mid-untar. For this to happen, extraction itself would have to hang, because a healthy restore finishes in under 30 s in total. If it does happen, the job fails, which is the same eviction as today but after 5 min instead of a 40-min hang. The change never makes the outcome worse.
  • Use SEGMENT_DOWNLOAD_TIMEOUT_MINS instead. Its default is already 10 min, and the 2026-10-26 hang ran past 14 min, so that setting didn't catch it. A step timeout covers every phase of the restore.
  • No warning when a restore times out. A timed-out step already gets an error annotation on the run, so it is visible.
  • The 5-min cap is too short for Windows/macOS. Checked: in the last nightly full-scope run (40276353917), the slowest Cache cargo on Windows or macOS took 19 s, and Windows took 24 s in the merge queue.
  • Other *-compatibility.yml workflows. They are not required checks and are out of scope here.
  • Use a YAML anchor instead of 11 copies. The new contract test enforces that the copies stay in sync.

https://claude.ai/code/session_01LZ8V19imL1WHdq6hZdoums


Generated by Claude Code

On 2026-10-26 the yarn-classic job of merge-queue head #1719 sat in
its `Cache cargo` (Swatinem/rust-cache) restore for 14+ minutes. The
job normally takes 2-3 minutes, and the 12 entries queued behind it
were blocked until the run was cancelled by hand. Without that, the
job would have run to its 40-minute timeout and then evicted #1719.

A healthy restore takes under 30 s, and a cache miss only costs a
cold build, so the restore is best-effort. Give every rust-cache step
in ci.yml a 5-minute step timeout and continue-on-error. A hung
restore now fails only that step, and the job builds without the
cache instead of stalling the queue.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZ8V19imL1WHdq6hZdoums
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) label Oct 26, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

The fail-fast contract test rejected any `continue-on-error` in ci.yml.
Its concern is a job-level key, which would let ci-ok pass despite a
failed job. The capped `Cache cargo` steps set it at step level, which
leaves the job's conclusion to its other steps, so narrow the check to
job-level keys (four-space indent).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZ8V19imL1WHdq6hZdoums
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: Ready for review at 05d96a9.

  • CI: 57/57 checks on 05d96a9 passed or skipped (38 success, 19 skipped); mergeable, no conflicts. No main-wide failures. CHANGELOG.md untouched.
  • Bugbot: reviewed 05d96a9, no findings; all review threads resolved.
  • CI-only change (ci.yml rust-cache step timeouts + contract test). The author's status block still listed a /code-review high pass before review; a reviewer may want to confirm that ran.
  • Slack announcement not sent this run (Slack send unavailable); the next run will retry.

Generated by Claude Code

Review follow-ups:
- The contract test allowed continue-on-error on any step, so a test
  or build step tolerating failure would slip past it. Allow it only
  on rust-cache / actions/cache/restore steps that also carry a
  timeout, and require every such restore in ci.yml to be capped.
- Cap the macOS vexctl actions/cache/restore the same way. If that
  restore times out on main, skip the save step, which would
  otherwise run with an empty key and fail the job.
- Correct the comment: the hang was in yarn-classic-matrix, and an
  uncapped restore runs to that job's own timeout.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZ8V19imL1WHdq6hZdoums
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. Gives every Swatinem/rust-cache step in ci.yml (11) and the macOS vexctl actions/cache/restore a 5-minute timeout-minutes plus step-level continue-on-error: true. A hung cache restore then fails only that step and the job builds cold, instead of holding the merge-queue head for 40 minutes (as it did to #1719 at 15:24 UTC today). The CI contract test now allows step-level continue-on-error only on capped cache restores.

Risk: low. CI-only. No test step, no required check name and no job-level key changes. The worst case is a slower cold build when a restore times out.

Look here

Verified

  • Read the full diff (3 files, +52/-2). CHANGELOG.md is untouched.
  • python3 -B -m unittest scripts/tests/test_merge_queue_fail_fast.py: 11 tests OK on d2398dc.
  • All 11 rust-cache@ steps in ci.yml carry both keys. The only other continue-on-error text in the file is a comment.
  • merge-queue-fail-fast.py reads job conclusions, and ci-ok reads needs results, so a step-level failure on a cache restore cannot hide a real test failure.
  • CI on d2398dc: 56/56 passed or skipped (37 success, 19 skipped), ci-ok and clippy green, mergeable, 0 review threads.
  • Bugbot: no Bugbot review or check run on this PR (bugbot run was requested at 15:45 UTC and never reported).

Changes I made: none.

Open questions (non-blocking)

  • The hosted-e2e invariants comment at line 2632 still says "NO continue-on-error", but the job's Cache cargo step now carries one. The intent (no hidden bypass of the e2e result) still holds, because only the cache restore tolerates failure. A follow-up could reword it to "NO job-level continue-on-error, and none on test steps".
  • The *-compatibility.yml workflows (12 more rust-cache steps, not required checks) are left for a follow-up, as the description says.

Auto-merge is armed, so approving sends this straight to the merge queue.


Generated by Claude Code

The invariant said the job has no continue-on-error, but its capped
Cache cargo restore now carries one. Say what still holds: none on the
job or its e2e steps.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZ8V19imL1WHdq6hZdoums
Merged via the queue into main with commit 708eee3 Oct 26, 2026
56 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the ci-janitor/bound-rust-cache-restore branch October 26, 2026 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants