Skip to content

fix(pd): disable overlap for spec+grammar in disagg decode loop - #28039

Merged
hnyls2002 merged 2 commits into
sgl-project:mainfrom
whybeyoung:fix/disagg-decode-overlap-grammar-desync
Jun 12, 2026
Merged

hnyls2002 merged 2 commits into
sgl-project:mainfrom
whybeyoung:fix/disagg-decode-overlap-grammar-desync

Conversation

@whybeyoung

@whybeyoung whybeyoung commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

In PD-disaggregation decode mode, event_loop_overlap_disagg_decode never applied the overlap + spec + grammar guard that the non-disaggregation event_loop_overlap has via is_disable_overlap_for_batch (the need_grammar_sync branch, added in #13425). This loop unconditionally overlapped every batch.

Root cause

The guard is gated on batch.is_spec_v2. Previously EAGLE/MTP ran on the spec V1 worker, so is_spec_v2 was False and this code path was never reached — the bug stayed dormant.

After #25464 (Deprecate Spec V1) and #27607 (remove V1 worker), EAGLE/MTP now always run on spec V2. Grammar tokens are accepted through the spec-V2 path (_apply_decode_grammar, accepting a list of speculated tokens). Because the disagg decode loop lacks the guard, a grammar rejection that triggers an abort makes one DP rank diverge under overlap (it issues a different number of forward/collective ops than its peers). That rank never returns to the top of the loop, so the remaining DP ranks deadlock in the per-iteration request broadcast (_broadcast_reqs_across_ranks → broadcast_pyobj, gloo). The whole engine hangs; /health stops responding and the pod is killed by the liveness probe.

Trigger conditions

disaggregation_mode=decode + EAGLE/MTP (spec V2) + grammar/constrained decoding (xgrammar) + DP attention. Other combinations don't reach the diverging path.

Fix

Apply is_disable_overlap_for_batch in event_loop_overlap_disagg_decode, mirroring event_loop_overlap: when spec + grammar is active, process the previous batch before launching the current one so all DP ranks make the same overlap decision and stay in lockstep.


CC @hnyls2002 @ShangmingCai


CI States

Latest PR Test (Base): ⏳ Run #27441929435
Latest PR Test (Extra): ❌ Run #27441929257

event_loop_overlap_disagg_decode never applied the overlap+spec+grammar
guard that event_loop_overlap has. With EAGLE now on spec V2, a grammar
abort desyncs one DP rank under overlap and the whole DP group deadlocks
in the per-iteration request broadcast. Apply is_disable_overlap_for_batch
so all DP ranks make the same overlap decision.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the event_loop_overlap_disagg_decode function in decode.py by introducing a helper function pop_and_process to handle popping and processing batch results. It also adds logic to check if overlapping should be disabled for a batch (e.g., to prevent DP rank desynchronization when combining overlap, speculative decoding, and grammar). If overlapping is disabled, the last batch is processed immediately before launching the current batch; otherwise, it is processed afterwards. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@whybeyoung

Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

@hnyls2002

Copy link
Copy Markdown
Collaborator

/rerun-test registered/disaggregation/test_disaggregation_basic.py

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Results for /rerun-test registered/disaggregation/test_disaggregation_basic.py:

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

cd test/ && python3 registered/disaggregation/test_disaggregation_basic.py

@hnyls2002
hnyls2002 merged commit 1e71c1a into sgl-project:main Jun 12, 2026
104 of 137 checks passed
Chronostasys pushed a commit to MindLab-Research/sglang that referenced this pull request Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants