Skip to content

[Bugfix][KV Connector][NIXL] Support matching DCP layouts with PCP - #49342

Closed
LucasWilkinson wants to merge 3 commits into
vllm-project:mainfrom
LucasWilkinson:codex/nixl-prefill-pcp
Closed

[Bugfix][KV Connector][NIXL] Support matching DCP layouts with PCP#49342
LucasWilkinson wants to merge 3 commits into
vllm-project:mainfrom
LucasWilkinson:codex/nixl-prefill-pcp

Conversation

@LucasWilkinson

@LucasWilkinson LucasWilkinson commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Note: this PR intentionally supports matching producer and consumer DCP sizes with PCP disabled on the consumer; unequal-DCP remapping and decoder-side PCP remain out of scope.

Purpose

Add DCP-aware NIXL P/D transfer support that composes with prefill PCP without changing attention or token-interleaving behavior.

This change:

  • uses explicit (pp_rank, tp_rank, pcp_rank) handshake keys and (pp_rank, tp_rank, dcp_rank) remote-agent keys;
  • routes each consumer to producer workers with the same DCP rank while preserving the existing TP transfer plan;
  • rejects mismatched producer and consumer DCP sizes;
  • rejects PCP on KV consumers, allowing consumer fan-in to use the direct tp_rank % dcp_size mapping;
  • publishes one PCP producer replica per distinct DCP shard and counts completion only from those published replicas;
  • preserves one-copy MLA pulls while notifying the other producer workers involved in the request.

The connector remains layout-opaque: it selects matching workers and copies KV blocks. PCP/DCP attention chunking, empty attention shards, decoder-side PCP, and unequal-DCP token-position remapping are outside this PR.

Related work and duplicate check

Open-PR searches for NIXL PCP/DCP and context-parallel NIXL changes found #38433, #45340, and this draft; no other open PR combines the reduced equal-DCP transfer path with PCP replica ownership.

Test plan and results

.venv/bin/python -m pytest tests/v1/kv_connector/unit/test_nixl_connector.py tests/v1/kv_connector/unit/test_nixl_push_connector.py -k "not abort_timeout_on_prefiller and not register_kv_caches and not reqs_to_send_deadline_rebased_to_worker_clock" -q
# 102 passed, 7 deselected
.venv/bin/python -m pytest tests/v1/kv_connector/unit/test_handshake_pp_aggregation.py tests/v1/kv_connector/unit/test_nixl_connector.py::TestNixlHandshake::test_consumer_rejects_pcp tests/v1/kv_connector/unit/test_nixl_connector.py::TestNixlHandshake::test_pcp_producer_publishes_one_replica_per_dcp_rank tests/v1/kv_connector/unit/test_nixl_connector.py::TestNixlHandshake::test_pcp_producer_waits_only_for_published_replicas tests/v1/kv_connector/unit/test_nixl_connector.py::TestNixlHandshake::test_prefill_tp_size_greater_than_decode_tp_size tests/v1/kv_connector/unit/test_nixl_connector.py::TestNixlHandshake::test_prefill_tp_size_greater_than_decode_tp_size_mla tests/v1/kv_connector/unit/test_nixl_push_connector.py::TestPushPipelineParallel tests/v1/kv_connector/unit/test_nixl_push_connector.py::TestPushWriterMlaReplication tests/v1/kv_connector/unit/test_transfer_topology_sharded.py -q
# 17 passed
.venv/bin/pre-commit run --files $(git diff --name-only upstream/main)
# passed

A final-branch four-GPU TP1+PCP2+DCP2+EP2 prefiller to TP2+DCP2 decoder run was attempted. NIXL initialized and model loading completed, but current main deadlocked before the first transfer during PCP×DCP attention warmup: the non-empty PCP worker entered the DCP collective while its empty-context peer skipped it. Because this is a core PCP×DCP attention issue rather than connector behavior, this PR does not add a failing integration job for that topology.

Earlier development on a larger superset branch completed both 1,319-example DeepSeek-V2-Lite-Chat GSM8K evaluations for TP1+PCP2+DCP2 to TP2+DCP2 and TP2+PCP2+DCP4 to TP4+DCP4 within the required accuracy band. Those runs also contained separate attention and MRV2 fixes that are not in this connector PR, so they are not final-branch validation.

AI assistance disclosure

OpenAI Codex assisted with investigation, implementation, testing, and drafting this PR.

Reviewed by the human submitter

@LucasWilkinson LucasWilkinson added the bug Something isn't working label Jul 21, 2026
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch from 768c1e2 to 7db1f00 Compare July 21, 2026 18:39
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch 2 times, most recently from 5054d49 to b80fdc1 Compare July 22, 2026 15:08
@mergify mergify Bot added the ci/build label Jul 22, 2026
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch from b80fdc1 to a61032d Compare July 23, 2026 15:12
@LucasWilkinson LucasWilkinson changed the title [KV Connector][NIXL] Support MRV2 prefill PCP replicas [Bugfix][KV Connector][NIXL] Compose MRV2 PCP and DCP for P/D Jul 23, 2026
@mergify mergify Bot added the nvidia label Jul 23, 2026
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch from a61032d to e05b735 Compare July 23, 2026 21:24
@LucasWilkinson LucasWilkinson changed the title [Bugfix][KV Connector][NIXL] Compose MRV2 PCP and DCP for P/D [Bugfix][KV Connector][NIXL] Support matching DCP layouts with PCP Jul 23, 2026
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch 2 times, most recently from 9b83ef9 to bf1738d Compare July 24, 2026 20:26
@mergify

mergify Bot commented Jul 26, 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, @LucasWilkinson.

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 Jul 26, 2026
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch 3 times, most recently from f966cfe to a13d854 Compare July 28, 2026 03:11
@mergify mergify Bot removed the needs-rebase label Jul 28, 2026
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch from a13d854 to 5a5f6aa Compare July 28, 2026 03:59
@mergify

mergify Bot commented Jul 28, 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, @LucasWilkinson.

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 Jul 28, 2026
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch 2 times, most recently from beecfee to 3cb112a Compare July 30, 2026 15:05
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch 2 times, most recently from f692ed6 to 4dbafc9 Compare July 30, 2026 15:34
@mergify mergify Bot removed the needs-rebase label Jul 30, 2026
Route transfers between matching DCP ranks while preserving the existing TP transfer plan. Publish one PCP replica per DCP rank and account for completion only from published replicas. Reject mismatched DCP sizes.

Co-authored-by: QiuChunshuo <qiuchunshuo@huawei.com>

Co-authored-by: OpenAI Codex <codex@openai.com>

Signed-off-by: QiuChunshuo <qiuchunshuo@huawei.com>

Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch 2 times, most recently from 361d6a5 to 834d671 Compare July 31, 2026 04:37
The DCP->PCP rank inversion was written twice: once in
TransferTopology.get_target_remote_worker_keys, which used it to filter
out TP ranks with no matching worker and then discarded it, and again in
_nixl_handshake via _tp_dcp_to_pcp_rank to build the handshake key. Both
of the helper's ValueError branches were therefore unreachable, since it
only ever saw keys the filter had already accepted.

Return the PCP rank alongside the routing key as a RemoteWorkerTarget so
the relation is stated once, in the place that enforces it.

Signed-off-by: Lucas Wilkinson <wilkinson.lucas@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
@LucasWilkinson
LucasWilkinson force-pushed the codex/nixl-prefill-pcp branch from 834d671 to ccb8f1e Compare July 31, 2026 05:17
@LucasWilkinson
LucasWilkinson marked this pull request as ready for review July 31, 2026 05:21

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

Two regressions from the DCP/PCP support commit, both masked locally by a
UCX initialization failure that aborted these tests earlier:

- NixlBaseConnectorWorker.__init__ read get_pcp_group().rank_in_group
  unconditionally, unlike the dcp_rank line below it, so constructing a
  worker without an initialized PCP group raised AssertionError. Guard it
  on pcp_size like DCP already does.

- test_read_blocks_for_req_expands_remote_ids builds a worker with
  object.__new__ and assigns attributes by hand; _read_blocks_for_req now
  touches _done_recving_without_xfer, which the fixture never set.

Signed-off-by: Lucas Wilkinson <wilkinson.lucas@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
@mergify

mergify Bot commented Aug 3, 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, @LucasWilkinson.

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

@LucasWilkinson

Copy link
Copy Markdown
Collaborator Author

Superseded by #52779, which contains only the PCP producer-replica portion on current main. The DCP routing/configuration portion is handled separately by #50611.

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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant