Repository navigation
fix: scope the bazel cpu reservation to the test action - #11567
Merged
Merged
Conversation
PR #10786 replaced the `cpu:n` tag with `exec_properties = {"cpu": "n"}` so that the CPU reservation is forwarded to the Remote Execution API. That worked, but it also widened the reservation: a target-level `exec_properties` applies to *every* action the target owns, so on a `rust_test` the reservation also lands on the `Rustc` action that compiles the test binary. The `cpu:n` tag it replaced only ever affected the test action. On the Namespace RBE cluster this starves the build. Workers run 16 actions concurrently (hence `--jobs=240` for 15 workers), so an action asking for `cpu: 16` only fits on a completely drained worker. With the cluster under load that essentially never happens, and the compile action sits in the scheduler queue instead of running: [36,930 / 36,936] [Sched] Compiling Rust bin icp_features-head-nns (2 files); 5221s ... (3 actions, 0 running) Error: The action 'Bazel Build with RBE' has timed out after 90 minutes. Note the `[Sched]` and `0 running`: the action never started. The build reaches 36,926 of 36,936 actions in 46 minutes and then spends the remaining 44 waiting. When such an action did eventually get a slot it finished in about a second, so this is queueing, not slow compilation. The `cpu: 8` targets in the same build compiled in 16-27s throughout. Prefixing the property with `test.` scopes it to the `test` exec group, i.e. to the test action only, restoring the semantics of the original `cpu:n` tag. Verified with `bazel aquery` on `//packages/pocket-ic:icp_features`: the `Rustc` action now carries no cpu property while the `TestRunner` action keeps `cpu: 16`. This is applied to every `cpu` execution property, not just the ones that currently starve: a compile action never needed the reservation, and reserving cores for it only wastes RBE capacity. Note that this does not by itself fix the `Bazel Test with RBE` step, where the test action still asks for a whole worker. Lowering those reservations below the worker size is left to a follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes consistently scope all CPU execution properties to validated test exec groups without altering requested resources.
Pull request overview
Scopes CPU reservations to test actions, preventing compilation actions from unnecessarily reserving scarce RBE capacity.
Changes:
- Prefixes CPU execution properties with
test.across Rust and shell tests. - Documents test-action scoping in the system-test macro.
File summaries
| File | Description |
|---|---|
rs/tests/system_tests.bzl |
Scopes computed CPU reservations to local test actions. |
rs/sns/integration_tests/BUILD.bazel |
Scopes SNS integration-test CPU reservations. |
rs/rosetta-api/icrc1/BUILD.bazel |
Scopes ICRC Rosetta test CPU reservations. |
rs/rosetta-api/icp/tests/integration_tests/BUILD.bazel |
Scopes ICP Rosetta integration-test reservations. |
rs/rosetta-api/icp/BUILD.bazel |
Scopes ICP Rosetta test reservations. |
rs/pocket_ic_server/BUILD.bazel |
Scopes PocketIC server test reservations. |
rs/nervous_system/integration_tests/BUILD.bazel |
Scopes nervous-system test reservations. |
rs/migration_canister/BUILD.bazel |
Scopes migration-canister test reservations. |
rs/ledger_suite/icrc1/index-ng/BUILD.bazel |
Scopes ICRC index test reservations. |
rs/ledger_suite/icp/ledger/BUILD.bazel |
Scopes ICP ledger test reservations. |
rs/ledger_suite/icp/index/BUILD.bazel |
Scopes ICP index test reservations. |
rs/http_endpoints/nns_delegation_manager/BUILD.bazel |
Scopes delegation-manager test reservations. |
rs/execution_environment/BUILD.bazel |
Scopes execution-environment test reservations. |
rs/bitcoin/adapter/BUILD.bazel |
Scopes Bitcoin adapter test reservations. |
packages/pocket-ic/BUILD.bazel |
Scopes PocketIC package test reservations. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
✅ No security or compliance issues detected. Reviewed everything up to 5ece952. Security Overview
Detected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to 5ece952. Security Overview
Detected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to 5ece952. Security Overview
Detected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to 5ece952. Security Overview
Detected Code Changes
|
mraszyk
approved these changes
Sep 14, 2026
gregorydemay
approved these changes
Sep 14, 2026
dfinityianblenke
approved these changes
Sep 14, 2026
Collaborator
Author
|
Force merging this since it fixes a bug that's effecting all RBE builds and tests. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
Bazel Build with RBEstep of theCI RBE Evaluationworkflow times out onessentially every push to master:
The action is not compiling slowly — it never starts.
[Sched]is Bazel's markerfor queued on the remote executor, the number next to it is queue time, and
0 runningconfirms nothing is executing. The build reaches 36,926 of 36,936actions in 46 minutes and then spends the remaining 44 waiting. When one of these
actions did eventually get a slot it finished in about a second:
Cause
Two things combine:
exec_propertiesapplies to every action the target owns,not just the test run. So on a
rust_test,exec_properties = {"cpu": "16"}also lands on the
Rustcaction that compiles the test binary. Thecpu:ntag that chore: set bazel cpu reservation via exec_properties instead of via the cpu:n tag #10786 replaced only ever affected the test action, so that PR
widened the reservation without meaning to.
--jobs=240for15 workers). An action asking for
cpu: 16therefore only fits on acompletely drained worker, which under cluster load essentially never happens.
The
cpu: 8targets in the same build are unaffected and compiled in 16–27sthroughout, which confirms the threshold is the worker size rather than target
complexity:
pocket-ic-server-head-nnsneuron_testupgrade_canister_testicp_features,icp_features-head-nnsThis has been failing on every master push since
rustc: 1.97.1 -> 1.98.1(#11534) landed, which invalidated the Rust compile cache and forced these test
binaries to be rebuilt. Because the compiles never finish they never populate the
CAS, so each run retries from scratch. Earlier failures hit the
Bazel Test with RBEstep with the same signature (Testing //packages/pocket-ic:slowqueued for7154s).
The non-RBE
Bazel Test Alljob is unaffected: it runs locally on one largerunner, where
cpu: 16is just a local core reservation.Fix
Prefix the property with
test.so it is scoped to thetestexec group, i.e.to the test action only, restoring the semantics of the original
cpu:ntag.This is applied to every
cpuexecution property, not only the ones thatcurrently starve — a compile action never needed the reservation, and reserving
cores for it only wastes RBE capacity.
Verification
bazel build //... --nobuildanalyzes all 3348 targets successfully, whichalso confirms
testis a valid exec group for every rule touched here(
rust_test,rust_ic_test, therust_test_suitevariants andsh_test);an unknown exec group fails analysis with "Tried to set exec_properties for
non-existent exec groups".
bazel aquery //packages/pocket-ic:icp_featuresshows the reservation hasmoved off the compile action:
bazel run //:buildifierreports no formatting changes.Follow-up
This does not by itself fix the
Bazel Test with RBEstep, where the test actionstill asks for a whole worker and can starve the same way. Lowering those
reservations below the worker size is left to a follow-up. It may also be worth
raising the whole-worker starvation with Namespace — a scheduler with reservation
or backfill shouldn't starve a 16-CPU request indefinitely.
🤖 Generated with Claude Code