Skip to content

[Bugfix][KV Connector][Mooncake] Suppress completion for empty pulls - #59347

Merged
ZJY0516 merged 1 commit into
vllm-project:mainfrom
wangyicong52:fix/mooncake-empty-pull-completion
Oct 3, 2026
Merged

ZJY0516 merged 1 commit into
vllm-project:mainfrom
wangyicong52:fix/mooncake-empty-pull-completion

Conversation

@wangyicong52

@wangyicong52 wangyicong52 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Mooncake can issue a pull with no local destination blocks only to notify the Prefill side that its producer blocks may be released. The Decode request does not wait for remote KV in this case.

MooncakeConnectorWorker.process_pulling_result currently reports every successful pull as finished_recving after its worker tasks drain. For an empty pull, this sends a completion for a request that is not in WAITING_FOR_REMOTE_KVS and can trip the scheduler's request ownership/status assertion.

Empty pulls are reachable both during early-abort cleanup and when a remote prefix hit is reduced to no transferable local blocks.

Change

Report a successful receive completion only when the pull contains at least one local destination block:

if pull_meta.pull_tasks_count == 0 and any(pull_meta.local_block_ids):
    self.finished_recving_reqs.add(pull_meta.d_req_id)

The producer notification and pull-task accounting are unchanged. This also matches the existing failed-receive path, which already skips scheduler notification when the pull has no local blocks.

Relationship to other work

#50565 addresses the early-abort cleanup variant by adding an explicit notify_scheduler flag. Its current version intentionally still reports completion for an ordinary empty-block full-prefix pull. That does not cover the case where Decode does not enter WAITING_FOR_REMOTE_KVS after the connector returns no transferable blocks.

This PR applies the check at the worker completion sink, covering both cleanup-only and full-prefix empty pulls while preserving completion for every pull with real local destinations.

Test Plan

VLLM_TARGET_DEVICE=cpu python -m pytest -q --confcutdir=tests/v1 \
  tests/v1/kv_connector/unit/test_mooncake_connector.py::test_pull_completion_requires_local_blocks \
  tests/v1/kv_connector/unit/test_mooncake_connector.py::test_kv_consumuer

ruff check \
  vllm/distributed/kv_transfer/kv_connector/v1/mooncake/mooncake_connector.py \
  tests/v1/kv_connector/unit/test_mooncake_connector.py

ruff format --check \
  vllm/distributed/kv_transfer/kv_connector/v1/mooncake/mooncake_connector.py \
  tests/v1/kv_connector/unit/test_mooncake_connector.py

The regression test covers:

  • local_block_ids=[]: no scheduler completion;
  • local_block_ids=[[]]: no scheduler completion;
  • a non-empty pull: normal scheduler completion.

Test Result

  • Targeted pytest: 4 passed.
  • Ruff check and format: passed.
  • Python compilation and git diff --check: passed.

GPU Validation

Environment

  • Image: vllm/vllm-openai:nightly-36768d1bfd39094681cdbc8cb37d4b31c0729c89 (v0.30.1rc1.dev312+g36768d1bf).
  • Hardware: two nodes with 8 NVIDIA H20 GPUs each.
  • Runtime: Mooncake RDMA, 64-token blocks, prefix caching, and SWA bounded replay.

Prefill

vllm serve <model-path> \
  --host 0.0.0.0 \
  --port 8001 \
  --served-model-name deepseek-v4.1-flash \
  --trust-remote-code \
  --tokenizer-mode deepseek_v41 \
  --engram-config '{"cpu_offload":true}' \
  --max-model-len auto \
  --enable-cumem-allocator \
  --kv-transfer-config '{"kv_connector":"MooncakeConnector","kv_role":"kv_producer","kv_load_failure_policy":"fail","kv_buffer_device":"cuda","kv_connector_extra_config":{"mooncake_protocol":"rdma","device_name":"mlx5_1,mlx5_2,mlx5_3,mlx5_4","enforce_handshake_compat":false}}' \
  --block-size 64 \
  --swa-bounded-replay \
  --enable-prefix-caching \
  --tool-call-parser deepseek_v41 \
  --enable-auto-tool-choice \
  --reasoning-parser deepseek_v41 \
  --mm-encoder-tp-mode data \
  --api-server-count 1 \
  --tensor-parallel-size 8 \
  --max-num-batched-tokens 8192 \
  --max-num-seqs 256 \
  --gpu-memory-utilization 0.92 \
  --no-async-scheduling \
  --compilation-config '{"cudagraph_mode":"NONE"}' \
  --speculative-config '{"method":"dspark","num_speculative_tokens":5,"draft_sample_method":"probabilistic","rejection_sample_method":"block","enable_adaptive_verification":false}'

Decode

vllm serve <model-path> \
  --host 0.0.0.0 \
  --port 8002 \
  --served-model-name deepseek-v4.1-flash \
  --trust-remote-code \
  --tokenizer-mode deepseek_v41 \
  --engram-config '{"cpu_offload":true}' \
  --max-model-len auto \
  --enable-cumem-allocator \
  --kv-transfer-config '{"kv_connector":"MooncakeConnector","kv_role":"kv_consumer","kv_load_failure_policy":"fail","kv_buffer_device":"cuda","kv_connector_extra_config":{"mooncake_protocol":"rdma","device_name":"mlx5_1,mlx5_2,mlx5_3,mlx5_4","enforce_handshake_compat":false}}' \
  --block-size 64 \
  --swa-bounded-replay \
  --enable-prefix-caching \
  --tool-call-parser deepseek_v41 \
  --enable-auto-tool-choice \
  --reasoning-parser deepseek_v41 \
  --mm-encoder-tp-mode data \
  --api-server-count 1 \
  --tensor-parallel-size 1 \
  --data-parallel-size 8 \
  --pipeline-parallel-size 1 \
  --kv-cache-dtype fp8_ds_mla \
  --cp-kv-cache-interleave-size 256 \
  --enable-expert-parallel \
  --max-num-batched-tokens 2048 \
  --max-num-seqs 16 \
  --gpu-memory-utilization 0.92 \
  --no-disable-hybrid-kv-cache-manager \
  --speculative-config '{"method":"dspark","num_speculative_tokens":5,"draft_sample_method":"probabilistic","rejection_sample_method":"block","enable_adaptive_verification":true}' \
  --compilation-config '{"cudagraph_mode":"FULL_DECODE_ONLY"}' \
  --enable-per-request-metrics \
  --per-request-spec-decode-metrics summary

Results

  • Original image: the first 13-token request reproduced the issue. Decode hit assert RequestStatus.is_finished(req.status), returned HTTP 500 through the P/D proxy, and shut down its API server with EngineDeadError.
  • With this PR: the exact same request returned HTTP 200 and generated all 32 requested tokens. Prefill, Decode, and the proxy remained healthy with zero pod restarts and no matching assertion, traceback, engine error, transfer failure, CUDA error, or segmentation fault.
  • GSM8K: EvalScope 1.11.1 evaluated all 1,319 zero-shot samples with batch size 8, temperature 0.2, top_p 0.95, seed 42, max_tokens 8192, and reasoning_effort high. All 1,319 requests succeeded with zero errors, and accuracy was 95.83% (1,264/1,319).

AI Assistance

AI assistance was used for investigation, test implementation, and PR drafting. The submitter reviewed the changes and ran the checks listed above.

@mergify mergify Bot added bug Something isn't working kv-connector labels Sep 30, 2026
@wangyicong52
wangyicong52 marked this pull request as ready for review September 30, 2026 03:40

@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 the mooncake Mooncake KV-transfer / EC-transfer label Oct 1, 2026
@wangyicong52
wangyicong52 force-pushed the fix/mooncake-empty-pull-completion branch from 525682e to 971efc5 Compare October 2, 2026 02:12
@ZJY0516 ZJY0516 added the verified Run pre-commit for new contributors without triggering other tests label Oct 2, 2026
@zhewenl

zhewenl commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

LGTM, thanks for the fix

@ZJY0516 ZJY0516 added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 3, 2026
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

✅ @wangyicong52, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Your branch must contain every commit currently on its upstream target branch. Merge or rebase onto the latest target branch, then rerun the command. Append --allow-stale to a run command to test an outdated branch at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@wangyicong52

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

❌ This PR is 71 commits behind upstream main. Your branch must contain every commit currently on upstream main. No new CI build was started. Merge or rebase onto the latest main, then rerun /ci run. To test this branch at your own risk, use /ci run --allow-stale.

Signed-off-by: wangyicong <wangyicong@bytedance.com>
@wangyicong52
wangyicong52 force-pushed the fix/mooncake-empty-pull-completion branch from 971efc5 to 90a0f96 Compare October 3, 2026 05:31
@wangyicong52

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92742 for commit 90a0f967a0b5.

@vllm-agent

Copy link
Copy Markdown
Collaborator

CI selector (shadow): 3 test steps (6 jobs) instead of 9 (14 jobs)

Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works.

Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.

steps (jobs) Today's rules Selector Would skip Would add
NVIDIA, CPU and others 9 (14) 3 (6) 6 (8) 0 (0)
AMD mirrors 18 (21) 2 (5) 16 (16) 0 (0)
Selector would run (3)
  • distributed-mooncakeconnector-pd-accuracy-4-gpus
  • v1-kv-connectors ×4
  • v1-others-cpu
Would skip (today's rules run them) (6)
  • ascend-npu-test
  • cpu-language-generation-and-pooling-model-tests ×3
  • v1-core
  • v1-executor-worker
  • v1-kv-offload
  • v1-metrics-lmeval
Would add (today's rules do not run them) (0)

none

AMD mirrors: would skip (16)
  • extract-hidden-states-integration-2-gpus
  • fault-tolerance-e2e-2xh100
  • fusion-and-compile-unit-tests-2xb200
  • fusion-e2e-tp2-ar-rms-config-sweep-h100
  • fusion-e2e-tp2-asynctp-config-sweep-h100
  • fusion-e2e-tp2-b200
  • fusion-e2e-tp2-quick-h100
  • gemm-rs-ar-2xb200
  • kernels-fusedmoe-layer-test-2-b200s
  • kernels-fusedmoe-layer-test-2-h100s
  • kernels-minimax-reduce-rms-test-2-gpus
  • platform-tests
  • v1-core
  • v1-executor-worker
  • v1-kv-offload
  • v1-metrics-lmeval
AMD mirrors: would add (0)

none

2 changed files · base 1a001d5842 · head 90a0f967a0 · Python record: build 92706 at 6e517b15c1 · kernel record: table 6e517b1 (build 92706), map 6e517b1 · not counted: 10 build steps, 5 A100 steps the generator no longer emits, 7 optional steps the selector would also run

@ZJY0516
ZJY0516 merged commit 5f30fc7 into vllm-project:main Oct 3, 2026
52 checks passed
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 mooncake Mooncake KV-transfer / EC-transfer ready ONLY add when PR is ready to merge/full CI is needed verified Run pre-commit for new contributors without triggering other tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants