Refresh DeepSeek-V4 B300 SGLang AgentX MTP - #2577
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 如需进行 PR 验证,请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
fc3da7f to
0c6c627
Compare
|
Claude finished @cquil11's task in 7m 52s —— View job PR Review — 1 blocking issue
Summary🔴 One blocking issue: the new Everything else checked out against repo conventions:
One non-blocking observation: |
0c6c627 to
f42a66a
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31621472248 |
| - config-keys: | ||
| - dsv4-fp4-b300-sglang-agentic-hicache-mtp | ||
| scenario-type: | ||
| - agentic-coding | ||
| description: | ||
| - "Refresh the purged B300 DeepSeek-V4-Pro SGLang AgentX TP4, TP8, DP-attention, and HiCache grid on SGLang v0.5.17." | ||
| - "Use native EAGLE MTP (3 steps, top-k 1, 4 draft tokens) and golden synthetic acceptance length 2.49 for throughput; eval retains real verification." | ||
| - "Follow the official SGLang DeepSeek-V4 Blackwell recipe and require nonempty SGLang server metrics." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2577 |
There was a problem hiding this comment.
🔴 BLOCKING: New perf-changelog.yaml entries must be appended to the END of the file.
Why it matters: The file is read chronologically (oldest at top, newest at bottom). This entry is inserted at line 5441 of 5906, ahead of existing entries (e.g. minimaxm3-fp4-b200-dynamo-vllm-mtp #2462 immediately below and everything after), which breaks the ordering.
Fix: Move this entry (lines 5441–5449) to the very bottom of the file, after the current last entry (qwen3.5-fp4-mi355x-sglang-agentic-mtp, PR #2562).
| export AIPERF_HTTP_TCP_USER_TIMEOUT=900000 | ||
| export SGLANG_JIT_DEEPGEMM_FAST_WARMUP=1 | ||
| export SGLANG_OPT_SWA_SPLIT_LEAF_ON_INSERT=1 |
There was a problem hiding this comment.
🔴 This new B300 DeepSeek-V4 SGLang AgentX script never sets SGLANG_TIMEOUT_KEEP_ALIVE, leaving uvicorn's keep-alive at its ~5s default for the whole run, unlike every sibling SGLang AgentX MTP recipe in this directory (glm5.2_fp4_b300/b200/mi355x set 900; qwen3.5 variants set 1800). Add export SGLANG_TIMEOUT_KEEP_ALIVE=900 right after the AIPERF_HTTP_TCP_USER_TIMEOUT=900000 line (~153) to avoid ECONNRESET/terminal warmup failures on inter-turn idle gaps, especially on the DP-attention/high-concurrency arms (conc up to 512).
Extended reasoning...
The bug: dsv4_fp4_b300_sglang_mtp.sh never exports SGLANG_TIMEOUT_KEEP_ALIVE. Since benchmarks/benchmark_lib.sh sets no fallback for this variable either, SGLang's uvicorn server runs the entire sweep with its built-in ~5-second keep-alive timeout.
Why this matters here specifically: AIPerf's agentic replay harness (build_replay_cmd/run_agentic_replay_and_write_outputs, used by this script) pins one pooled HTTP keep-alive connection per session, with a client-side keep-alive window far longer than 5 seconds. During the long inter-turn idle gaps that are inherent to agentic multi-turn replay, a request can arrive on a pooled socket right after uvicorn has already closed it server-side (since the server's 5s timer fired first), producing an ECONNRESET instead of a normal request. Sibling recipes describe this exact failure mode as a terminal warmup failure — not a retryable blip, but one that can kill the whole sweep run.
The strongest evidence this is an accidental omission, not a deliberate choice: every other SGLang AgentX MTP recipe in benchmarks/single_node/agentic/ sets this variable — glm5.2_fp4_b300_sglang_mtp.sh, glm5.2_fp4_b200, and glm5.2_fp4_mi355x all set SGLANG_TIMEOUT_KEEP_ALIVE=900, and the qwen3.5 SGLang variants set 1800. In glm5.2_fp4_b300_sglang_mtp.sh specifically, SGLANG_TIMEOUT_KEEP_ALIVE=900 (line 184) sits immediately after AIPERF_HTTP_TCP_USER_TIMEOUT=900000 (line 178) — the two lines form a matched client/server pair fixing two sides of the same idle-connection problem (client TCP-level timeout vs. server HTTP keep-alive timeout). This DSv4 script copies the client-side half of that pair verbatim (AIPERF_HTTP_TCP_USER_TIMEOUT=900000 at line 153, with an analogous comment about long admission bursts) but drops the server-side half that follows it in every sibling. That is a strong signature of a partial copy-paste, not an intentional decision to leave the default alone.
Step-by-step proof of the failure path:
- Server starts via
sglang.launch_serverwith noSGLANG_TIMEOUT_KEEP_ALIVEenv set → uvicorn keep-alive timeout defaults to ~5s. - AIPerf's agentic replay client opens a pooled connection to the server (or, on the DP-attention arms, through the sglang-router) and reuses it across a session's multi-turn trace.
- Between two turns of a session, the client is idle (e.g. waiting on tool-call/subagent completion) for more than 5 seconds — routine for agentic traces.
- Because the server's keep-alive timer fires before the client's, uvicorn closes the idle socket while the client still considers it open.
- The client's next write on that socket happens after the server already closed it, and Linux answers with
RST, producing a client-sideECONNRESET. - Since this happens during warmup dispatch of the initial large-prompt burst on high-concurrency/DP-attention arms (conc up to 512), the failure is described by the sibling scripts as a terminal warmup failure — the run doesn't just log a retried request, it aborts.
Fix: add export SGLANG_TIMEOUT_KEEP_ALIVE=900 (matching the B300 glm5.2 sibling, since this script shares the same 900000ms AIPERF_HTTP_TCP_USER_TIMEOUT) immediately after the AIPERF_HTTP_TCP_USER_TIMEOUT=900000 export at line 153.
Why nothing else in the diff prevents this: benchmark_lib.sh has no global default for this variable, the router configuration (--connect-timeout-secs 900, --request-timeout-secs 14400) governs router-side timeouts, not the SGLang backend's own uvicorn keep-alive, and the TP-only arms (kv-offloading none, TP4/TP8) connect AIPerf directly to the SGLang backend with no router in between, so nothing shields those arms from the default 5s keep-alive either.
| if [ "${#METRICS_ARGS[@]}" -gt 0 ]; then | ||
| capture_cache_metrics | ||
| trap capture_cache_metrics EXIT | ||
| fi |
There was a problem hiding this comment.
🟡 The guard if [ "${#METRICS_ARGS[@]}" -gt 0 ] at lines 253-256 in dsv4_fp4_b300_sglang_mtp.sh is dead code: METRICS_ARGS is assigned once at line 108 as a fixed two-element array and is never modified, so the condition is always true. This is a minor readability nit — recommend removing the guard and running capture_cache_metrics and trap capture_cache_metrics EXIT unconditionally, matching the sibling recipe qwen3.5_fp4_b300_sglang_mtp.sh, which does exactly this with no guard.
Extended reasoning...
The bug: In benchmarks/single_node/agentic/dsv4_fp4_b300_sglang_mtp.sh, line 108 assigns METRICS_ARGS=(--enable-metrics --enable-cache-report) once, and this array is never reassigned, appended to, or conditionally cleared anywhere else in the script. Later, lines 253-256 guard the cache-metrics capture behind this array's length:
if [ "${#METRICS_ARGS[@]}" -gt 0 ]; then
capture_cache_metrics
trap capture_cache_metrics EXIT
fiBecause METRICS_ARGS always has exactly 2 elements, ${#METRICS_ARGS[@]} is always 2, so the condition 2 -gt 0 is always true. The if block can never be skipped.
Code path: METRICS_ARGS is referenced in exactly three places — the assignment (line 108), its expansion into SGLANG_CMD (line 198, "${METRICS_ARGS[@]}"), and the guard (line 253). Unlike CACHE_ARGS (which starts empty and is populated only inside the require_agentic_kv_offload_backend hicache branch) or PARALLEL_ARGS (which branches on DP_ATTENTION), there is no code path in this script that leaves METRICS_ARGS empty or shrinks it. The DP_ATTENTION branch only touches PARALLEL_ARGS.
Why nothing catches this today: The script is a shell recipe with no linter enforcing dead-conditional detection, and the guard reads as plausible defensive code — a reviewer skimming it would reasonably assume METRICS_ARGS can be empty in some arm of the recipe, since that's the pattern used elsewhere in the same file for CACHE_ARGS. It isn't until you trace all three references that the always-true nature becomes apparent.
Impact: None functionally — capture_cache_metrics and the EXIT trap always run today regardless of the guard, so behavior is correct. The only cost is misleading readability: a future maintainer might assume there's a code path where metrics capture is skipped, and either preserve the guard when refactoring (carrying the confusion forward) or spend time looking for the non-existent empty-array case.
Proof by exhaustive trace:
- Line 108:
METRICS_ARGS=(--enable-metrics --enable-cache-report)— this is the only assignment in the file. grep -n METRICS_ARGS benchmarks/single_node/agentic/dsv4_fp4_b300_sglang_mtp.shreturns exactly 3 lines: 108 (assignment), 198 (expansion inSGLANG_CMD), 253 (the guard).- No
+=, no reassignment, no conditional that could zero it out — contrast withCACHE_ARGS=()at line 34, which starts empty and is only populated insideif require_agentic_kv_offload_backend hicache; then ... fi. - Therefore for every possible combination of
TP,DP_ATTENTION,KV_OFFLOADING, etc.,${#METRICS_ARGS[@]}evaluates to2at line 253, and[ "2" -gt 0 ]is always true. - Corroborating evidence: the sibling recipe
qwen3.5_fp4_b300_sglang_mtp.shcallscapture_cache_metricsand installstrap capture_cache_metrics EXITunconditionally (no guard at all), confirming the intended/idiomatic form in this codebase is to run these unconditionally.
Fix: Remove the if guard and run the two statements unconditionally:
capture_cache_metrics
trap capture_cache_metrics EXITThis is a strictly simplifying, behavior-preserving cleanup — a pure readability nit, not a functional bug, so it should not block merging.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31621812330 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31624943822 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31656586534 |
Summary
sglang:server metrics.This supersedes the B300 portion of #2145 and the narrower stale #2452 proposal.
Production data audit
The current production database has no aggregate B300 DeepSeek-V4 SGLang AgentX MTP results, so this refresh does not duplicate an up-to-date submission.
Validation