Skip to content

[Rust] Gate health on startup warmup completion - #37994

Merged
alisonshao merged 1 commit into
sgl-project:mainfrom
nvpohanh:codex/fix-rust-openai-parity-ci
Sep 9, 2026
Merged

alisonshao merged 1 commit into
sgl-project:mainfrom
nvpohanh:codex/fix-rust-openai-parity-ci

Conversation

@nvpohanh

@nvpohanh nvpohanh commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

[by Codex]

Summary

  • Align Rust frontend readiness with the Python frontend: a successful health response must mean startup warmup has completed.
  • Keep Rust /health and /health_generate at 503 until startup warmup has completed successfully.
  • Mark the main process's language, embedding, or multimodal warmup request and flip Rust readiness only after that request returns 2xx.
  • Preserve --skip-server-warmup: when it is explicitly set, Rust starts ready.
  • Leave test_openai_completion_rust.py unchanged; radix cache and normal server warmup remain enabled.

CI impact

This fixes a recurring flake in test/registered/openai_server/basic/test_openai_completion_rust.py that has broken pre-merge CI for many unrelated changes. I found the same failure in at least eight pre-merge runs across seven PRs: #28403, #30315, #33068, #35599, #37284, #37601 (twice), and #37820. The test compares Python and Rust frontend logprobs exactly, so the readiness race could fail an otherwise healthy PR without any relevant code changes.

Error and root cause

The Python frontend keeps both health endpoints at 503 while ServerStatus is Starting, and changes the status only after startup warmup succeeds. The Rust listener is intentionally bound before the main process runs _execute_server_warmup, because the warmup path first queries /model_info. However, the Rust health handler previously submitted its own one-token scheduler probe immediately and returned 200 when that probe made progress. Rust could therefore advertise readiness earlier than Python: popen_launch_server could observe a healthy server and send the parity test's completion while the real eight-token startup warmup was still running.

With radix cache enabled, this race changed which request populated or reused the shared prompt prefix. The Python launch commonly evaluated the completion with #new-token: 3, #cached-token: 2/3, while the raced Rust launch evaluated the full prompt with #new-token: 5/6, #cached-token: 0, or co-batched it with warmup. The output tokens were the same, but the different forward-pass shape could produce bitwise-different logprobs on the CI GPU. Radix cache exposed the startup race; it was not the underlying bug.

This behavior originated with the native Rust health handlers in #32876. The parity test was added in #33103. The earlier flake caused by default prefill CUDA graphs (#33352) was a separate issue and was fixed by #34146.

Why this works

The main process adds an internal marker header only to the startup warmup request. Rust middleware recognizes that marker only on the three actual warmup routes (/generate, /encode, and /v1/chat/completions) and records readiness after a successful response. Both health modes check the same atomic readiness state before returning 200 or running a generation probe. Therefore Rust now follows the Python frontend's ServerStatus.Starting contract: health remains 503 during warmup and a health success implies that startup warmup has completed.

If warmup fails, readiness remains false. If warmup is explicitly skipped, the launch-time flag initializes readiness true.

Validation

  • Mandatory full pre-commit command passed, including Rust workspace clippy and rustfmt.
  • Focused Rust tests passed for readiness transitions and pre-warmup health returning 503.
  • ComputeLab L40S: unchanged test_openai_completion_rust.py passed with normal warmup and radix cache enabled (1 passed, 421.484 s).
  • The Rust run log showed startup warmup first (POST /generate 200, 6 new / 0 cached), then health (GET /health_generate 200), then the tested completion (3 new / 2 cached), matching the Python request shape.

Failure being addressed: https://github.com/sgl-project/sglang/actions/runs/33831875719/job/100902930637?pr=35599


CI States

Latest PR Test (Base): ✅ Run #34100216762
Latest PR Test (Extra): ✅ Run #34414379326
Latest PR Test (AMD ROCm 10): ❌ Run #34100215649

@nvpohanh
nvpohanh force-pushed the codex/fix-rust-openai-parity-ci branch from 63de001 to 4946e5a Compare September 4, 2026 13:36
@nvpohanh nvpohanh changed the title [CI] Isolate Rust completion parity from warmup state [Rust] Gate health on startup warmup completion Sep 4, 2026
@nvpohanh

nvpohanh commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

cc @rainj-me @mrain

@nvpohanh
nvpohanh marked this pull request as ready for review September 4, 2026 13:59
@nvpohanh

nvpohanh commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

/rerun-tests test/registered/openai_server/basic/test_openai_completion_rust.py

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Results for /rerun-tests test/registered/openai_server/basic/test_openai_completion_rust.py:

🚀 1-gpu-5090 (1 test): ✅ View workflow run

cd test/ && python3 registered/openai_server/basic/test_openai_completion_rust.py

Comment thread rust/sglang-server/src/api_server/app.rs
@b8zhong b8zhong added the run-ci label Sep 4, 2026
@nvpohanh

nvpohanh commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

@nvpohanh
nvpohanh force-pushed the codex/fix-rust-openai-parity-ci branch from 4946e5a to 3a28ccd Compare September 7, 2026 08:22
@nvpohanh

nvpohanh commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

/rerun-failed-ci

@nvpohanh
nvpohanh enabled auto-merge (squash) September 8, 2026 13:10
@nvpohanh

nvpohanh commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

All NV pipelines have passed

@alisonshao
alisonshao self-requested a review September 9, 2026 18:51
@alisonshao
alisonshao disabled auto-merge September 9, 2026 20:15
@alisonshao
alisonshao merged commit 51c8581 into sgl-project:main Sep 9, 2026
374 of 437 checks passed
pllimax added a commit to pllimax/sglang that referenced this pull request Sep 10, 2026
* origin/main: (27 commits)
  [Simulator] Give the OFFLINE/BLOCKING comparison tolerances real headroom (sgl-project#38732)
  [Config] msgspec.Struct for the config tier (sgl-project#38753)
  [AMD] ci: move the miles nightlies from rocm700 to rocm10 (sgl-project#37495)
  [Config] One writer for the declaration stash; no exception to the write seal (sgl-project#38752)
  docker(xpu): drop redundant setvars.sh from torch_memory_saver RUN (sgl-project#38665)
  [XPU][Fix] Pack device-pointer tables as uint64 to avoid 64-bit address overflow (sgl-project#35051)
  [CI] Temporarily disable GB300 tests (sgl-project#38770)
  [diffusion] feat: spill large tensors over shared memory like numpy arrays (sgl-project#38656)
  [diffusion] refactor: refactor utility ownership and document helper placement (sgl-project#38699)
  [NPU]Support GLM5.2 and FP8 DSA&Indexer kvcache for 950 (sgl-project#38250)
  [CI] Answer unrecognized slash commands instead of skipping silently (sgl-project#38736)
  [AMD] Parallelize aiter spec-decode KV index building over token blocks (sgl-project#37659)
  [DSv4] Integrate TRT-LLM DSv4 Attention for SM100/103 (sgl-project#30805)
  Add Opt-In for GLM-5.3 Flash breakable prefill CUDA graphs (sgl-project#38522)
  [CI] Install helion 1.4.0 for the KDA Helion kernel tests (sgl-project#38688)
  [Rust] Gate health on startup warmup completion (sgl-project#37994)
  [HiCache] Replace skip_lock_node_ids with a segment lock protocol (sgl-project#36848)
  feat: add optimized Domino rollout to DFlash V2 (sgl-project#36899)
  [CI] Add /run-full-ci and /run-extra-ci slash commands (sgl-project#38734)
  [Model] Support GLM-5.3 Flash NVFP4 loading (sgl-project#38621)
  ...
mqhc2020 pushed a commit to mqhc2020/sglang that referenced this pull request Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants