Skip to content

Run ci-ok on a GitHub-hosted runner - #1747

Open
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
ci-perf/1745-ci-ok-hosted
Open

Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
ci-perf/1745-ci-ok-hosted

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

Fixes #1745

Problem

ci-ok runs one 2 s python3 check over toJSON(needs), alone, after every other CI job. It ran on depot-ubuntu-24.04-4, so each run paid a Depot host boot right on the merge's critical path. From #1745 (n=21, 10-27 13:07–20:16 UTC): start gap (job start → first step) mean 32 s, p90 131 s, max 155 s; since 16:17 the mean was 75 s.

Re-checked on the latest 4 runs before this change:

run event runner start gap
40577778835 pull_request depot-71zsq7z22d 32 s
40575163903 pull_request depot-r3sr8hpl78 28 s
40574951881 push depot-qbxw5fws87 5 s
40573814234 merge_group depot-5mz2mw730c 123 s

Change

  • .github/workflows/ci.yml, job ci-ok: runs-on: ${{ vars.CI_OK_RUNNER || 'ubuntu-latest' }} (GitHub-hosted by default) instead of the Depot/DISABLE_DEPOT_RUNNERS expression. Setting the repo variable CI_OK_RUNNER (e.g. depot-ubuntu-24.04-4) moves it back without a merge. Name, if: always(), needs, timeout-minutes and the check are unchanged.
  • scripts/tests/test_ci_scheduling.py: new test_ci_ok_defaults_to_a_hosted_runner pins that runs-on line.

Hosted queue check (asked for in #1745, because #1372 once moved ci-ok off ubuntu-latest during a 9–18 min hosted backlog)

GitHub-hosted jobs in merge-queue-fail-fast, merge-queue-janitor and pin-check, up to 3 runs per workflow per day, 10-24 → 10-27 (n=27): queue wait (created → started) p50 2 s, p90 2 s, max 3 s; start gap (started → first step) p50/p90/max 1 s. Well under the 10 s p90 bar. If hosted Linux ever backs up again, set CI_OK_RUNNER; no PR or merge needed.

Expected saving

Measured result

This PR's CI run 40578824535 (head e631e18, draft, so the heavy jobs skip as designed): ci-ok job ran on hosted ubuntu-24.04 (GitHub Actions 1002782885). Queued 2 s, first step 0 s after start, check took <1 s, whole job 3 s, result success.

baseline (#1745, Depot, n=21) latest 4 Depot runs this PR (hosted)
start gap mean 32 s, p90 131 s, max 155 s 32 / 28 / 5 / 123 s 0 s
job duration ~5–159 s 6–128 s 3 s

The merge_group saving shows only after merge (the queue runs the base branch's workflow plus this change). The profiler will verify it against the 10-27 baseline.

Where each test runs

No test moves. ci-ok keeps its name and needs, so the required check is the same.

Risk

Low. Only the runner of the aggregate check changes. The workflow change takes effect on this PR's head commit; merge_group effects show after merge, and the profiler will verify them.

Validation: python3 -m unittest discover -s scripts/tests (374 tests OK); YAML parses; zizmor findings on ci.yml identical to main (59, 0 medium/high); actionlint 1.7.7 reports only pre-existing anchor/matrix findings, none in ci-ok.

Code review (/code-review high, before leaving draft)

Fixed in 800ff09: (1) a hard-coded hosted label meant a hosted backlog could only be undone by a PR that itself waits on ci-ok, so vars.CI_OK_RUNNER is the escape hatch; (3) ubuntu-latest instead of ubuntu-24.04, matching every other hosted Linux job; (4) the assertion now lives in its own test; (6) dated measurements dropped from the YAML comment.
Not fixed: (2) hosted concurrency shared with the Windows/macOS legs under merge-queue peaks is a real unknown, but CI_OK_RUNNER now covers it and the profiler checks merge_group after merge; (5) moving other short critical-path jobs such as release-readiness off Depot is out of scope for #1745 and would need its own measurement.

https://claude.ai/code/session_011f9aePipa5qoK8iqhBVEbW


Note

Low Risk
Only the runner for the final aggregate status job changes; required-check logic and job dependencies are unchanged, with an optional repo variable to revert runner choice without a code change.

Overview
Moves the merge-blocking ci-ok aggregate job off Depot by default so the last step of every CI run no longer pays a Depot runner cold-start on the critical path (#1745).

ci-ok now uses runs-on: ${{ vars.CI_OK_RUNNER || 'ubuntu-latest' }} instead of the DISABLE_DEPOT_RUNNERS / depot-ubuntu-24.04-4 expression. The job name, needs, if: always(), and the short Python verdict step are unchanged; only the runner label changes. Workflow comments document that CI_OK_RUNNER (e.g. depot-ubuntu-24.04-4) is the repo-variable escape hatch if hosted Linux queues up.

scripts/tests/test_ci_scheduling.py adds test_ci_ok_defaults_to_a_hosted_runner to lock in that runs-on line.

Reviewed by Cursor Bugbot for commit 800ff09. Configure here.


Generated by Claude Code

ci-ok is one 2 s python3 check that runs alone after every other job,
so any Depot host boot lands on the merge's critical path: 32 s mean,
up to 155 s, in the 10-27 sample (#1745). Hosted ubuntu-24.04 jobs in
this repo started in 2-3 s at p90 over 10-24..10-27 (n=27). The job's
name, needs, if: always() and logic are unchanged, so the required
check stays the same. A scheduling test pins the runner.

Fixes #1745

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011f9aePipa5qoK8iqhBVEbW
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-perf CI / merge-queue performance finding (profiler routine) label Oct 27, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Code review on #1747: with a hard-coded hosted label, a hosted Linux
backlog could only be undone by a PR that itself waits on ci-ok in the
backed-up pool. vars.CI_OK_RUNNER restores a no-merge escape hatch,
like DISABLE_DEPOT_RUNNERS for the other jobs. Use ubuntu-latest to
match every other hosted Linux job, move the assertion into its own
test, and drop dated measurements from the YAML comment.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011f9aeP2GXjchXdv7YRSbGe
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 27, 2026 22:14
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 800ff09. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 27, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at head 800ff09.

  • CI: 83/83 checks on 800ff09 passed or skipped (33 success, 50 skipped by path filters). Mergeable with main; no main-wide failures.
  • /code-review high: 6 findings. Fixed 4 in 800ff09 (repo-variable CI_OK_RUNNER escape hatch, ubuntu-latest for consistency, own test, no dated numbers in YAML). 2 left as design notes in the description (hosted concurrency at merge-queue peaks; other short Depot jobs out of scope).
  • Bugbot: reviewed 800ff09, no new issues. No open review threads.
  • Reviewer: the one-line runs-on change in ci-ok is the whole effect; the merge_group saving only shows after merge.
  • Slack: not announced, because this run had no Slack send tool.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-perf CI / merge-queue performance finding (profiler routine) 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.

CI perf: ci-ok — 2-second check waits up to 2.6 min for a Depot host to boot, serially at the end of every run (~0.5–2.6 min per merge)

2 participants