Skip to content

[MRV2] Pass FULL replay context to synchronous KV loads - #60636

Open
whx-sjtu wants to merge 1 commit into
vllm-project:mainfrom
whx-sjtu:whx/mrv2-full-read-context-20261008
Open

whx-sjtu wants to merge 1 commit into
vllm-project:mainfrom
whx-sjtu:whx/mrv2-full-read-context-20261008

Conversation

@whx-sjtu

@whx-sjtu whx-sjtu commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Pass the actual FULL graph mode to MRV2's synchronous KV-load pre-forward hook, so a connector can wait for its loads before graph replay bypasses Python layer hooks. This is the standalone FULL-context fix extracted from #58968.

Rebased onto main bfa7e5b7afe40ebe48288801a2baf5c1135cd881. This remains an independent child of closed #58968.

Claims

  • Run kv_connector.pre_forward() under a forward context whose runtime mode is CUDAGraphMode.FULL on the FULL replay branch.
  • Preserve eager/piecewise paths and avoid an additional DP batch-coordination operation.
FULL context -> pre_forward / synchronous load wait -> graph replay

Validation

Validated on 61398a91a907fafb364cb0ffd7b2f902844abdb3 (2026-10-12): 17 passed. Changed-file pre-commit passed, including mypy for Python 3.11 and test/CI tethering checks.

VLLM_TARGET_DEVICE=cpu .venv/bin/python -m pytest -q -ra \
  tests/v1/worker/test_gpu_model_runner_v2.py tests/v1/worker/test_gpu_kv_connector.py

Fresh negative control on the current main: the two new FULL-context cases fail for DP sizes 1 and 2; both pass here. The tests check that pre_forward observes FULL mode without triggering another DP synchronization. Rebase preserves #60754's (scheduler_output, input_batch, attn_metadata) interface.

Exact-source FULL-graph PD/RDMA execution remains pending. Eager accuracy and a mocked replay boundary do not establish that result.

The eight independent PRs compose on this same main. The combined CPU suite passes 344 tests, with 33 skips (32 ROCm/aiter-gated connector cases and one GPU LLM case) and one model-loading deselection. Counts overlap with the independent suites.

Details

ActiveKVConnector starts synchronous loads during pre_forward(). Without a caller-provided context it constructs a default context. MoRI's READ path distinguishes FULL replay, which must finish all required reads before replay, from execution that can wait through per-layer hooks. The missing FULL context can therefore select the wrong existing wait path.

This changes only the FULL call site and its tests. It adds no connector capability, does not introduce a new READ barrier and does not depend on the MultiConnector ownership or speculative-readiness split. #57700 explicitly left this context correction to #58968; #48464 concerns MRV1 imperative lifecycle helpers, not this MRV2 replay call site.

Merge impact: reversible code change; MRV2 FULL pre-forward hooks. An existing connector's FULL-specific waiting behavior can now run at this call site. No throughput or latency equivalence is claimed.

#60808 changes MoRIIO to await all READs before any forward; if it lands first, MoRI no longer relies on FULL mode for that decision. This PR separately preserves the common MRV2 replay-context contract. #60637 handles output publication after draft writes, not input loading.

AI assistance was used for implementation, rebase, regression checks and this description. Human line-by-line review and the runtime acceptance items above remain pending. No performance improvement is claimed.


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: Source/compatibility review completed; human line-by-line review remains pending.
  • Testing and Validation: Rebased CPU regressions passed; the runtime acceptance gaps above remain open.
  • Code Quality and Style: Changed-file pre-commit, mypy and test/CI tethering checks passed.
  • Pull Request Contents: Current source, evidence, related PRs and remaining limitations documented.

@mergify mergify Bot added the mrv2 Model Runner V2 specific label Oct 8, 2026
@mergify

mergify Bot commented Oct 10, 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, @whx-sjtu.

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 10, 2026
Extract the focused fix and regression coverage from vllm-project#58968.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: whx-sjtu <xiaowang990929@gmail.com>
@whx-sjtu
whx-sjtu force-pushed the whx/mrv2-full-read-context-20261008 branch from a78375b to 61398a9 Compare October 12, 2026 03:41
@mergify mergify Bot removed the needs-rebase label Oct 12, 2026
@whx-sjtu
whx-sjtu marked this pull request as ready for review October 12, 2026 14:42

@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 commented Oct 12, 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, @whx-sjtu.

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 12, 2026

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

mrv2 Model Runner V2 specific needs-rebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant