[https://nvbugs/6517846][fix] Raise AGG server-ready timeout to 3600s and unwaive 4 perf-sanity cases - #17459
Conversation
|
/bot run --disable-fail-fast --stage-list "GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-*,GB200-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-1,GB200-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-3,GB300-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-1" |
WalkthroughThe pull request increases the aggregated-server readiness timeout from 1800 to 3600 seconds. It also removes several performance-test waivers and adds GLM5 context-only waivers. ChangesPerformance test configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #64938 [ run ] triggered by Bot. Commit: |
|
PR_Github #64938 [ run ] completed with state
|
Correction on the v4-pro case (not unwaived here)My description above understated how healthy the v4-pro run actually is, so to be accurate: Under ( So the workload is functionally fine. Both obstacles are harness-level:
Whether (2) reproduces in Jenkins is open — a shm bus error is also what a container Neither point affects the three cases this PR unwaives; they were verified independently and CI-faithfully. |
v4-pro ctx_only verified passing under this PR (aws-cmh GB300, job 2944440)Re-ran the case with
Both blockers on nvbugs/6517846 are therefore cleared:
This PR now unwaives all 4 cases originally in the cohort. |
|
/bot run --disable-fail-fast --stage-list "GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-,GB200-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-1,GB200-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-3,GB300-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-1,GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-" |
Re-ran the following aggregated perf-sanity cases on current ToT and all three pass end to end, so their waives are stale: | case | job | result | requests | |---|---|---|---| | deepseek_r1_fp4_v2_2_nodes_grace_blackwell-r1_fp4_v2_dep8_mtp1_8k1k | 2929320 | 1 passed in 1386.44s | 10240/10240 ok | | k25_thinking_fp4_2_nodes_grace_blackwell-k25_thinking_fp4_tep8_32k8k | 2929336 | 1 passed in 689.88s | 10/10 ok | | deepseek_r1_fp4_v2_grace_blackwell-r1_fp4_v2_dep4_mtp1_1k8k | 2929319 | 1 passed in 4879.88s | 10240/10240 ok | All three ran CI-faithful (no server-readiness override), finished COMPLETED 0:0 with the launcher's own "Aggregated test completed successfully" line, every MPI rank exiting 0, no bus errors, and report.xml tests=1 failures=0 errors=0 skipped=0. Bug status: nvbugs/6422339 (cases 2 and 3) is already Dev - Closed - Verified. nvbugs/6530213 (case 1) is still open, but the case passes on ToT, so the waive is stale regardless. The fourth case in this cohort, ctx_only-gb300_deepseek-v4-pro-fp4_8k1k_con4301_..._ccb-NIXL (nvbugs/6517846), is deliberately left waived: it still fails, for two independent reasons (server needs ~2000s to become ready vs the 1800s AGG cap, and the MPI worker ranks SIGBUS during teardown even after the benchmark completes). Signed-off-by: chenfeiz0326 <203214996+chenfeiz0326@users.noreply.github.com>
… and unwaive v4-pro ctx_only The gb300 DeepSeek-V4-Pro ctx_only case (con4301) needs ~2000s of model load + autotune before /health answers, but AGG_SERVER_READY_TIMEOUT was 1800s, so it failed readiness while the server was still coming up. The per-case test-db TIMEOUT annotation cannot help, because the wait is computed as min(self.timeout, server_ready_timeout(...)). Raise the agg bound to 3600s, matching the disagg bound, and unwaive the case. Verified on aws-cmh GB300 with the equivalent env override (TRTLLM_TEST_AGG_SERVER_READY_TIMEOUT=3600): the server becomes ready, zero readiness timeouts, and the benchmark completes with 43010/43010 successful requests, 0 failed, 62530.98 tok/s total token throughput. Signed-off-by: chenfeiz0326 <203214996+chenfeiz0326@users.noreply.github.com>
a073cb5 to
6c50743
Compare
|
Rebased onto The conflict was purely positional. Three upstream commits churned the same region of Resolution kept every upstream line and dropped only the four this PR intends to unwaive. Verified post-rebase:
No re-verification run is needed: the rebase changed no test code, and the aws-cmh GB300 verification above already exercised the exact /bot run --disable-fail-fast --stage-list "GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-,GB200-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-1,GB200-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-3,GB300-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-1,GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-" |
|
/bot skip --comment "Only unwaive perf tests, no need to run the whole CI pipeline" |
|
PR_Github #65021 [ skip ] triggered by Bot. Commit: |
|
PR_Github #65021 [ skip ] completed with state |
Summary
Four aggregated perf-sanity cases in
tests/integration/test_lists/waives.txtwere re-run on current ToT. All four pass end to end, so this PR removes their waives.Three were simply stale waives. The fourth —
gb300_deepseek-v4-proctx_only— needed a one-line fix first: it was failing on a server-readiness budget that no per-case knob can raise, so this PR raisesAGG_SERVER_READY_TIMEOUTfrom 1800s to 3600s and unwaives the case alongside it.The readiness fix
test_perf_sanity.pygates server startup on:The
min()means a case's test-dbTIMEOUT (N)annotation cannot buy more startup time — readiness is global per mode, previously 1800s for aggregated and 3600s for disaggregated. The largest aggregated case needs ~2000s of model load + autotune before/healthanswers, so it failed readiness while the server was still coming up correctly.This PR raises the aggregated bound to match the disaggregated one. Nothing else changes: the constant is only ever consumed by that one
min(), and it is still overridable byTRTLLM_TEST_AGG_SERVER_READY_TIMEOUT.Worth noting for reviewers: the cap is newer than the waive it blocks. It was introduced by
ebd197f776(#16403, 2026-07-29), while this case was waived by1ae9b86f12(#16882, 2026-07-27) — two days earlier. So the cap was never the original waive cause; it is a second obstacle that appeared later and would have kept the case red even after the original issue was gone.Cases unwaived
aggr_upload-deepseek_r1_fp4_v2_2_nodes_grace_blackwell-r1_fp4_v2_dep8_mtp1_8k1k1 passed in 1386.44saggr_upload-k25_thinking_fp4_2_nodes_grace_blackwell-k25_thinking_fp4_tep8_32k8k1 passed in 689.88snum_prompts=10, max_concurrency=2)aggr_upload-deepseek_r1_fp4_v2_grace_blackwell-r1_fp4_v2_dep4_mtp1_1k8k1 passed in 4879.88saggr_upload-ctx_only-gb300_deepseek-v4-pro-fp4_8k1k_con4301_ctx12_dep4_gen1_dep8_eplb384_mtp1_ccb-NIXL1 passed in 7041.40sEvery run was CI-faithful — no server-readiness env override. The v4-pro run (2944440) used
test_perf_sanity.pychecked out verbatim from this PR's commit, so the 3600s came from the source constant, not from an env var.Each case was confirmed against independent sources, because a green pytest summary alone is not sufficient here (see below):
1 passedreport.xml:tests="1" failures="0" errors="0" skipped="0"— executed, not deselectedsacct:COMPLETED 0:0Aggregated test completed successfullyBus erroroccurrences<run_dir>/aggr-*/trtllm-benchmark.*.log— note200 OKnever appears in slurm stdout, so the case subdirectory is the only place this can be readv4-pro before / after
sacctCANCELLEDCOMPLETED 0:01 failed in 1802.20s1 passed in 7041.40sError: Aggregated test failedAggregated test completed successfullyBus errorOne caveat reviewers should weigh
An earlier v4-pro run that worked around the readiness cap with an env override reached
1 passed in 7116.39sand still failed the job: after rank 0 shut down cleanly, all three peermgmn_worker_noderanks hitSignal: Bus error (7) / Non-existant physical address→ exit 135, sosrun --kill-on-bad-exit=1returned non-zero and the launcher'scleanup_on_failurefailed the stage.That did not recur in the verification run above (0 bus errors, all ranks 0). But this PR's change cannot have fixed it — raising a readiness timeout does not touch MPI or shared-memory teardown. The honest reading is that the SIGBUS is intermittent, not repaired. One clean run shows it is not deterministic; it is not proof it can never recur.
If it reappears post-merge, it should get its own bug rather than a re-waive of this case — the workload itself is functionally healthy in both runs (43010/43010 successful, 0 failed), so a teardown-ordering fault says nothing about serving correctness.
Two adjacent things that look alarming but are not:
sacctreportingCANCELLED by <uid>is the launcher scancelling itself, not preemption; andTPOT/ITL/User throughputreading0.00withAvg Decoded Tokens per Iter = 1.00is expected for actx_onlycase, which runsrandom_output_len=1and so has no inter-token phase.Bug status
nvbugs/6422339(r1_fp4_v2_dep4_mtp1_1k8k,k25_thinking_fp4_tep8_32k8k) — already Dev - Closed - Verified (fixed 2026-07-20). Cleanly stale.nvbugs/6530213(r1_fp4_v2_dep8_mtp1_8k1k) — still Dev - Open - To fix, but the case passes on ToT, so the waive is stale regardless.nvbugs/6517846(v4-proctx_only) — this PR addresses the readiness half. Two siblinggb300_glm-5-fp4cases under the same bug remain waived and are untouched here; they were not re-tested.The
disagg_upload-v4-pro variants undernvbugs/6581075are also untouched — this PR unwaives only theaggr_upload-ctx_onlyvariant.Test Coverage
The unwaived cases are re-enabled in these post-merge stages, which run pre-merge on this PR:
GB200-4_GPUs-PyTorch-PerfSanity-Post-Merge-*—r1_fp4_v2_dep4_mtp1_1k8kGB200-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-1/-3GB300-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-1GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-*— the v4-proctx_onlycase, confirmed atl0_gb300_multi_gpus_perf_sanity.yml:33and mapped byL0_Test.groovy:5428-5430The
AGG_SERVER_READY_TIMEOUTchange affects every aggregated perf-sanity case, not just the four here, so the whole aggregated set in those stages is the real coverage for it. The change can only lengthen how long a case is willing to wait for a healthy server, so a case that passed before still passes; the risk it carries is the opposite one — a genuinely dead server now burns up to 3600s instead of 1800s before failing.Note the verification runs were executed on GB300 (
supported_gpusis advisory in perf-sanity and never read bytest_perf_sanity.py), so the GB200 stages above are what confirm the GB200 hardware path.PR Checklist
Dev Engineer Review
tests/integration/test_lists/waives.txt.AGG_SERVER_READY_TIMEOUTfrom 1800 to 3600 seconds.ci: full pre-merge approvedlabel was missing.QA Engineer Review
test-db/orqa/files were modified.AGG_SERVER_READY_TIMEOUTintests/integration/defs/perf/test_perf_sanity.py.tests/integration/test_lists/waives.txt.