Skip to content

[v0.26.0rc][BugFix] resolve shape mismatch in DCP PD-disaggregated recomputation (#16487) - #16833

Open
lllyys wants to merge 1 commit into
vllm-project:releases/v0.26.0rcfrom
lllyys:cp/16487-v0.26.0rc
Open

lllyys wants to merge 1 commit into
vllm-project:releases/v0.26.0rcfrom
lllyys:cp/16487-v0.26.0rc

Conversation

@lllyys

@lllyys lllyys commented Sep 18, 2026 •

Copy link
Copy Markdown

What this PR does / why we need it?

Cherry-pick of #16487 to releases/v0.26.0rc.

Fix shape mismatches when DCP is enabled with PD-disaggregated recomputation:
short recomputation requests (including last-token recomputation) are now
correctly classified as decode requests in the attention, MLA, and SFA
metadata builders. DCP state is cached during builder initialization and the
builder's own configuration is used, so metadata construction works outside
the current-config context.

Original PR: #16487 (main commit cab3719)

Backport conflicts and how they were resolved

The cherry-pick conflicted in 4 files (this branch carries 165 backports of
its own; test_attention_cp.py / test_mla_v1.py live under
tests/ut/attention/a2/ here and were applied via rename detection).

  1. vllm_ascend/attention/mla_v1.py — 3 hunks:
    • Imports: kept this branch's imports; added only
      is_pd_decode_recompute_scheduler_enabled (already present in
      utils.py from an earlier backport).
    • Builder __init__: main's hunk carried the whole PCP block
      (pcp_size/pcp_enabled/pcp_rank), which does not exist on this
      branch; added only self.dcp_enabled = enable_dcp().
    • Decode/prefill split gate: main changed
      not (pcp_enabled or dcp_size > 1) to add the DCP + PD-recompute
      override. This branch's gate is
      context_parallel_metadata is None; resolved as
      context_parallel_metadata is None or (self.dcp_enabled and is_pd_decode_recompute_scheduler_enabled(self.vllm_config))
      — same intent, expressed on this branch's gate. Verified
      AscendMlaDCPMetadataBuilder (mla_cp.py) inherits build() from
      AscendMLAMetadataBuilder, so the DCP MLA path is covered.
  2. attention_cp.py — import hunk only: kept this branch's
    memcache_comm_fence import path (main moved that symbol elsewhere) and
    added the helper import. All functional hunks applied cleanly.
  3. sfa_cp.py — import hunk only: main's side carried its whole
    utils/weight_switch/pcp import section, none of which exists here; added
    only from vllm_ascend.utils import is_pd_decode_recompute_scheduler_enabled.
    Functional hunks applied cleanly.
  4. tests/ut/attention/test_sfa_cp.py — main's version of
    test_sfa_dcp_builder_sizes_replicated_view_from_padded_block_table is
    parametrized against buffers/patches this branch does not have; applied
    only the PR's intent onto this branch's shape of the test (patch
    enable_dcp, assert it is called exactly once at init and cached).

Adaptations of cleanly-applied test hunks:

  • patch("vllm.config.get_current_vllm_config_or_none", ...) →
    patch("vllm.config.get_current_vllm_config", side_effect=AssertionError):
    this branch's helper fallback calls the latter, so patching the former
    would never simulate "no current config context".
  • patch.object(builder, "_update_parallel_slot_mapping") →
    _update_dsa_cp_slot_mapping_for_dcp (this branch's method name).
  • The MLA "without DCP" sub-test looped over pcp_size in (1, 2) — reduced
    to the single no-DCP case (no PCP in mla_v1 on this branch); it still
    asserts the DCP-only override is never evaluated when DCP is off.

Independent review (Codex)

Reviewed pre-submission with Codex against both branches and both commits.
Verdict: APPROVE WITH NITS; both findings addressed before submission:

  1. (minor) The adapted MLA regression test left
    context_parallel_metadata=None, so the is None gate short-circuited
    and the new override was never exercised. Fixed: the test now populates
    context_parallel_metadata, so classification hinges on the override
    itself, and a recompute-off case asserts short extends stay prefills.
  2. (nit) A documentation claim about the vLLM pin was wrong (vLLM v0.26.0
    exports both config getters; the patch-target change is needed because
    this branch's helper fallback calls get_current_vllm_config). Corrected.

Codex also confirmed: no functional hunks dropped, no other
split_decodes_and_prefills call sites on this branch need the same fix,
and no main-only code was pulled in.

Does this PR introduce any user-facing change?

No. Fixes failures in DCP-enabled PD-disaggregated recomputation.

How was this patch tested?

  • Regression tests from the original PR, adapted to this branch's code shape
    (see above), in tests/ut/attention/a2/test_attention_cp.py,
    tests/ut/attention/a2/test_mla_v1.py, tests/ut/attention/test_sfa_cp.py.
  • CI unit tests on this branch.

…vllm-project#16487)

Fix shape mismatches when DCP is enabled with PD-disaggregated
recomputation. Short recomputation requests, including last-token
recomputation, are now correctly classified as decode requests in the
attention, MLA, and SFA metadata builders.

Cache DCP state during initialization and use the builder's
configuration so metadata construction works outside the current-config
context.

No API or configuration changes. This fixes failures in DCP-enabled
PD-disaggregated recomputation.

- Added regression tests covering PD recomputation, mixed query lengths,
and behavior when the override does not apply.
- Added coverage for metadata construction without an active
current-config context.

- vLLM main:
vllm-project/vllm@a97dacb

---------

Signed-off-by: weiguihua2 <weiguihua2@huawei.com>
(cherry picked from commit cab3719)
@github-actions

Copy link
Copy Markdown
Contributor

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.


Tip

💡 Consider Linking a Related Issue or RFC

Your PR title contains the [BugFix] tag, indicating a bug fix or new feature.

Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:

  • Fixes #<issue_number>
  • Closes #<issue_number>
  • Resolves #<issue_number>
  • Refs #<rfc_or_issue_number> (for RFCs)

🙏 Thanks for helping us keep the project well-organized!

@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

Suggested PR Title:

[Attention][Feature] Cache DCP state in metadata builders for PD decode recompute scheduler

Suggested PR Summary:

### What this PR does / why we need it?
This PR caches the DCP (Distributed Context Parallel) enabled state during the initialization of metadata builders (`AscendAttentionDCPMetadataBuilder`, `AscendSFADCPMetadataBuilder`, and `AscendMLAMetadataBuilder`). This cached state, along with the builder's own `vllm_config`, is used to determine whether to treat short extends as decodes when splitting decodes and prefills. This avoids relying on `get_current_vllm_config()` during metadata building, which can run outside the active vLLM config context, and ensures correct behavior when the PD decode recompute scheduler is enabled.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Added unit tests in `test_attention_cp.py`, `test_mla_v1.py`, and `test_sfa_cp.py` to verify that the decodes/prefills split correctly uses the builder's config and cached DCP state without requiring an active vLLM config context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants