Skip to content

[KV Connector][Mooncake] Start async lookup when a request arrives via on_new_request - #61292

Merged
njhill merged 8 commits into
vllm-project:mainfrom
Dao007forever:dao/preschedule-lookup
Nov 5, 2026
Merged

njhill merged 8 commits into
vllm-project:mainfrom
Dao007forever:dao/preschedule-lookup

Conversation

@Dao007forever

@Dao007forever Dao007forever commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Start MooncakeStore's async prefix lookup from on_new_request(), when the scheduler adds a request, so the lookup can finish while the request waits in the scheduler's waiting queue. GPU KV allocation and transfer stay in the scheduler's existing path.

This uses the existing connector hook KVConnectorBase_V1.on_new_request(request). An earlier revision added a new hook that ran on the input thread. That needed a handle handoff to stay lock-free, and it bought little: the engine core thread and the input thread share the GIL, so the only thing the lookup needs to overlap is GPU execution. on_new_request runs on the engine core thread, so all lookup state stays on one thread and needs no locking.

on_new_request runs when the core thread handles the ADD:

Design

  • MooncakeStoreConnector.on_new_request calls MooncakeStoreConnectorScheduler.start_lookup_prefetch. That calls LookupKeyClient.prefetch(), which submits the lookup to the existing single lookup executor and registers it in _lookups. The ZMQ socket is still used only by the executor thread.
  • The scheduler's first get_num_new_matched_tokens probe goes through the existing lookup(), which reuses the registered lookup instead of submitting a second RPC.
  • A completed async result is cached per request and reused by later probes until it is discarded or expires. Before this PR, the first read popped it.
  • The lookup is discarded in update_state_after_alloc (consumed; a reschedule after a failed load looks up again), in request_finished / request_finished_all_groups, on preemption in build_connector_meta, and on reset() and close().
  • Requests with skip_reading_external_kv, prompts shorter than the lookup alignment, or no block hashes are not prefetched.

Claims

  • With lookup_async=True, enable_lookup=True, and a positive lookup_prefetch_limit, eligible requests submit their lookup when they are added. Off by default.
  • lookup_prefetch_limit caps arrival-time lookups in flight. A slot is released when the RPC completes or is cancelled. Over the cap, the request is looked up when the scheduler first reaches it. Scheduler lookups are not capped.
  • Arrival-time lookups create no LoadSpec or GPU allocation. The scheduler still computes the external tokens to load from its current local-prefix hit count.
  • A completed async result older than lookup_prefetch_ttl_s (default 2 s, measured from when its RPC returned) is looked up again. So is a lookup whose token count or block-hash chain changed. A lookup still in flight never expires, so it is not resubmitted behind the backlog it is waiting on.
  • MultiConnector already forwards on_new_request, so this also works when MooncakeStore is wrapped in one.

Validation

.venv/bin/python -m pytest --confcutdir=tests/v1/kv_connector/unit \
  tests/v1/kv_connector/unit/test_mooncake_store_scheduler.py \
  tests/v1/kv_connector/unit/test_mooncake_store_connector.py -q

141 passed (Linux aarch64, Python 3.12, at 86cc07f). New and updated cases cover:

  • a lookup started by on_new_request and reused by the scheduler's probes with a single RPC, then dropped once allocated
  • the in-flight cap, including slot release on completion and scheduler lookups bypassing the cap
  • refreshing on a changed token count or hash chain, or an expired result, and not restarting a lookup that is still in flight
  • skipping ineligible requests
  • a finished request's queued lookup being cancelled before it runs, and discarding on preemption, reset and close

Not run: GPU serving, real Mooncake transfers, model evaluation, TTFT/throughput benchmarks.

Trade-offs

  • An earlier lookup adds backend work for requests that later abort or hit entirely in the local cache, and the single lookup worker can queue that work ahead of later requests. The cap bounds this.
  • The TTL can cause a second lookup after a long queue wait.
  • Both defaults need measurement on real workloads.
  • Cancelling a queued future works; an RPC that is already running cannot be interrupted.

Duplicate-work checks: searched open vllm-project/vllm PRs for Mooncake lookup, async/prefetch lookup, and on_new_request lookup. #48230 skips lookups for locally reused prefixes, which is a different change. #42321 (enqueue-time prefetch for LMCacheMPConnector via on_new_request) is the same idea for another connector; it was closed for inactivity, not rejected.

AI assistance: Codex and Claude Code assisted with implementation, source analysis, and validation.


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: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI. CPU validation is complete; GPU serving/model evaluation and benchmarks remain pending.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

…n_preschedule_request

Add KVConnectorBase_V1.on_preschedule_request(request), called on the
engine-core input thread right after the request is built. It may run
concurrently with schedule(), so a connector may only start self-contained,
thread-safe work and return a handle. The engine stores the handle on
request.kv_preschedule_handle, which hands it to the scheduler thread through
the input queue; the connector adopts it in get_num_new_matched_tokens.
Connector state stays scheduler-thread-only, so no lock is needed.

MooncakeStore uses it to start its async prefix lookup when a request
arrives, so the lookup overlaps the input-queue and waiting-queue wait.
LookupKeyClient.prefetch() only submits to the lookup executor and takes a
slot from a semaphore (lookup_prefetch_limit caps in-flight prefetches; off
by default). adopt() registers the result, and LookupKeyClient now keeps
async results until expiry (lookup_prefetch_ttl_s), a changed token/hash
range, or discard on finish, preemption, reset or close. A handle not yet
adopted is cancelled when its request finishes.

Co-authored-by: Codex <codex@openai.com>
Signed-off-by: Dao Le <Dao007forever@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qGetiYhFjQ6b3gmBJeTDi
@mergify

mergify Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--61292.org.readthedocs.build/en/61292/

@mergify

mergify Bot commented Oct 27, 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, @Dao007forever.

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 27, 2026
Sy0307 pushed a commit to Sy0307/vllm that referenced this pull request Oct 28, 2026
…n_preschedule_request

Add KVConnectorBase_V1.on_preschedule_request(request), called on the
engine-core input thread right after the request is built. It may run
concurrently with schedule(), so a connector may only start self-contained,
thread-safe work and return a handle. The engine stores the handle on
request.kv_preschedule_handle, which hands it to the scheduler thread through
the input queue; the connector adopts it in get_num_new_matched_tokens.
Connector state stays scheduler-thread-only, so no lock is needed.

MooncakeStore uses it to start its async prefix lookup when a request
arrives, so the lookup overlaps the input-queue and waiting-queue wait.
LookupKeyClient.prefetch() only submits to the lookup executor and takes a
slot from a semaphore (lookup_prefetch_limit caps in-flight prefetches; off
by default). adopt() registers the result, and LookupKeyClient now keeps
async results until expiry (lookup_prefetch_ttl_s), a changed token/hash
range, or discard on finish, preemption, reset or close. A handle not yet
adopted is cancelled when its request finishes.

Co-authored-by: Codex <codex@openai.com>
Signed-off-by: Dao Le <Dao007forever@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qGetiYhFjQ6b3gmBJeTDi

Rebased prerequisite from vllm-project#61292 at 83bc03e.

Signed-off-by: Sy03 <1370724210@qq.com>
Sy0307 added a commit to Sy0307/vllm that referenced this pull request Oct 28, 2026
Keep input-thread work request-owned and isolate child handles during scheduler callbacks. Preserve child order and cancellation through nested connectors. Depends on the on_preschedule_request interface from vllm-project#61292.

Co-authored-by: OpenAI Codex <noreply@openai.com>
Signed-off-by: Sy03 <1370724210@qq.com>
@Dao007forever
Dao007forever marked this pull request as ready for review October 28, 2026 22:49

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

Resolve the vllm/v1/request.py conflict with vllm-project#60960, which replaced
Request.skip_reading_prefix_cache with prefix_cache_hit_limit, and move
the admission lookup to request.skip_reading_external_kv like the other
connectors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrGRraHy6yNvfSeyS3usoB
@mergify mergify Bot removed the needs-rebase label Oct 28, 2026
@Sy0307

Sy0307 commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Thanks for the prefetch work — it matches the head-of-line blocking we hit in production: the store lookup client is single-threaded with no RPC timeout, so one slow batch_is_exist (large key lists from long prompts × TP/PP/DCP rank namespaces) can defer every waiting prefill.

Upstream build_connector_meta already discards lookups for finished_req_ids/preempted_req_ids. Our #63932 only moves that discard into the request_finished callback (2 lines). The difference matters when a request is aborted or times out while its lookup is still queued: future.cancel() only works before the RPC starts, so discarding at finish drops the dead lookup before the single executor picks it up, instead of waiting for the next metadata batch — which never comes if the engine goes idle, leaving the dead lookup (and its future) occupying the only lookup slot until the next request arrives. That dead request's latency then lands on the next real request.

Would you be open to folding that two-line discard into this PR (with attribution), or should we close #63932 as low-value and keep the timeout work separate?

The above information was written by me and polished with AI assistance.

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

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

Copy link
Copy Markdown
Contributor Author

Hey @Sy0307, I'll fold it in and let's do the RPC timeout as a separate PR.

Dao007forever and others added 2 commits November 2, 2026 22:23
Resolve the vllm/v1/engine/core.py conflict with main's frontend block
hashes: build the Request with frontend_hashes, then start the
connector's preschedule work on it.

Signed-off-by: Dao Le <Dao007forever@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TwEq3d2Nyvagw8ujXUmfci
Fold vllm-project#63932 by @Sy0307 into the finish hook, as they
suggested in review. request_finished now releases both lookup states:
an unadopted arrival-time prefetch on the request and any adopted or
scheduler-submitted lookup in LookupKeyClient. Previously the latter was
only discarded by the next build_connector_meta, so if the single lookup
executor freed up in between, the dead request's queued RPC started,
could no longer be cancelled, and delayed the next real request's lookup.

Co-authored-by: Sy0307 <24841857+Sy0307@users.noreply.github.com>
Signed-off-by: Dao Le <Dao007forever@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TwEq3d2Nyvagw8ujXUmfci
@mergify mergify Bot removed the needs-rebase label Nov 2, 2026
… paths

Address review feedback:
- Measure lookup_prefetch_ttl_s from when the lookup RPC returned (stamped
  in a done callback) and never expire an in-flight lookup. Expiring a
  queued prefetch by submission age resubmitted it behind the same backlog,
  doubling RPC load under the load prefetch is meant for.
- Remove the finished-request discard in build_connector_meta; every
  finish already goes through request_finished -> release_lookup.
- adopt() asserts the request has no registered lookup instead of handling
  an unreachable duplicate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U5Xg465seG36GHKr3k29vq
Signed-off-by: Dao Le <Dao007forever@gmail.com>
@Dao007forever
Dao007forever requested a review from njhill November 3, 2026 04:40

@Alex-ai-future Alex-ai-future left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the careful input-thread → scheduler handoff. The bounded prefetch and scheduler-owned adoption make the concurrency boundary easy to follow.

One small suggestion for the planned serving/TTFT validation: report the fraction of eligible requests whose prefetch has completed by their first scheduler probe, alongside the request wait time. That would directly show whether the change hides queue wait as intended, and how often the cap falls back to a scheduler-time lookup.

@njhill

njhill commented Nov 3, 2026

Copy link
Copy Markdown
Member

@Dao007forever I have opened #65466, please take a look. I think if we merge that first, this PR can be simplified quite a bit. In particular we should not longer need the new connector hook and request field, and can just initiate the lookup in the existing on_new_request().

@mergify

mergify Bot commented Nov 4, 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, @Dao007forever.

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

@mergify

mergify Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--61292.org.readthedocs.build/en/61292/

@mergify

mergify Bot commented Nov 4, 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, @Dao007forever.

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

Drop the on_preschedule_request hook, Request.kv_preschedule_handle, and
the input-thread prefetch/adopt handoff. The store connector now starts
its async lookup from the existing on_new_request hook, which runs on
the scheduler thread, so LookupKeyClient.prefetch() registers the lookup
directly and the client is used from a single thread.

With requests handled while waiting for model output (vllm-project/vllm
vllm-project#65466), on_new_request runs up to a step before the request is
scheduled, giving the lookup the same head start without a new connector
API or core changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TwEq3d2Nyvagw8ujXUmfci
Signed-off-by: Dao Le <Dao007forever@gmail.com>
@mergify mergify Bot removed the needs-rebase label Nov 4, 2026
- Discard the cached async lookup in update_state_after_alloc. A request
  rescheduled after a failed KV load otherwise reused the stale hit until
  the TTL expired and retried loading the same missing keys.
- Restore the executor-shutdown guard in prefetch() so a request added
  during teardown neither raises out of add_request nor leaks a slot.
- Describe arrival-time lookup without relying on the engine handling
  inputs during a step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TwEq3d2Nyvagw8ujXUmfci
Signed-off-by: Dao Le <Dao007forever@gmail.com>

@njhill njhill left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dao007forever Dao007forever changed the title [KV Connector][Mooncake] Start async lookup on the input thread via on_preschedule_request [KV Connector][Mooncake] Start async lookup on the input thread via on_new_request Nov 5, 2026
@Dao007forever Dao007forever changed the title [KV Connector][Mooncake] Start async lookup on the input thread via on_new_request [KV Connector][Mooncake] Start async lookup when a request arrives via on_new_request Nov 5, 2026
@Dao007forever

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #100095 for commit 2b1b48168a0a.

@njhill
njhill merged commit 80ff32c into vllm-project:main Nov 5, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation kv-connector mooncake Mooncake KV-transfer / EC-transfer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants