Repository navigation
ci(*): charge the idle ceiling for waiting, not for queueing behind a busy runner - #466
Conversation
… busy runner The ceiling reads wall clock minus CPU and calls the difference waiting. A thread that is runnable and not on a CPU spends it the same way, and with four workers, the controller and coverage on a four-vCPU runner that queueing reached three to four seconds inside one long test, so green shards failed on whichever test was executing when contention peaked. Warming the catalogue did not stop it; halving the workers does, at the price of every shard's wall clock. Idle now subtracts the time the test's threads spent queued for a CPU, read from the scheduler's own account in /proc/self/task/<tid>/schedstat. Three things had to hold for that to work, and each one showed up as a residual in a contrast run before it did: - every thread of the process is read, not only the one running the test: the rpc model handlers do their catalogue reads on asyncio.to_thread; - a thread writes its own last reading on the way out, through a wrapper on Thread._bootstrap_inner, because the per-test event loop is closed at the end of the test and takes its executor threads with it, and /proc forgets a thread the moment it ends; - CPU stays on os.times, which keeps the CPU of ended threads and of reaped children; the scheduler's per-thread run time would have been lost the same way the queue time was. A sleeping thread is not on a run queue, so a test that waits on purpose is still charged the whole wait: an unmarked time.sleep(0.5) reports 0.53s idle with 0.00s queued. Without schedstat (macOS, Windows) the arithmetic is the old one, and the strict gate only runs on Linux; a Linux-only test fails loudly if the runner's kernel stops keeping the statistics. Measured in a Linux container with four workers pinned to two CPUs: on tests/test_rpc_model.py at a 0.05s ceiling, 70 tests over the line before and 2 after (worst 0.12s idle, 0.19s queued); shard 3/4 under the CI command failed its session on a 3.11s idle before and passed with no idle report after, and the other three shards report none either. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
I reviewed the complete github/main...HEAD diff and the surrounding pytest lifecycle hooks, the idle-ceiling history and CI invocation, the thread/executor callers that motivate retaining ended-thread scheduler data, and the current PR record. I also checked the applicable AGENTS.md constraints, compatibility/fallback behavior, and whether the tests were weakened. The change is confined to test infrastructure, retains the old arithmetic when schedstat is unavailable, and adds coverage for live threads, ended threads, malformed or absent scheduler data, and the Linux integration path; I found no weakened assertion or architectural violation.
Verification:
uv run --frozen --all-extras pytest -q tests/test_conftest_idle_ceiling.py --idle-ceiling-strict: 13 passed.uv run --frozen --all-extras pytest -q tests/test_rpc_model.py --idle-ceiling-strict: 195 passed.taskset -c 0-1 uv run --frozen --all-extras pytest -q tests/test_rpc_model.py --idle-ceiling 0.05: 195 passed in 33.57s with no idle-ceiling report.uv run --frozen --extra dev ruff check tests/conftest.py tests/test_conftest_idle_ceiling.py: passed.uv run --frozen --extra dev ruff format --check tests/conftest.py tests/test_conftest_idle_ceiling.py: passed.uv run python scripts/check_source_language.py github/main...HEAD: passed. The Make wrapper was unavailable in this environment becausemakeis not installed.- All reported GitHub checks pass, including the four Linux unit shards with the strict idle ceiling.
… busy runner (#466) ## Summary Since the idle ceiling became a gate (#428), a unit shard has failed on `main` and on most pull requests with every test passing: the ceiling reads wall clock minus CPU and calls the difference waiting, and a thread that is runnable and not on a CPU spends it the same way. With four workers, the controller and coverage on a four-vCPU runner that queueing reached three to four seconds inside one long test, so the shard died on whichever test was executing when contention peaked, a different one each run. Warming the catalogue (#452) did not stop it. #449 halves the workers per shard, which pays for the same measurement with every shard's wall clock. This changes the measurement instead. Idle now subtracts the time the test's threads spent queued for a CPU, read from the scheduler's own account in `/proc/self/task/<tid>/schedstat`: - every thread of the process is read, not only the one running the test, because the rpc model handlers do their catalogue reads on `asyncio.to_thread`; - a thread writes its own last reading on the way out, through a wrapper on `Thread._bootstrap_inner`, because the per-test event loop is closed at the end of the test and takes its executor threads with it, and `/proc` forgets a thread the moment it ends; - CPU stays on `os.times`, which keeps the CPU of ended threads and of reaped children; the scheduler's per-thread run time would have been lost the same way the queue time was. A sleeping thread is not on a run queue, so a test that waits on purpose is still charged the whole wait. Without `schedstat` (macOS, Windows) the arithmetic is the old one; the strict gate only runs on Linux, and a Linux-only test fails loudly if the runner's kernel stops keeping the statistics rather than letting the gate degrade quietly. `-n 4`, the 3 s ceiling and the four shards are unchanged. If this lands, #449 is not needed. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [x] CI / tooling - [ ] Refactor - [ ] Other ## Verification Unit tests for the clocks (12 on macOS, 13 on Linux, the last one reads the live kernel): ``` uv run --frozen --all-extras pytest -q tests/test_conftest_idle_ceiling.py macOS: 12 passed, 1 skipped Linux container: 13 passed ``` Full suite on macOS, where the fallback arithmetic runs: ``` uv run --frozen --all-extras pytest -q -p no:cacheprovider 23137 passed, 109 skipped in 291.30s ``` Contrast runs in a Linux container (Docker, linux/arm64, kernel 6.12), the checkout and venv on container-local storage, `main` at 2bdc4bd against this branch. Four workers pinned to two CPUs stands in for the four-vCPU runner; the ceiling is lowered to 0.05 s so the effect shows on one file: ``` docker run --cpuset-cpus=0-1 ... pytest tests/test_rpc_model.py --idle-ceiling 0.05 -n 4 main: 70 unmarked tests over the ceiling, worst 1.67s idle of 2.89s branch: 2 unmarked tests over the ceiling, worst 0.12s idle of 0.50s (0.19s queued) docker run --cpuset-cpus=0-3 ... pytest tests/test_rpc_model.py --idle-ceiling 0.05 -n 16 main: 81 over, worst 6.11s idle of 8.16s branch: 1 over, 0.13s idle of 1.11s (0.73s queued) an unmarked test that does time.sleep(0.5), on the branch: 0.53s idle of 1.70s (0.00s queued) # an honest wait is still charged in full ``` The CI shard command, same container, four workers pinned to two CPUs: ``` pytest -q --shard 3/4 --durations=25 --idle-ceiling-strict -n 4 --cov=raven --cov=raven_everos --cov-branch --cov-report= main: 5131 passed, 10 skipped; "1 unmarked test(s) waited more than 3s (failing the session)" 3.11s idle of 6.04s tests/test_ppt_engine_template_tool.py::test_binding_a_template_by_path; exit 1 branch: 5378 passed, 11 skipped; no idle report; exit 0 branch, shards 1/4, 2/4, 4/4: no idle report in any (three failures, all `git ls-files` exit 128 because the copied worktree's gitdir is not inside the container) ``` - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed (none; the option help text and the summary line say what is now subtracted) ## Risk - Behaviour change is confined to what the ceiling charges a test: less on a busy runner, the same for a real wait. Nothing under `raven/` changes. - `Thread._bootstrap_inner` is private CPython API (present from 3.8 through 3.13); the wrapper is installed once, guarded against a second import of the module, and pinned by a test. If a future interpreter drops it the module fails to import at collection, which is loud. - Rollback: revert the commit; the gate goes back to wall clock minus CPU. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues #428 made the ceiling a gate; #452 warmed the catalogue; #449 halves the workers instead. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
Summary
Since the idle ceiling became a gate (#428), a unit shard has failed on
mainand on most pull requests with every test passing: the ceiling reads wall clock minus CPU and calls the difference waiting, and a thread that is runnable and not on a CPU spends it the same way. With four workers, the controller and coverage on a four-vCPU runner that queueing reached three to four seconds inside one long test, so the shard died on whichever test was executing when contention peaked, a different one each run. Warming the catalogue (#452) did not stop it. #449 halves the workers per shard, which pays for the same measurement with every shard's wall clock.This changes the measurement instead. Idle now subtracts the time the test's threads spent queued for a CPU, read from the scheduler's own account in
/proc/self/task/<tid>/schedstat:asyncio.to_thread;Thread._bootstrap_inner, because the per-test event loop is closed at the end of the test and takes its executor threads with it, and/procforgets a thread the moment it ends;os.times, which keeps the CPU of ended threads and of reaped children; the scheduler's per-thread run time would have been lost the same way the queue time was.A sleeping thread is not on a run queue, so a test that waits on purpose is still charged the whole wait. Without
schedstat(macOS, Windows) the arithmetic is the old one; the strict gate only runs on Linux, and a Linux-only test fails loudly if the runner's kernel stops keeping the statistics rather than letting the gate degrade quietly.-n 4, the 3 s ceiling and the four shards are unchanged. If this lands, #449 is not needed.Type
Verification
Unit tests for the clocks (12 on macOS, 13 on Linux, the last one reads the live kernel):
Full suite on macOS, where the fallback arithmetic runs:
Contrast runs in a Linux container (Docker, linux/arm64, kernel 6.12), the checkout and venv on container-local storage,
mainat 2bdc4bd against this branch. Four workers pinned to two CPUs stands in for the four-vCPU runner; the ceiling is lowered to 0.05 s so the effect shows on one file:The CI shard command, same container, four workers pinned to two CPUs:
Risk
Behaviour change is confined to what the ceiling charges a test: less on a busy runner, the same for a real wait. Nothing under
raven/changes.Thread._bootstrap_inneris private CPython API (present from 3.8 through 3.13); the wrapper is installed once, guarded against a second import of the module, and pinned by a test. If a future interpreter drops it the module fails to import at collection, which is loud.Rollback: revert the commit; the gate goes back to wall clock minus CPU.
Security impact considered
Backward compatibility considered
Rollback path is clear for risky changes
Related Issues
#428 made the ceiling a gate; #452 warmed the catalogue; #449 halves the workers instead.