Skip to content

ci(bench-gpu): stop building on half-provisioned or bad-RAM boxes - #939

Merged
MauroToscano merged 2 commits into
mainfrom
fix/gpu-bench-provisioning
Aug 18, 2026
Merged

ci(bench-gpu): stop building on half-provisioned or bad-RAM boxes#939
MauroToscano merged 2 commits into
mainfrom
fix/gpu-bench-provisioning

Conversation

@ColoCarletti

Copy link
Copy Markdown
Collaborator

The GPU ABBA bench kept failing on rented Vast boxes in ways that looked like code bugs but were the harness building before the box was ready:

  • The provisioning-complete check fell back to "these few artifacts exist" and started the build while onstart was still populating the sysroot, so the C compiler read a half-written header (truncated bits/timex.h -> "unterminated #ifndef"). Require the "=== done ===" marker only; drop the premature fallback.
  • Add a toolchain sanity gate (trivial gcc + rustc compile) after provisioning: a bad-RAM host that SIGSEGVs the compiler on the first heavy crate (jemalloc, serde_derive) now fails fast here with a clear message instead of mid-build with an internal-compiler-error backtrace.
  • Cap the dual build at CARGO_BUILD_JOBS=8 so the initial ramp (LLVM codegen units + jemalloc's nested make -j) can't transiently exceed the box's RAM and trigger OOM-induced compiler crashes.
  • Filter offers by reliability>=0.95 to skip chronically-flaky hosts before renting (fails safe: over-strict just yields no offers).

A full box-reroll (rent another host on a build/prove failure) is the next step but needs a live run to validate against paid infra, so it is left out of this change.

The GPU ABBA bench kept failing on rented Vast boxes in ways that looked like
code bugs but were the harness building before the box was ready:

- The provisioning-complete check fell back to "these few artifacts exist"
  and started the build while onstart was still populating the sysroot, so
  the C compiler read a half-written header (truncated bits/timex.h ->
  "unterminated #ifndef"). Require the "=== done ===" marker only; drop the
  premature fallback.
- Add a toolchain sanity gate (trivial gcc + rustc compile) after
  provisioning: a bad-RAM host that SIGSEGVs the compiler on the first heavy
  crate (jemalloc, serde_derive) now fails fast here with a clear message
  instead of mid-build with an internal-compiler-error backtrace.
- Cap the dual build at CARGO_BUILD_JOBS=8 so the initial ramp (LLVM codegen
  units + jemalloc's nested make -j) can't transiently exceed the box's RAM
  and trigger OOM-induced compiler crashes.
- Filter offers by reliability>=0.95 to skip chronically-flaky hosts before
  renting (fails safe: over-strict just yields no offers).

A full box-reroll (rent another host on a build/prove failure) is the next
step but needs a live run to validate against paid infra, so it is left out
of this change.
@ColoCarletti

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

No actionable issues found in the PR changes.

Comment thread .github/workflows/benchmark-gpu.yml Outdated
@claude

claude Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Review — CI-only change, no runtime/prover code touched

The four changes are individually sound and the reasoning in the comments is accurate. Verified against the repo: bench_abba.sh builds cli_B then cli_A sequentially (scripts/bench_abba.sh:211-213), so CARGO_BUILD_JOBS=8 really does cap the whole dual build at 8 rustc + jemalloc's make -j8 (jemalloc-sys reads cargo's NUM_JOBS) — no accidental 2x. The DONE/break rewrite is correct (the loop previously exit 0'd the step; it now falls through to the gate), and 25 min of provisioning wait is comfortably inside timeout-minutes: 330.

No Critical/High issues. Three Low ones:

1. The toolchain sanity gate tests a different toolchain than the build. Details inline — short version: the sysroot corruption this PR is chasing is under /opt/lambda-vm-sysroot, compiled by clang --target=riscv64 --sysroot=... (Makefile:87), while the gate uses host cc + /usr/include; and ~/.cargo/bin/rustc from $HOME resolves the default toolchain, not the pinned 1.94.0. Dropping the artifact fallback also means nothing verifies the sysroot exists anymore.

2. reliability>=0.95 failure mode is hard to diagnose. Combined with gpu_frac=1 + the price cap the pool was already ~7 offers; if the added filter empties it (or the field name is rejected server-side — vastai search offers output is || true'd into an empty offers.json), the job spins 20 × 30 s and then dies with an error string that never mentions reliability:

::error::No RTX 5090 offer matched after ... (>=16 cores, >=48GB RAM, >=64GB disk, driver>=..., <= $.../hr)

Worth adding reliability>=0.95 to that message (benchmark-gpu.yml:263) so the next empty-pool run is one log line to diagnose.

3. The premature-fallback bug still lives in gpu-tests.yml. .github/workflows/gpu-tests.yml:173 has the exact grep '=== done ===' || { cargo && stdlib.h && .git; } fallback this PR just removed, and that workflow builds on the box too — so it can still start a build on a half-populated sysroot and hit the same truncated-header failure. It already rerolls boxes, so tightening it there is cheap. Out of scope for this PR, but shouldn't be left as the last copy of the pattern.

Nothing blocking.

@github-actions

Copy link
Copy Markdown

AI Review

PR #939 · 1 changed files

Findings

Status Sev Location Finding Found by
confirmed medium .github/workflows/benchmark-gpu.yml:394 Toolchain sanity check cannot detect a broken compiler (set -e + && + trailing rm masks the exit code) glm
openrouter/z-ai/glm-5.2

Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro).

AI-002: Toolchain sanity check cannot detect a broken compiler (set -e + && + trailing rm masks the exit code)
  • Status: confirmed
  • Severity: medium
  • Location: .github/workflows/benchmark-gpu.yml:394
  • Found by: glm:openrouter/z-ai/glm-5.2
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

The new toolchain sanity check almost always returns success (exit 0) even when both cc and rustc are broken (e.g. SIGSEGV from bad RAM), which is the exact failure mode it is meant to catch. The remote script is set -e; ...; cc ... &amp;&amp; "$d/tc"; ...; rustc ... &amp;&amp; "$d/tr"; rm -rf "$d". Under bash set -e, a command that fails while it is part of a &amp;&amp;/|| list (except the final command of that list) is explicitly ignored: per the bash manual, "If a compound command ... returns a non-zero status because a command failed while -e was being ignored, the shell does not exit." So when cc (or rustc) fails, it is the non-final command of its &amp;&amp; list, set -e does NOT exit, and execution falls through to the next line. The very last command is rm -rf "$d", which succeeds, so the script's exit code is 0. ! 0 is false, so the error branch is skipped and the check reports "toolchain sane". The only failures it can actually catch are runtime crashes of the trivial compiled binaries (the final command of each &amp;&amp;), which essentially never happen for int main(){return 0;} / fn main(){} — the realistic failure (the compiler itself crashing) is in the excepted, uncaught position.

Evidence

Lines 394-399: if ! $SSH 'set -e; d=$(mktemp -d); printf ... &gt; "$d/t.c"; cc -O2 "$d/t.c" -o "$d/tc" &amp;&amp; "$d/tc"; printf ... &gt; "$d/t.rs"; "$HOME/.cargo/bin/rustc" -O "$d/t.rs" -o "$d/tr" &amp;&amp; "$d/tr"; rm -rf "$d"'. cc and rustc are the non-final commands of their &amp;&amp; lists, so set -e ignores their failures; the trailing rm -rf "$d" then exits 0 and masks the result. The comment (lines 387-391) explicitly states the goal is to catch "gcc/rustc SIGSEGV on the first heavy crate" — i.e. the compiler crashing — which is precisely the case this structure does not detect.

Suggested fix

Drop the &amp;&amp; and use ; so each compile/run is its own simple command under set -e (set -e then exits on a compiler failure), e.g. ...; cc -O2 "$d/t.c" -o "$d/tc"; "$d/tc"; ...; "$HOME/.cargo/bin/rustc" -O "$d/t.rs" -o "$d/tr"; "$d/tr"; rm -rf "$d". If cleanup-on-failure matters, wrap the body in a subshell with a trap rather than relying on the trailing rm to be the last command.

Reviewer Lanes

Lane Model Prompt Status Findings
glm openrouter/z-ai/glm-5.2 general success 1
kimi openrouter/moonshotai/kimi-k2.7-code general success 0
minimax minimax/MiniMax-M3 general error: opencode failed (provider/auth/runtime error) and no findings were submitted 0
moonmath zro/minimax-m3 general error: opencode failed (provider/auth/runtime error) and no findings were submitted 0
nemotron openrouter/nvidia/nemotron-3-ultra-550b-a55b general success 3

Verification Lanes

Lane Model Status Confirmed Rejected Uncertain
deepseek-verifier openrouter/deepseek/deepseek-v4-pro success 1 3 0

Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report.

Discarded candidates (3) — rejected by the verifier
  • Reliability filter may be too aggressive for RTX 5090 pool (.github/workflows/benchmark-gpu.yml:246, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The reliability>=0.95 filter is an intentional design choice with an explicit comment acknowledging the trade-off ('Over-strict just yields no offers, surfaced by the retry loop's "No offer"'). The retry loop (20 attempts × 30s) exists exactly to handle transient scarcity. The claim that reliability scores 'may not be well-populated for newer GPUs' is pure speculation with no evidence. This is a design opinion, not a bug.
  • Toolchain sanity check assumes specific compiler paths (.github/workflows/benchmark-gpu.yml:394, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — cc is the universally-available C compiler command on POSIX systems. $HOME/.cargo/bin/rustc is the standard rustup installation path. The onstart template is controlled by the same repository team and installs Rust via rustup — there is zero evidence it would place rustc elsewhere. The claim about '/opt/rust' or '/usr/local/cargo' is baseless speculation.
  • CARGO_BUILD_JOBS=8 may underutilize larger boxes (.github/workflows/benchmark-gpu.yml:463, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The comment at lines 451-457 explicitly justifies CARGO_BUILD_JOBS=8 as a deliberate safeguard against RAM pressure: 'Uncapped, cargo runs one rustc per core (16-32 here)... can transiently exceed the box's RAM — memory pressure that shows up as compiler SIGSEGV. 8 leaves ~6 GB/job on the >=48 GB floor.' This is not a bug or oversight; it's an intentional, documented trade-off prioritizing reliability over build speed. The author is aware cores may be idle and chose that deliberately.

Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts.

* fix(bench-gpu): make the toolchain gate able to fail, and say why

Follow-ups from review of the provisioning hardening.

- The sanity gate could not fail on a compiler failure. Under `set -e` a
  non-final operand of an `&&` list is exempt from errexit, and the list's
  non-zero status does not re-trigger it, so a dead cc/rustc was swallowed
  and the remote exit status was that of the trailing `rm -rf`. The gate
  returned 0 and printed "toolchain sane" on a host whose compiler had just
  crashed. Measured, before -> after: cc SIGSEGV 0 -> 139, cc missing
  0 -> 127, cc error 0 -> 1, rustc SIGSEGV 0 -> 139, rustc missing 0 -> 127,
  healthy 0 -> 0. Every command is now a bare statement; a trap keeps the
  tmpdir cleanup on both paths.
- Distinguish ssh's own exit 255 from a verdict on the toolchain, so a
  network blip no longer reports the host's compilers as broken.
- Run the probe from the repo so rustup resolves the pinned toolchain in
  rust-toolchain.toml rather than whatever default the image carries.
- A failure in this step posted "Run failed" above an EMPTY code block: the
  PR-comment step tails $RUNNER_TEMP/abba_out.txt, and only the bench step
  ever wrote it. Record the reason and the compiler output there.
- Reword the gate's error. It establishes "cc or rustc could not compile and
  run a trivial program"; bad RAM is named as one possible cause rather than
  asserted as the diagnosis.

Comments, each previously at odds with the code or with each other:

- the gate blamed bad RAM while the CARGO_BUILD_JOBS comment blamed memory
  pressure for the same symptom. The latter now describes OOM as it actually
  presents (SIGKILL, or an allocation failure) and names jemalloc-sys's
  CARGO_MAKEFLAGS forwarding, which is what makes the cap bind its nested make.
- drop the unmeasured "~10 min dual build", and annotate the 3 min 56 s ETA
  reference as a pre-cap measurement that CARGO_BUILD_JOBS=8 will raise.
- the no-offer error and the env header now list reliability, gpu_frac and
  cuda_max_good, which they had drifted from.
- state the gate's scope: it does not exercise /opt/lambda-vm-sysroot, and a
  1 s compile surfaces marginal RAM only sometimes.

* fix(bench-gpu): tell the operator to wait before re-rolling the box

Both host-fault messages said "Re-run /bench-gpu to reroll the box", but
offer selection is deterministic — `sort_by(.dph_total) | reverse | .[0]`
with no machine_id exclusion — so an immediate re-run can re-pick the same
machine once it relists and fail identically. Say to wait a few minutes
instead, and say why, so the advice matches what the picker actually does.

The ssh-255 message is left as an immediate retry: a transport failure is
not a verdict on the host, so there is nothing to roll off.

Still not an automated reroll (the sibling gpu-tests.yml carries a TRIED
machine_id list for that); this only stops the message promising something
the selection logic does not do.
@MauroToscano
MauroToscano enabled auto-merge August 18, 2026 22:20
@MauroToscano
MauroToscano added this pull request to the merge queue Aug 18, 2026
Merged via the queue into main with commit bc3a3c6 Aug 18, 2026
15 checks passed
@MauroToscano
MauroToscano deleted the fix/gpu-bench-provisioning branch August 18, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants