Skip to content

[Rust Frontend] Run utility calls to completion when the caller is cancelled - #64265

Merged
BugenZhao merged 3 commits into
vllm-project:mainfrom
BugenZhao:bz/rust-uncancellable-utility
Nov 2, 2026
Merged

BugenZhao merged 3 commits into
vllm-project:mainfrom
BugenZhao:bz/rust-uncancellable-utility

Conversation

@BugenZhao

@BugenZhao BugenZhao commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Purpose

EngineCoreClient::call_utility fans a utility request out to every engine and waits for the replies. Dropping its future ran UtilityCallGuard, which unregistered the waiters, but since #46051 each request is sent by a task on the client's ZMQ runtime, so requests already handed off were still sent and the engines executed them. Cancellation only abandoned the client-side bookkeeping, and the replies arrived with no waiter.

So a caller cannot tie state to a call's completion: anything it holds is released when its future is dropped, which can be before the engines see the request. #63870 needs exactly that: readiness withdrawn for a pause must stay withdrawn until the engines have handled it, even if the HTTP or gRPC client that sent /pause disconnects.

This PR makes a utility call run to completion once its requests are prepared. The first commit moves the fan-out unchanged from EngineCoreClient to ClientInner, which owns the socket; the second changes it:

  • The caller registers the waiters and encodes the requests, borrowing the arguments.
  • One task on the client runtime sends the requests directly and collects every reply, so a utility call spawns once rather than once per engine through send_to_engine. The task unregisters the waiters of requests that failed to send.
  • The caller awaits that task and decodes the replies, so neither the arguments nor the result type need 'static.

Dropping the returned future now only stops waiting. UtilityCallGuard is removed. A call that fails while preparing still unregisters the waiters it allocated. If the client runtime shuts down during a call, the caller gets a ClientClosed error.

Not a duplicate: no open PR changes Rust utility call cancellation (searched rust utility call cancel and call_utility).

Test Plan

  • cancelled_utility_call_unregisters_waiter becomes cancelled_utility_call_still_collects_the_reply: after the caller is aborted, the waiter stays until the engine replies, and the reply then resolves it. It fails on main, where the abort unregisters the waiter.
  • cargo nextest run --workspace in rust/, plus cargo clippy --workspace --tests --examples -- -D warnings and cargo fmt --check.

Test Result

  • cargo nextest run --workspace: 2376 passed, 13 skipped.
  • clippy and fmt passed.

No model evaluation: generated tokens are unaffected.

AI assistance (Claude) was used for this change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CJckCCUEjbFWgsUABtnUGD

@mergify mergify Bot added the rust label Oct 28, 2026
BugenZhao and others added 2 commits October 28, 2026 11:26
Move `call_utility`'s fan-out unchanged from `EngineCoreClient` to
`ClientInner`, which now keeps the engine IDs and the client index;
`EngineCoreClient::call_utility` delegates. The next commit runs the
fan-out in a task that owns the inner client.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJckCCUEjbFWgsUABtnUGD
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
…ncelled

Dropping a `call_utility` future unregistered its waiters, but requests
already handed to the send tasks on the client runtime were still sent,
and the engines executed them. Cancellation only abandoned the
bookkeeping, so callers could not tie state to a call's completion.

Prepare and encode the requests in the caller, then send them and
collect every reply in one task on the client runtime; the caller awaits
that task and decodes the replies. The task sends directly instead of
through `send_to_engine`, which would spawn again per request. Dropping
the future now only stops waiting. This removes `UtilityCallGuard`; the
task unregisters the waiters of requests that failed to send.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJckCCUEjbFWgsUABtnUGD
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao
BugenZhao force-pushed the bz/rust-uncancellable-utility branch from ecab322 to 9c78e6b Compare October 28, 2026 11:29
BugenZhao added a commit to JulienDarve/vllm that referenced this pull request Oct 28, 2026
Local rebase of vllm-project#63870 onto vllm-project#64265: the
PR's net change as of 758a88f, without its two workarounds for
cancelled control calls. The utility call fan-out on `ClientInner` now
comes from vllm-project#64265.

Co-authored-by: Bugen Zhao <i@bugenzhao.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJckCCUEjbFWgsUABtnUGD
Signed-off-by: Julien Darve <jdarve@NVIDIA.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
BugenZhao added a commit to JulienDarve/vllm that referenced this pull request Oct 28, 2026
Pass a control call's `RecheckGuard` into `ClientInner::call_utility`,
whose task (vllm-project#64265) holds it until the engines reply.
A caller cancelled after sending a pause, for example by a client
disconnecting from `/pause`, then keeps readiness withdrawn until the
engines have handled it instead of rechecking at once, which could
overtake the pause and publish ready after it took effect. The guard
now owns an `Arc<Readiness>`, and `start_weight_update` passes its
withdrawal into the `collective_rpc` it runs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJckCCUEjbFWgsUABtnUGD
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao
BugenZhao marked this pull request as ready for review October 28, 2026 11:51
@BugenZhao
BugenZhao requested a review from njhill as a code owner October 28, 2026 11:51

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

@BugenZhao

Copy link
Copy Markdown
Member Author

/ci run

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #99146 for commit dee5a42a1f3d.

ℹ️ This PR is 312 commits behind upstream main. Merged upstream main into this branch as dee5a42a1f3d before starting CI. Pull the branch before pushing more changes.

@BugenZhao
BugenZhao merged commit 785b0e4 into vllm-project:main Nov 2, 2026
30 checks passed
BugenZhao added a commit to JulienDarve/vllm that referenced this pull request Nov 2, 2026
Resolve conflicts after vllm-project#64265 was squash-merged to main:

- gpu_worker.py: keep main's finish_weight_update (try/finally around
  the engine finish, _end_weight_update, KV cache loan release, direct
  reload handling) and clear the incomplete-weight mark only after
  _end_weight_update(refresh=True) returns, so a failed finish or a
  failed draft head restore still reports incomplete weights.
- Test helpers: initialize both _incomplete_weight_updates and main's
  KV cache loan fields.
- Rust engine-core-client: keep the readiness guard parameter on
  call_utility and is_inference_ready on top of the squashed vllm-project#64265;
  take main's test_utils import list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJckCCukQgyfESDciCFndy
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
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.

3 participants