Skip to content

[Bugfix][Mooncake] Order async loads after cache block copies - #50360

Open
GirasoleY wants to merge 1 commit into
vllm-project:mainfrom
GirasoleY:fix/mooncake-async-load-ordering
Open

GirasoleY wants to merge 1 commit into
vllm-project:mainfrom
GirasoleY:fix/mooncake-async-load-ordering

Conversation

@GirasoleY

Copy link
Copy Markdown
Contributor

Purpose

Prevent asynchronous Mooncake loads from racing scheduler-issued copy-on-write cache block copies.

The model runner enqueues a cache-page copy on the compute stream before Mooncake may write the destination page. Previously, a background receive thread could start the Mooncake GET before that copy completed, allowing the copy and external DMA to write the same page concurrently.

This change records one CUDA event after the copies are enqueued, attaches it to the load requests, and makes the background receive thread wait before issuing the external write. The wait is scoped to the background transfer thread and the recorded stream; it is not a device-wide synchronization. It mirrors the existing store path, which waits before Mooncake reads GPU KV memory.

Duplicate-work check

The following open-PR searches were run:

gh pr list --repo vllm-project/vllm --state open --search "Mooncake asynchronous load cache block copy"
gh pr list --repo vllm-project/vllm --state open --search "Mooncake RDMA copy-on-write race"

No open PR addresses this load/copy ordering race. Returned PRs concerned EC hidden-state transfer, MultiConnector state, or request-lifecycle leaks (#47302, #42841, and #36014).

AI assistance was used. The human submitter reviewed the change intent and is responsible for reviewing the final OSS diff before merge.

Test Plan

.venv/bin/python -m pytest -q tests/v1/kv_connector/unit/test_mooncake_store_worker.py
pre-commit run ruff-check --files tests/v1/kv_connector/unit/test_mooncake_store_worker.py vllm/distributed/kv_transfer/kv_connector/v1/mooncake/store/worker.py
pre-commit run ruff-format --files tests/v1/kv_connector/unit/test_mooncake_store_worker.py vllm/distributed/kv_transfer/kv_connector/v1/mooncake/store/worker.py

No model evaluation was run because this changes transfer ordering rather than model computation. A real GPU/Mooncake overlap benchmark remains follow-up validation.

Test Result

89 passed, 14 warnings
ruff-check: passed
ruff-format: passed

Essential Elements of an Effective PR Description Checklist
  • Purpose and race description
  • Test plan and results
  • Duplicate-work and AI-assistance disclosures

Record a CUDA event after scheduler-issued cache block copies are enqueued and attach it to asynchronous load requests. Make receive threads wait before writing destination pages so a background Mooncake transfer cannot race with copy-on-write preservation.

Signed-off-by: girasoley <girasoley@inferact.ai>

@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 v1 bug Something isn't working kv-connector labels Jul 30, 2026
@mergify

mergify Bot commented Aug 1, 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, @GirasoleY.

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 Aug 1, 2026
@ivanium

ivanium commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Per offline discussion, it seems this race condition cannot happen. More concretely, with #49502, a local partial-tail CoW and a remote connector load should be mutually exclusive.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants