[DO NOT MERGE] CI - #2196
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
8db6d35 to
0e123d3
Compare
ec72da7 to
63c943b
Compare
fac7194 to
dfc4680
Compare
63c943b to
891358a
Compare
CI went fully green at
|
| Stage | Result |
|---|---|
stage-a-cpu ×4, stage-b-cpu |
SUCCESS |
stage-b-2-gpu-h200, stage-c-2-gpu-h200 ×2 |
SUCCESS |
stage-c-4-gpu-h200 ×3 |
SUCCESS |
stage-c-8-gpu-h100 ×2, stage-c-8-gpu-h200 ×2 |
SUCCESS |
stage-c-4-gpu-mi300x ×2 |
FAILURE — not caused by this PR, see below |
The bypass-fastfail label is on, so each shard runs every test rather than stopping at the first failure. A shard reporting SUCCESS therefore means every test in it passed.
The three long-running failures this closes
test_qwen3_4B_offload_disk— failed deterministically 4× at ~43s. Cause: nothing cleaned up accelerator memory between CI test files. The sglang scheduler is a grandchild of the test process, so per-test teardown never reached it, and the workflow'spkillonly runs once at job start. The CUDA suite runner now reaps between files. Two earlier explanations were refuted by evidence and dropped: disk exhaustion (zeroENOSPC/No space lefthits across two runs) and stale dependency pins (a 4th failure reproduced on freshly pinned sglang).test_qwen3_4B_disk_delta—Weight version mismatch! Engine: default, Updater: 0, reproduced on two separate rounds. Only rank 0 publishes the engine weight version in_capture_baseline, while theci_testcheck runs on every rank with no synchronisation between them. The broadcast and p2p paths already close withdist.barrier(group=get_gloo_group()); the delta path did not. Added.test_deepseek_v4_flash_4layer_ci— all four ranks died together insidewake_upwithconnection error code 2, 119/139 GB of device memory still free, i.e. host-RAM pressure rather than anything in this branch. Not reproduced on the rerun.
Also fixed here
- The sglang launch command is now rendered through the parser's own argparse actions instead of hand-spelled option strings, which fixes
--no-flags, JSON-typed fields, and fields whose flag differs from their dest. The dict input is kept deliberately:ServerArgs.__post_init__is not idempotent (DP attention divideschunked_prefill_sizebydp_sizeon every run), so rendering from an already-post-inited instance breaks the roundtrip assertion whensglang_dp_size > 1. - A disposed rollout cell now unregisters from the router based on whether it actually registered, not on which state it is in. Both registration sites await
add_workerbefore the state advances, so a cell can own a live router entry while still initializing or pending weights — and, in the other direction, can reach either state having never registered at all. - Around 20 tests the base left stale, two infinite hangs, and an
examples/breakage that would have failed withunrecognized arguments.
Caveat: the ROCm lane is failing for a reason outside this PR
stage-c-4-gpu-mi300x fails with ValueError: Unknown suite stage-c-4-gpu-mi300x for backend ROCM.
pr-test-rocm.yml is a pull_request_target workflow, so its definition comes from main, where tests/ci/run_suite.py registers stage-c-4-gpu-mi300x. This PR's base branch does not have that registration yet, so any PR based on it fails this lane identically. On main the mi300x support is a whole feature — the amd label, suite="stage-c-4-gpu-mi300x" on nine e2e files, and an assertion in test_run_suite.py — so pulling it into this stack just to turn the lane green would be the wrong move. Left as is, per @fzyzcjy.
Verification beyond CI
The full tests/fast suite was also run on a separate CPU devbox: 6989 passed, with two failures that are environment-only and pass in CI — the baked sglang in the image predates gated_launch_port, and the router --help subprocess takes 10.05s against its own timeout=10. That run is what caught the dispose regression above before CI did.
Heads-up: no checks are being created for this PR right nowActions is not creating any workflow run for this PR — not Cause. Run 31108219821 is stuck: it reports What I tried, all with no effect: What unblocks it: cancelling or deleting that run from the web UI, or waiting for GitHub to expire it (queued runs age out after about 24h). The lesson, for the record: a job that no runner picks up reports Where the branch actually standsVerified at
Still unverified, all from the interrupted round:
One thing worth a decision: |
CI green on
|
| Stage | Result |
|---|---|
stage-a-cpu (0)(1)(2)(3) |
SUCCESS |
stage-b-cpu |
SUCCESS |
stage-b-2-gpu-h200 |
SUCCESS |
stage-c-2-gpu-h200 (0)(1) |
SUCCESS |
stage-c-4-gpu-h200 (0)(1)(2) |
SUCCESS |
stage-c-8-gpu-h100 (0)(1) |
SUCCESS |
stage-c-8-gpu-h200 (0)(1) |
SUCCESS |
stage-c-4-gpu-mi300x (0)(1) |
FAILURE — unrelated, exempt |
A full tests/fast run on a devbox is green as well: 7065 passed, 20 skipped, 6 xfailed, 0 failed, 0 errors (39m34s, h200-sci-k8s).
Two flakes worth naming
stage-c-4-gpu-h200 (0)test_r3_baseline.pyneeded three attempts;(1)test_int4_rollout.pyneeded two. Both died the same way: a trainer worker vanished withSYSTEM_ERROR ... connection error code 2. End of file, with no CUDA OOM traceback. That config runs a 30B MoE on 4 GPUs with--colocate --optimizer-cpu-offload, so a host OOM-kill fits better than a CUDA one. The e2e itself is untouched by this branch apart from a mechanical rename, and the seed logic is identical to the commit these stages passed on, so this is a margin problem rather than a regression — but 1-pass-in-3 is not a clean flake signal. If it keeps failing on main, the first thing to check is whether this stack raised peak memory on the colocate path.test_deepseekv4's TITO threshold (13/64 = 0.203 > 0.2) failed on an earlier commit and passes now. That ratio is decided by generated text, and this branch had left sglang'srandom_seedunset, so it was redrawn every launch and sat right on the line. The seed is derived from--seedagain, so the ratio is deterministic now. If it starts drifting again, that means the seed stopped reaching the engines.
Weakening audit
Per the review agreement, subagents audited the whole deliver-1-base..HEAD diff (production code, tests, and everything outside miles/) for behaviour or tests that got weaker. Fourteen real weakenings were found and fixed, plus three tests that had been rewritten to assert the weakened behaviour. The most consequential:
--use-fault-tolerancealone no longer healed anything: the health checkers only publish a status, and the only thing that acts on it needed--mini-ft-controller-enableand a non-zero--api-server-port, both off by default, while sixteen launch scripts pass neither. It now drives the cell registry in-process and follows--ft-components.- A pinned port that someone else already held was no longer refused, so the new process would fail to bind and die while whoever waited for the port connected to the stale occupant instead.
- The sglang engine seed was commented out with no way to put it back, so
--seedstopped reaching the engines. - Multi-LoRA on an Inkling checkpoint silently widened the adapter to every compatible module.
- Dumper mode stopped silencing the heartbeats it says it silences (
ft_componentsis resolved before the override runs). --rollout-externalraisedNotImplementedError; it works again, and so does pinning the router port.
A second, adversarial pass over those fixes found four bugs in the fixes themselves — the worst being that the restored seed keyed off the node-local GPU id, which gives every engine of a one-engine-per-node fleet the same seed. All four are fixed.
Seven rollout test modules had decayed into pytest.skip husks kept parseable by a noqa for symbols that no longer exist. Their 61 tests were mapped against the current suite; the uncovered guarantees are covered by new tests and the husks are gone. RolloutServer's three fan-outs had no behavioural test at all, which is how the needs_offload filter, the tags pass-through, and the reach across every cell could each have been dropped silently.
Before merge
15ac046ba is an empty poke commit from unwedging a stuck CI run; worth squashing out.
47e842e to
b2afcf2
Compare
c337eed to
7bbce62
Compare
The previous head The ROCm failures are independent base drift: both shard 0 and shard 1 request |
|
f1b5169 to
0c6786f
Compare
|
CI status for head ece8ee7:
I am treating the repeated H100 failure as a deterministic port-allocation/startup bug rather than a flake. I will reproduce and validate the fix on a tom-remote GPU devbox before pushing a new head and rerunning CI. |
|
CI status summary for head Branch-related failure and fix
Remaining red H200 check
Current rollup
From a code-regression perspective, I consider this head fully validated: every branch-attributable failure has been fixed and the relevant test has passed repeatedly. GitHub is not literally all-green because |
251a554 to
8021c90
Compare
098853c to
8021c90
Compare
|
agent wrong operation, no real merge to main |
ci-sglang-pr: tom/refactor-miles-repo-sglang/deliver-1
ci-megatron-pr: #72