Skip to content

[Bugfix][NIXL] Preserve packed-row aliases during region dedup - #64661

Open
G-haoyu wants to merge 1 commit into
vllm-project:mainfrom
G-haoyu:fix/nixl-region-alias-compatibility
Open

G-haoyu wants to merge 1 commit into
vllm-project:mainfrom
G-haoyu:fix/nixl-region-alias-compatibility

Conversation

@G-haoyu

@G-haoyu G-haoyu commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Overview

Follow-up to #57662, addressing the packed DeepSeek-V4.1 alias regression that remained after region deduplication was keyed by (base_addr, block_len). The fix keeps aliases that are genuinely compatible together while preserving separate regions where a fold could truncate data.

Related issue: #57661.

Claims

  • Fixes test_register_packed_dsv4_mla_cache_as_single_region[v41-aliased-groups], without introducing failures in the exercised NIXL unit suites.
  • Preserves the original exact-length alias behavior used by hybrid MLA and CSA-linear layouts.
  • Allows a shorter non-MLA view to reuse a packed whole-row region only when its stride and block count match; wider views and incompatible scratch layouts get their own region.
  • Prevents the original failure mode in which a later, narrower view changed the descriptor length for a wider page and truncated the P-to-D transfer.

Validation

The same source-covered nightly container was used for both versions. The original mainline and this fix branch were compared by changing only base_worker.py.

Primary NIXL connector suites (test_nixl_connector.py, test_nixl_desc_geometry.py, test_nixl_connector_hma.py, test_nixl_push_connector.py, and test_tp_mapping.py):

Version Result
Original mainline 5 failed, 670 passed
Fix branch 4 failed, 671 passed

The fix branch resolves test_register_packed_dsv4_mla_cache_as_single_region[v41-aliased-groups]. There were no fix-only failures. The four remaining failures were identical between versions: two test_abort_timeout_on_prefiller cases because Ray is unavailable in the container, and two compatibility-hash validation cases unrelated to region registration.

Ten additional NIXL-related suites were run: bidirectional_kv_transfer, config, mooncake_connector, multi_connector, nixl_heartbeat, nixl_rocm_gpu_mem_diag, nixl_simple_cpu_offload, remote_prefill_lifecycle, simple_cpu_offload_connector, and v1/worker/test_gpu_kv_connector.

Version Result
Original mainline 35 failed, 224 passed, 2 skipped
Fix branch 35 failed, 224 passed, 2 skipped

The failure sets were identical. They are environment/dependency failures in Mooncake, configuration, and multi-connector tests and are not affected by this change. Across all 15 runnable NIXL-related files, the fix branch introduced no new failures.

The six nixl_integration suites were not run because the current environment has no GPU/model setup or running prefill/decode services. End-to-end P/D validation for DeepSeek-V4.1-Flash remains outstanding; community members with a P/D deployment are invited to verify transfer correctness and packed-layout behavior.

Details

Root cause

register_kv_caches builds the region table consumed by both sides of a NIXL transfer:

  1. The prefill worker registers each KV-cache view and records its base address, block length, stride, block count, and transfer group.
  2. The decode worker registers its corresponding views and uses the resulting region metadata to construct descriptors.
  3. Shared transfer groups may read a region owned by another group when the views alias the same physical storage.
  4. If a wider page is folded into a region whose block length was set by a narrower, later-registered view, the descriptor sends only the shorter prefix. DeepSeek-V4.1-Flash exposes this when SWA pages and MLA/indexer pages share a row but have different page lengths.

The (base_addr, block_len) key added by #57662 correctly prevents that truncation. Treating every alias as incompatible, however, breaks an established packed-MLA whole-row layout: shorter views in the same packed row are intentionally covered by the row region. A universal (base_addr, block_len, block_stride, num_blocks) key also splits hybrid MLA and CSA-linear views whose kernel-granularity metadata differs even though their transfer region is intentionally shared.

Design

The implementation records whether each region came from the packed whole-row path and uses a small compatibility helper at registration time:

  • Exact-length aliases continue to use the existing region. MLA aliases still update the canonical block length, stride, and block count as before.
  • A shorter non-MLA view may reuse an existing region only when that region is explicitly marked as a packed whole row and its stride and block count match the view.
  • A wider view never folds into an existing region, so its full page cannot be truncated.
  • Ordinary scratch/CSA pages are not treated as packed-row coverage. A wider ring or PLE page therefore keeps a separate region, while compatible exact-length aliases retain their established identity.
  • The packed-row marker is set at the point where the whole-row region is created, instead of inferring intent later from address or geometry alone.

This keeps the special case local to region registration and avoids a growing list of model-specific conditions. The alternative approaches were rejected because base-address-only deduplication reintroduces truncation, a universal geometry key regresses hybrid MLA layouts, and generic shorter-view coverage incorrectly absorbs scratch/CSA pages. Removing the packed whole-row path would avoid the ambiguity by losing the descriptor reduction it provides for packed MLA storage.

Scope and limitations

This is a metadata/descriptor-registration fix; it does not change the NIXL wire format or add synchronization to the inference path. It is intended to be compatible with the per-group region addressing from #62317. No user-visible API or documentation change is required.

This PR is a follow-up to #57662, not a duplicate of #61121. #61121 changes which DeepSeek-V4.1 layouts enter the packed-MLA whole-row path; this PR fixes the compatibility rules for aliases after region deduplication and preserves the behavior of existing hybrid/scratch layouts.

AI assistance

AI assistance was used for root-cause analysis, implementation, and test analysis. The submitter reviewed the changed lines and performed the validation reported above.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: The change is localized to NIXL region registration, reuses existing metadata, and documents the compatibility rules and trade-offs.

  • Testing and Validation: The fix was compared against the original mainline across 15 runnable NIXL-related unit files; no fix-only failures were observed. P/D integration validation is documented as unavailable in the current environment.

  • Code Quality and Style: The change preserves existing interfaces and formatting; no user-visible API or documentation update is needed.

  • Pull Request Contents: The description includes the predecessor PR, duplicate-work check, root cause, implementation details, validation evidence, limitations, and AI assistance disclosure.

@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.

@mergify mergify Bot added bug Something isn't working kv-connector nixl NIXL KV connector / multi-tiered KV offloading (KVCR) labels Oct 30, 2026
@G-haoyu
G-haoyu force-pushed the fix/nixl-region-alias-compatibility branch from c9ab330 to d3fa2cc Compare October 30, 2026 05:19
@mergify

mergify Bot commented Oct 31, 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, @G-haoyu.

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 31, 2026
Keep exact-length aliases intact while allowing shorter non-MLA views to reuse only compatible packed whole-row regions. Wider views and incompatible scratch layouts remain separate to avoid truncation and region-shape regressions.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: 葛昊宇 <haoyu_ge@intsig.net>
@G-haoyu
G-haoyu force-pushed the fix/nixl-region-alias-compatibility branch from d3fa2cc to 899b088 Compare October 31, 2026 11:00
@G-haoyu

G-haoyu commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

I also re-ran the targeted tests after rebasing:

  • All 10 targeted test_nixl_desc_geometry.py cases passed on both mainline and the fix branch.
  • v4-padded-pages passed on both versions.
  • v41-aliased-groups failed on mainline and passed with #64661.
  • No new failures were introduced in the targeted suites.
    We currently do not have P/D resources for full DeepSeek-V4.1 end-to-end validation, so community validation in a real P/D deployment would still be appreciated.

This branch has not been deployed

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

Labels

bug Something isn't working kv-connector nixl NIXL KV connector / multi-tiered KV offloading (KVCR)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant