Repository navigation
chore: set bazel cpu reservation via exec_properties instead of via the cpu:n tag - #10786
Merged
basvandijk merged 5 commits intoJul 16, 2026
Merged
Conversation
|
✅ No security or compliance issues detected. Reviewed everything up to 2911715. Security Overview
Detected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to 2911715. Security Overview
Detected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to 2911715. Security Overview
Detected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to 2911715. Security Overview
Detected Code Changes
|
nmattia
approved these changes
Jul 16, 2026
nmattia
left a comment
nmattia
left a comment
Contributor
There was a problem hiding this comment.
LGTM. For reference: bazelbuild/bazel#29426
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates CPU core reservation for Bazel tests from the cpu:n tag to exec_properties = {"cpu": "n"}, so the reservation is forwarded to Remote Execution (REAPI) and remains effective for local execution.
Changes:
- Replaced
tags = ["cpu:n"]withexec_properties = {"cpu": "n"}across multiple Rust test targets. - Updated the
system_testmacro to set CPU reservation viaexec_propertiesfor the_localvariant and adjusted its documentation accordingly.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rs/tests/system_tests.bzl | Switches local system-test CPU reservation from cpu:n tag to exec_properties. |
| rs/sns/integration_tests/BUILD.bazel | Updates SNS integration tests to reserve CPUs via exec_properties. |
| rs/rosetta-api/icrc1/BUILD.bazel | Updates ICRC1 rosetta test suite to reserve CPUs via exec_properties. |
| rs/rosetta-api/icp/tests/integration_tests/BUILD.bazel | Updates ICP rosetta integration tests to reserve CPUs via exec_properties. |
| rs/rosetta-api/icp/BUILD.bazel | Updates ICP rosetta long test to reserve CPUs via exec_properties. |
| rs/pocket_ic_server/BUILD.bazel | Updates PocketIC server tests to reserve CPUs via exec_properties. |
| rs/nervous_system/integration_tests/BUILD.bazel | Updates nervous system integration tests to reserve CPUs via exec_properties. |
| rs/migration_canister/BUILD.bazel | Updates migration canister test to reserve CPUs via exec_properties. |
| rs/ledger_suite/icrc1/index-ng/BUILD.bazel | Updates index-ng test to reserve CPUs via exec_properties. |
| rs/ledger_suite/icp/ledger/BUILD.bazel | Updates ICP ledger test to reserve CPUs via exec_properties. |
| rs/ledger_suite/icp/index/BUILD.bazel | Updates ICP index test to reserve CPUs via exec_properties. |
| rs/http_endpoints/nns_delegation_manager/BUILD.bazel | Updates delegation manager test to reserve CPUs via exec_properties. |
| rs/execution_environment/BUILD.bazel | Updates execution environment slow tests to reserve CPUs via exec_properties. |
| rs/bitcoin/adapter/BUILD.bazel | Updates bitcoin adapter integration test to reserve CPUs via exec_properties. |
| packages/pocket-ic/BUILD.bazel | Updates pocket-ic package tests to reserve CPUs via exec_properties. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
basvandijk
enabled auto-merge
July 16, 2026 12:25
schneiderstefan
approved these changes
Jul 16, 2026
bjoernek
approved these changes
Jul 16, 2026
basvandijk
deleted the
basvandijk/set-cpu-reservation-via-exec_properties
branch
July 16, 2026 13:52
basvandijk
added a commit
that referenced
this pull request
Sep 14, 2026
## Problem
The `Bazel Build with RBE` step of the `CI RBE Evaluation` workflow
times out on
essentially every push to master:
```
[36,930 / 36,936] [Sched] Compiling Rust bin icp_features (2 files); 5239s ... (3 actions, 0 running)
Error: The action 'Bazel Build with RBE' has timed out after 90 minutes.
```
The action is not compiling slowly — it never starts. `[Sched]` is
Bazel's marker
for *queued on the remote executor*, the number next to it is queue
time, and
`0 running` confirms nothing is executing. The build reaches 36,926 of
36,936
actions 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:
```
11:58:23 [36,926 / 36,936] [Sched] Compiling Rust bin unix (2 files); 4439s ... (5 actions, 0 running)
11:58:46 [36,926 / 36,936] Compiling Rust bin tests (2 files); 0s remote, remote-cache ... (5 actions, 1 running)
11:58:57 [36,928 / 36,936] Compiling Rust bin unix (2 files); 1s remote, remote-cache ...
```
## Cause
Two things combine:
1. **A target-level `exec_properties` applies to every action the target
owns**,
not just the test run. So on a `rust_test`, `exec_properties = {"cpu":
"16"}`
also lands on the `Rustc` action that compiles the test binary. The
`cpu:n`
tag that #10786 replaced only ever affected the test action, so that PR
widened the reservation without meaning to.
2. **Namespace RBE workers run 16 actions concurrently** (hence
`--jobs=240` for
15 workers). An action asking for `cpu: 16` therefore only fits on a
completely drained worker, which under cluster load essentially never
happens.
The `cpu: 8` targets in the same build are unaffected and compiled in
16–27s
throughout, which confirms the threshold is the worker size rather than
target
complexity:
| target | cpu | outcome |
| --- | --- | --- |
| `pocket-ic-server-head-nns` | 8 | compiled in 25s |
| `neuron_test` | 8 | compiled in 16s |
| `upgrade_canister_test` | 8 | compiled in 0–11s |
| `icp_features`, `icp_features-head-nns` | 16 | starved 87 min →
timeout |
This 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
RBE` step with the same signature (`Testing //packages/pocket-ic:slow`
queued for
7154s).
The non-RBE `Bazel Test All` job is unaffected: it runs locally on one
large
runner, where `cpu: 16` is just a local core reservation.
## Fix
Prefix the property with `test.` so it is scoped to the `test` exec
group, i.e.
to the test action only, restoring the semantics of the original `cpu:n`
tag.
This is applied to every `cpu` execution property, not only the ones
that
currently starve — a compile action never needed the reservation, and
reserving
cores for it only wastes RBE capacity.
## Verification
- `bazel build //... --nobuild` analyzes all 3348 targets successfully,
which
also confirms `test` is a valid exec group for every rule touched here
(`rust_test`, `rust_ic_test`, the `rust_test_suite` variants and
`sh_test`);
an unknown exec group fails analysis with *"Tried to set exec_properties
for
non-existent exec groups"*.
- `bazel aquery //packages/pocket-ic:icp_features` shows the reservation
has
moved off the compile action:
```
Rustc cpu-related execution_info: []
TestRunner cpu-related execution_info: [{'key': 'cpu', 'value': '16'}]
```
- `bazel run //:buildifier` reports no formatting changes.
## Follow-up
This does not by itself fix the `Bazel Test with RBE` step, where the
test action
still 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](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Bazel doesn't forward the
cpu:ntag, for reserving cpu cores, to the Remote Execution API. This means that on Remote Build Execution clusters (like @ Namespace) actions don't get the minimal CPUs they specified. To fix this we replacecpu:ntags withexec_properties = {"cpu": "n"}settings which are forwarded to the REAPI and work locally as well.Note that this is supported since bazel-9.2.0. See: bazelbuild/bazel#29419.