Skip to content

ci: queue a pull request's admission for a root runner when Blacksmith's wait is longer - #15376

Merged
teamleaderleo merged 2 commits into
mainfrom
ci/admission-queues-on-roots-when-shorter
Sep 28, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
ci/admission-queues-on-roots-when-shorter

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

With split placement (CI_PR_POOL_OWNED_SPLIT=1), a pull request run whose owned root runners are past the queue bound places compile admission on the retry runner, and every job after it follows. That happens however long Blacksmith's queue is.

Measured from the Actions webhook feed, 2026-09-28 07:00 to 10:30Z:

First-attempt admissions on Blacksmith 79, average queue wait 16 min, 39 cancelled before they got a runner
Owned root runners' queue wait, same window q50 3.4 min, q90 10.8 min
One picker decision 0 of 16 root runners free, it needs 1 ... (Blacksmith's expected wait 58 min), sent to 12vcpu, admission waited 44 min

After the morning peak the picker behaves: in a sample of 89 runs created after 10:30Z, 87 admissions ran on the minis with a median wait of about 1 min. So this change matters under load, which is when Blacksmith is also backed up.

Scope

owned_room() already queues admission on the root runners while its wait is within the rounds (20 min at CI_PR_POOL_QUEUE_ROUNDS=2). This adds the band from 20 min up to half a job under the rescue's budget (25 of 30 min), and only when Blacksmith's wait is longer. It is a narrow change for peak load.

Change

In pick(), when a split run's chosen owned pool has no root runner within the bound, admission gets one root runner only if both of these hold:

  • its expected wait for that root runner is shorter than the best Blacksmith pool's expected wait for it; and
  • that wait ends at least half a job before the rescue's budget for the queue rounds.

The expected wait counts queued jobs, newer runs and replayed runs over the root runners, times a job length. The rescue budget is QUEUE_ROUND_MINUTES per round. Past that budget the rescue would move admission to the back of Blacksmith's queue, which is worse than going there now.

  • owned_pool_rescue.QUEUE_ROUND_SECONDS now derives from pr_runner_pool.QUEUE_ROUND_MINUTES, so the two cannot drift.
  • The reason line says when this applies, e.g. admission queues for a root runner, about 29 min against 41 min on Blacksmith.
  • Unchanged: the kill switch (CI_PR_POOL_QUEUE_ROUNDS=0), main's full suite with a reserve (split off), and runs that fit.

Tests

python3 -m unittest tests.test_ci_pr_runner_pool tests.test_ci_owned_pool_rescue tests.test_ci_late_placement (380 OK). New cases in PullRequestAdmissionRootQueue: queues when Blacksmith is longer; takes Blacksmith when it is shorter; never past the rescue's budget; the kill switch keeps the split.

🤖 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

With split placement, a pull request run whose owned root runners are past the queue bound now queues admission for a root runner when its expected wait there is shorter than Blacksmith's and ends at least half a job before the rescue's budget. Before, admission (and so every job after it) took the retry runner however long Blacksmith's queue was.

  • During a 3.5-hour morning peak, 79 first-attempt admissions waited an average of 16 minutes on Blacksmith (one at an expected 58), while root runners' queue drained with a p90 of 11 minutes.
  • The half-job margin keeps admission out of the rescue's reach: it fires just past the rounds' budget, and an estimate at the budget is unreliable because mini admissions run past the round price and newer runs are missing from the live root count.
  • QUEUE_ROUND_SECONDS now derives from the new QUEUE_ROUND_MINUTES in pr_runner_pool so the two cannot drift.
  • The kill switch (CI_PR_POOL_QUEUE_ROUNDS=0) and main's reserve keep the old split.

Written for commit 27e1990. Summary will update on new commits.

Review in cubic

…h's wait is longer

With split placement, a run whose root runners are past the queue bound
places admission, and so every job after it, on the retry runner however
long Blacksmith's queue is. On 2026-09-28 from 07:00 to 10:30Z, 79
first-attempt admissions took Blacksmith and waited 16 minutes on average
(one picked at an expected 58), while the root runners' queue drained with a
p90 of 11.

pick() now gives admission one root runner past the bound when its expected
wait there is shorter than the best Blacksmith pool's and within the owned
rescue's budget for the rounds, so the rescue never moves it to Blacksmith's
tail. The rescue's round length moves to pr_runner_pool.QUEUE_ROUND_MINUTES
so the two cannot drift. The kill switch (rounds 0) and main's reserve keep
the old split.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 28, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 6 minutes.

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: 91109b59-7272-4a43-a25a-8a39094143e3

📥 Commits

Reviewing files that changed from the base of the PR and between 56ec600 and 27e1990.

📒 Files selected for processing (3)
  • scripts/ci/owned_pool_rescue.py
  • scripts/ci/pr_runner_pool.py
  • tests/test_ci_pr_runner_pool.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.

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Dogfood build of 27e1990f767802bea4d68ec92af566b3db26fe2b

cmux DEV pr-15376-27e1990f.app

The link opens this exact commit in the cmux dev menu bar app. The build starts on each push and the page waits until it is ready; a newer push replaces it. It signs in against production, so Cloud or backend changes still need a tagged build with a development backend.

Review follow-up. The rescue fires 90 s past the rounds' budget, while a
mini's admission runs past the 10 minutes a round is priced at and runs 3 to
10 minutes old are missing from the live root count, so an estimate at the
budget would often be rescued to Blacksmith's tail. Tests now pin the jobs
placed, the boundary, and the kill switch's placement.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Subagent review of 238251e: FIX-FIRST.

Fixed:

  1. The allowed wait ended about 90 s before the rescue fired. A mini's admission runs past the 10 minutes a round is priced at, and runs 3 to 10 minutes old are missing from the live root count, so an admission at the edge would often be rescued to Blacksmith's tail. It now has to end half a job (5 min) before the budget; there is a boundary test.
  2. The tests now pin the full place() tuple (admission plus the one shard it hands its runner to), and the kill switch's placement.

Accepted as is:

  • The replay prices replayed runs at split=False, so under a burst it is optimistic about the root queue; the margin covers it.
  • waits[best] includes the cold macOS 15 pool, which understates Blacksmith's wait. That errs toward Blacksmith, so it is safe.
  • The reason text says "1 queue places" when admission is past the bound. Cosmetic.

The review also narrowed the scope: owned_room() already allowed 20 minutes, so this change only adds the band from 20 up to 25 minutes at rounds 2. The PR description now says so.

🤖 Generated with Claude Code

@teamleaderleo
teamleaderleo merged commit 9373164 into main Sep 28, 2026
55 checks passed
@teamleaderleo
teamleaderleo deleted the ci/admission-queues-on-roots-when-shorter branch September 28, 2026 13:38
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 27e1990f76: every check was green at merge (14 verified; 18 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 28, 2026
0e298fb ci: wait for the product's canonical root instead of compiling beside it (manaflow-ai#15379)
3088273 ci: UI test runs adopt compile admission's product, skip the re-upload, and report progress (manaflow-ai#15331)
b681e7e Keep a pending banner quiet once its pane is focused (manaflow-ai#15357)
03a2f6e Record that cloud_vm_sessions.attachment_count is cumulative (manaflow-ai#15321)
48258b4 fix(iroh-v2): check the team socket cap before opening the session (manaflow-ai#15340)
2638d56 Agent activity reorder follow-ups: group on-top check, search, subtitle (manaflow-ai#15362)
9ed83fd Dogfood journey: record whether a paused Cloud machine is asleep (manaflow-ai#15293)
7171ea8 Add app.tabBarVisibility to hide the pane tab bar when a pane has one tab (manaflow-ai#15294)
8743ec8 test: stop Computer Use onboarding tests waiting out the helper status deadline (manaflow-ai#15329)
6e4f1da ci: drain the snapshot's owned queue by what the machines finished since (manaflow-ai#15374)
9373164 ci: queue a pull request's admission for a root runner when Blacksmith's wait is longer (manaflow-ai#15376)
634a155 test: expect injected pane attention accent (manaflow-ai#15370)
cd030e9 Keep a named Cloud machine's prompt name instead of flipping to its slug (manaflow-ai#15288)
24ee0ee Exit 1 when cmux terminal screen wait times out (manaflow-ai#15282)
1b857ac test: cover a live Codex turn owner keeping its turn on SessionStart (manaflow-ai#13588)
56ec600 PR media: prune media of long-closed pull requests (manaflow-ai#15364)
4898cde ci: bound the SwiftPM scratch holder and cache scratch sizes (manaflow-ai#15366)

# Conflicts:
#	.github/workflows/ci-guards.yml
#	.github/workflows/ci.yml
#	.github/workflows/test-e2e.yml
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