Skip to content

ci(e2e): give DerivedData adoption a budget, and log where its time goes - #14095

Closed
teamleaderleo wants to merge 3 commits into
mainfrom
ci/e2e-warm-adoption-budget
Closed

teamleaderleo wants to merge 3 commits into
mainfrom
ci/e2e-warm-adoption-budget

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Since #14016, every macOS e2e run's "Adopt main's DerivedData" step has run until the step timeout: 612 s under the old 10-minute limit, 912 s under the 15-minute one. None has ever hit. Every focused run pays those minutes before it compiles cold anyway. The step prints nothing until it finishes, so the logs can't say which phase is slow.

Sample of timed-out runs: 35937847074, 35938349258, 35938871900, 35938878301, 35939305209, 35939623231, 35940458918.

What is and is not slow

Phase Off the fleet (Linux, same 1.9 GB artifact) On the macOS fleet
Parallel-range download 34 s 35 s for the test product, in another step of the same runs
Unzip 1 s —
List + extract 65,028 tar members (6.3 GB) 35 s unmeasured
Hash the workspace (Record step, same walk as replay) — 5–8 s

The likeliest cost is extraction onto the VM's disk. That's a hypothesis: this PR's phase timings will confirm or refute it on the first run.

Change

The restore step now:

  • Logs as it goes: each phase and its duration print to stderr immediately, not at the end.
  • Gives up on its own: it stops after CMUX_WARM_BUDGET_SECONDS (default 420 s), using SIGALRM inside the process. Its own cleanup then deletes the partial DerivedData and reports which phase it was in. A step timeout kills the process before any of that can run.
  • Can't cut off a finished restore: the alarm is cancelled in a finally.

The step timeout drops to 10 minutes as a backstop. #14084's discard step still covers a process that dies anyway.

Testing

  • New tests in tests/test_e2e_warm_derived_data.py:
    • An adoption over budget returns hit=false, leaves DerivedData empty, and names the phase it was in.
    • An adoption within budget is not interrupted afterwards.
    • Negative control: removing the alarm makes the first test fail.
  • Guard lane: linux-guard exits 0 with 434 passing.
  • Other checks: the self-hosted guard exits 0, and actionlint is clean.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Stops macOS e2e DerivedData adoption from burning the full step timeout (612 s, then 912 s since #14016) with no hit. The restore step now gives up on its own after a time budget and logs where its time goes as it runs.

  • Stops after CMUX_WARM_BUDGET_SECONDS (default 420 s) via SIGALRM inside the process, so cleanup deletes the partial DerivedData and names the phase it was in; a zero or malformed budget falls back to the default instead of disabling or crashing.
  • Each phase (find archive, download, verify digest, unzip, extract, replay) logs its duration to stderr as it goes, and a timeout just after a phase ends no longer blames that phase.
  • The alarm is cancelled in a finally, so a finished restore can't be interrupted afterwards.
  • The step timeout drops to 10 minutes as a backstop.
  • Adds tests covering over-budget and in-budget adoptions; they drive the budget by delivering the alarm signal itself instead of sleeping, and restore the signal handler they change.

Written for commit fc63447. Summary will update on new commits.

Review in cubic

Every macOS adoption since #14016 ran into the step timeout: 612 s under
the old 10-minute limit, 912 s under the 15-minute one, and never a hit.
Each one added those minutes of silence to a focused run before it
compiled cold anyway. The step printed nothing until it finished, so the
logs cannot say which phase is slow.

Measured off the fleet, the same 1.9 GB archive takes 34 s to download
over the parallel transport, 1 s to unzip and 35 s to list and extract
its 65,028 members. On the macOS fleet the transport reads the test
product in 35 s and the input record hashes the workspace in 5-8 s, which
leaves extraction onto the VM's disk as the likeliest cost. That is a
hypothesis; the phase timings this adds will settle it on the first run.

The restore now:
- logs each phase and its duration to stderr as it goes
- gives up after CMUX_WARM_BUDGET_SECONDS (default 420 s) through SIGALRM,
  inside the process, so its own cleanup deletes the partial DerivedData
  and reports the phase it was in (a step timeout kills it before that)
- cancels the alarm in a finally, so a restore that finished cannot be
  interrupted afterwards

The step timeout drops to 10 minutes as a backstop, and #14084's discard
step still covers the case where the process dies anyway.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 18 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e16029cc-b039-4acc-8ac7-a201f91f9f19

📥 Commits

Reviewing files that changed from the base of the PR and between 19144b6 and fc63447.

📒 Files selected for processing (3)
  • .github/workflows/test-e2e.yml
  • scripts/ci/e2e_warm_derived_data.py
  • tests/test_e2e_warm_derived_data.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

…h off

Review follow-ups: a timeout just after a phase no longer blames that
phase; a zero or malformed CMUX_WARM_BUDGET_SECONDS falls back to a real
budget instead of disabling it or crashing outside the try; the budget
tests restore the SIGALRM handler and phase they change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Independent subagent review at 68a3821: no blocking defects. Follow-ups are in 855c2bff1b; the budget tests pass (11/11).

  • Timeout message: a timeout right after a phase no longer names that phase.
  • Budget setting: a zero or malformed CMUX_WARM_BUDGET_SECONDS now falls back to a real budget. Before, zero disabled the budget and a malformed value crashed outside the try.
  • Test cleanup: the budget tests restore the SIGALRM handler and phase they change.

Accepted as-is:

  • The alarm can arrive late during the download. Leaving the ThreadPoolExecutor block waits for in-flight range requests, so there the effective limit is the transport's own deadline (360 s plus at most one 60 s read). That still fits inside the 10-minute step. The alarm interrupts the verify, unzip, extract and replay phases promptly.
  • A staging directory can be left behind if the alarm fires during tempdir cleanup on the success path. It only matters on non-ephemeral runners.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 24, 2026 01:51
quality-determinism rejected the two time.sleep calls. The over-budget case
now checks that the 1 s alarm is armed and delivers SIGALRM itself; the
within-budget case checks that no alarm is left armed, which is what a
sleep past the budget was standing in for. Both fail if the source stops
arming or cancelling the alarm.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Closing as mostly superseded by #14098 (with #14084). Adoption now happens only when the app build is unchanged, which removes most of the 612–912 s timeouts that motivated this PR. Reopen if adopt steps still run to the timeout.

auto-merge was automatically disabled September 24, 2026 06:28

Pull request was closed

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