Repository navigation
[Fix] Skip cross-node probe in MultimemAllGatherer on single-node runs (fixes mooncake EP segfault) - #30139
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
/rerun-test test_mooncake_ep_small.py |
|
Results for 🚀 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ec8e2f401
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # in_the_same_node_as() all-reduce, which can segfault under some | ||
| # EP/mooncake setups, and keep multimem enabled. | ||
| if ( | ||
| get_global_server_args().nnodes > 1 |
There was a problem hiding this comment.
This new get_global_server_args() call grows the ratcheted legacy accessor count in python/sglang/srt from 346 to 347, so test/registered/unit/test_legacy_global_ratchet.py will fail with “New code must use the sglang.srt.runtime_context accessors”. Use sglang.srt.runtime_context.get_server_args() here instead of adding another legacy call-site.
Useful? React with 👍 / 👎.
5ec8e2f to
f84b15d
Compare
|
/rerun-test test_mooncake_ep_small.py |
|
/tag-and-rerun-ci |
|
Results for 🚀 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f84b15dd42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR #29881 added an in_the_same_node_as(tp_group.cpu_group) probe to MultimemAllGatherer.__init__ to disable the logits multimem all-gather on cross-node TP groups. That probe does a gloo all_reduce during logits-processor construction, which segfaults under the mooncake EP + DP-attention config (tp4/dp2/ep4, DeepEP, single node), crashing the server at startup (test/registered/ep/test_mooncake_ep_small.py, scheduled full run). On a single-node deployment every TP rank is trivially co-located, so the probe is unnecessary. Gate it on nnodes > 1: single-node runs skip the fragile all_reduce and keep multimem enabled (pre-#29881 behavior), while genuine multi-node deployments still get #29881's cross-node guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
f84b15d to
e2bdfb2
Compare
…s (fixes mooncake EP segfault) (sgl-project#30139)
Problem
test/registered/ep/test_mooncake_ep_small.pycrashes the server at startup with a segmentation fault in today's scheduled run:→
Server process exited with code 1→FAILED (errors=1, skipped=6).Root cause
PR #29881 ("Avoid logits multimem all-gather on cross-node TP groups") added an
in_the_same_node_as(tp_group.cpu_group, source_rank=0)probe toMultimemAllGatherer.__init__. That probe runs a glooall_reduceduring logits-processor construction, and it segfaults under the mooncake EP + DP-attention config (tp4 / dp2 / ep4, DeepEP,nnodes=1).Timeline for this test: it last passed on 0702_23; broke on 0703 with the router NaN crash (#29771, fixed by #30079); and now fails on 0704 with this segfault from #29881. Both fixes are needed for it to go green — #30079 is already merged, this PR is the second half.
Fix
On a single-node deployment every TP rank is trivially co-located, so the cross-node probe is unnecessary. Gate it on
nnodes > 1:all_reduceand keep multimem enabled (pre-Avoid logits multimem all-gather on cross-node TP groups #29881 behavior),This unblocks CI and sidesteps the deeper question of why that gloo
all_reducesegfaults under mooncake/DeepEP (worth a follow-up with the #29881 author). Needs a 4-GPU mooncake-EP run to confirm green (I can't run that locally).🤖 Generated with Claude Code
CI States
Latest PR Test (Base): 🚫 Run #28721778107
Latest PR Test (Extra): ❌ Run #28721778022