Repository navigation
ci: run build-wheel and radix-tree benchmark on the CPU runner; let CPU runners use GPU-node CPU - #1730
Conversation
build-wheel is a CPU-only compile (Rust workspace + Python wheel + Go FFI + WASM fixtures) but ran on `k8s-runner-gpu`. That label is currently served by a wedged runner pool pinned to BM.GPU.A10.4 nodes, whose GPUs are fully consumed by other workloads — so the job queued with 'no runner' even though the cluster had ~2600 idle GPU-node CPU cores. - Move build-wheel to `runs-on: k8s-runner-cpu`. It no longer competes for scarce GPU runners. - Add an `nvidia.com/gpu` toleration (operator: Exists) to the CPU runner so it can also schedule onto GPU nodes' stranded CPU. No nodeSelector, so it still runs on plain CPU nodes; robobrain / robobrain-vla / ci-runner taints are deliberately NOT tolerated. Note: the runner-values change requires `helm upgrade k8s-runner-cpu -f scripts/k8s-runner-resources/runner-values-cpu.yaml` to apply — the repo manifests are not auto-reconciled to the cluster. The build-wheel runner stays 8 CPU / 16Gi (down from 32 CPU / 128Gi on the GPU runner); this PR's own build-wheel run on the CPU pool is the live check for fit. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
|
Warning Review limit reached
More reviews will be available in 1 hour, 27 minutes, and 25 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe ChangesCPU Runner Scheduling for build-wheel and Benchmark
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds tolerations to runner-values-cpu.yaml to allow CPU runners to schedule on GPU nodes with the nvidia.com/gpu taint, utilizing idle CPU capacity. The reviewer recommends adding a preferred node affinity to bias scheduling toward plain CPU nodes first, preventing potential resource fragmentation or starvation on GPU nodes. Additionally, they suggest applying these changes consistently to arc-runner-cpu.yaml.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| tolerations: | ||
| - key: "nvidia.com/gpu" | ||
| operator: "Exists" | ||
| effect: "NoSchedule" |
There was a problem hiding this comment.
While tolerating the nvidia.com/gpu taint allows CPU runners to utilize idle CPU capacity on GPU nodes, without a soft node affinity, the Kubernetes scheduler might schedule these CPU-only runners on GPU nodes even when plain CPU nodes have ample capacity. This can lead to CPU/memory fragmentation or starvation on GPU nodes, potentially blocking expensive GPU workloads from scheduling.
To prevent this, it is highly recommended to add a preferred node affinity that biases the scheduler toward plain CPU nodes first, only falling back to GPU nodes when CPU nodes are fully utilized.
Additionally, to maintain consistency across all deployment paths, the same tolerations and affinity should be applied to the alternative CPU runner deployment in scripts/k8s-runner-resources/arc-runner-cpu.yaml.
tolerations:
- key: "nvidia.com/gpu"
operator: "Exists"
effect: "NoSchedule"
affinity:
nodeAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- weight: 100
preference:
matchExpressions:
- key: "nvidia.com/gpu"
operator: "DoesNotExist"References
- To maintain consistency, changes to a feature on one execution path should align with its behavior on related paths.
The radix-tree benchmark only runs `cargo bench` on the kv-index radix tree and throughput suites — pure CPU, no GPU. Move it off `k8s-runner-gpu` onto `k8s-runner-cpu` like build-wheel, so it doesn't queue behind GPU-runner contention. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
|
👋 The PR description doesn't fully follow
Please update the PR description so reviewers have the context they need. |
slin1237
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…rget dir The build-wheel job was slow on every code PR because its caching was ineffective: - The actions/cache 'wheel and Go FFI artifacts' step was keyed on a hash of every crate's source, so it missed on essentially any PR that touches Rust (all-or-nothing). Worse, on a miss it skipped the build via 'if: cache-hit != true', which masked the incremental compilation cache (rust-cache + sccache) and meant rust-cache rarely got saved. - The Go FFI build (bindings/golang/Makefile) overrides CARGO_TARGET_DIR to bindings/golang/target, which Swatinem rust-cache's 'workspaces: .' never covered — so the Go FFI's dependency compilation was never cached across runs at all. Changes: - Remove the wheel-output cache step and the cache-hit guards; the build now always runs and relies on rust-cache + sccache, which degrade gracefully — with the dependency graph cached, only changed crates recompile. - Add bindings/golang/target to rust-cache's cache-directories so the Go FFI build is incremental across runs like the wheel build. Note: the wheel stays manylinux 2_28 + vendored-openssl (portability is required — the cluster has glibc-2.28 nodes), and maturin's --zig cross-compile keeps the wheel and Go FFI on separate toolchains, so this targets cross-run caching rather than within-run dedup. Trade-off: no-Rust PRs lose the ~34s build skip in favor of a fast incremental build, while code PRs (the common case) drop from a full ~8-23m build to an incremental one. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0fbc14c64
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # CPU-only criterion/throughput benchmarks of the kv-index radix tree — | ||
| # no GPU needed, so it runs on the CPU runner pool rather than competing | ||
| # for GPU runners. | ||
| runs-on: k8s-runner-cpu |
There was a problem hiding this comment.
When this workflow runs its scheduled/PR performance benchmarks, switching from k8s-runner-gpu to k8s-runner-cpu also switches hardware: the repo’s k8s-runner-gpu values request 32 CPUs/128Gi, while k8s-runner-cpu requests only 8 CPUs/16Gi. The criterion/throughput jobs here are CPU-bound (including the 256-worker throughput run below), so the reported artifacts become dominated by the smaller runner and are not comparable with prior runs. Please use a CPU runner label with equivalent dedicated resources, or adjust/rename the benchmark to make the new baseline explicit.
Useful? React with 👍 / 👎.
No-op commit to produce a second build-wheel run that restores the rust-cache saved by the previous (cold) run, demonstrating the incremental build time. Can be squashed/dropped before merge. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
1 participant
Summary
Moves CPU-only jobs off the GPU runner pool and lets the CPU runner use GPU-node CPU.
The problem (from a live cluster check):
build-wheeland the radix-tree benchmark are pure compiles/benchmarks — no GPU at build or runtime — but ran onruns-on: k8s-runner-gpu. That label is currently served by a runner pool pinned toBM.GPU.A10.4nodes, whose A10 GPUs are fully consumed by other (VLA/robobrain) workloads, so the runner pods satPendingfor 60–90 min and jobs reported "no runner" — even though the cluster had ~2,600 idle CPU cores on GPU nodes (GPUs get exhausted while CPUs strand).Changes
build-wheel→k8s-runner-cpu(pr-test-rust.yml) — Rust workspace + Python wheel + Go FFI + WASM compile, no GPU.benchmark-radix-tree→k8s-runner-cpu(benchmark-radix-tree.yml) —cargo benchon the kv-index radix tree / throughput suites, CPU-only.nvidia.com/gputaint (runner-values-cpu.yaml,operator: Exists) so it can also schedule onto GPU nodes' stranded CPU. NonodeSelector(still runs on plain CPU nodes);robobrain/robobrain-vla/ci-runnertaints are deliberately not tolerated.To apply the runner change
The
runner-values-cpu.yamledit needs a Helm upgrade — the repo manifests aren't auto-reconciled to the cluster (live scale sets are already drifted from these files):The
runs-onworkflow changes take effect on merge regardless (they route to the existing CPU runners, which already run on the CPU node pool).Caveat / live test
The CPU runner is 8 CPU / 16Gi, down from 32 CPU / 128Gi that build-wheel got on the GPU runner. This PR's own
build-wheelrun executes onk8s-runner-cpu— watch its build time and for OOM. If 16Gi is too tight for a from-scratch workspace compile, the easy follow-up is bumping just the runner memory; kept at 8c/16Gi per the decision to start small.Not touched (separate review)
nightly-benchmark.ymland the disabledgo-bindings-benchmarkjob still targetk8s-runner-gpu; the nightly benchmark may legitimately want GPU.Summary by CodeRabbit