Repository navigation
Conversation
|
/ci run |
|
✅ Triggered Buildkite CI #90079 for commit |
68610e5 to
24d8396
Compare
|
/ci run |
|
❌ This PR is 1 commit behind upstream |
24d8396 to
758382b
Compare
|
/ci run |
|
❌ This PR is 1 commit behind upstream |
|
Documentation preview: https://vllm--57810.org.readthedocs.build/en/57810/ |
|
/ci run |
758382b to
b3315a4
Compare
|
❌ This PR is 1 commit behind upstream |
|
/ci run |
b3315a4 to
37c675d
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
bcfbd48 to
9ce24e4
Compare
|
/ci run |
|
✅ Triggered Buildkite CI #90115 for commit |
9ce24e4 to
a94ff5c
Compare
|
/ci run |
|
❌ This PR is 3 commits behind upstream |
An external KV connector tier is the one cache that survives a pause/sleep: the GPU blocks are discarded, the offloaded copy is not. Today the pause/release cascade always evicts it, so a caller that only wants the GPU back (an RL rollout time-sharing the device with training) loses the very thing that makes resuming cheap and has to re-prefill. Add clear_connector_cache to pause_generation()/sleep() and thread it to EngineCore._reset_caches(). It defaults to True, so behavior is unchanged: block hashes do not cover the weights, and weight updates do not reset connectors, so callers that replace the weights must keep evicting the tier or they would serve stale KV. Callers that leave the weights alone can now opt out. Signed-off-by: Ao Shen <aoshen@inferact.ai> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai>
a94ff5c to
6f470a0
Compare
|
/ci run |
|
❌ This PR is 3 commits behind upstream |
|
This pull request has merge conflicts that must be resolved before it can be |
Summary
pause/sleepexist to hand GPU memory back. An external KV connector tier(CPU/SSD offload) is the one cache that survives that handover, and the
pause/sleep cascade evicts it. For an RL rollout that time-shares the GPU
(
sleep(1)→ train →wake_up()), that tier is what makes resumption cheap:without it the resume re-prefills every in-flight sample.
Whether the tier is still valid depends on the caller replacing the weights,
which the engine cannot know at pause time. Block hashes do not cover the
weights, so the default stays "evict" and this adds an opt-out:
Threaded through
pause_scheduler/sleepto_reset_caches(reset_connector=),the Python and Rust clients, and
/pauseand/sleep. DefaultTrueeverywhere, so nothing changes unless a caller opts out. The Rust client sends
the argument only when opting out, so a new frontend still drives an older
engine.
Measured
2x GB200, slurm job 30001, on the nightly image
vllm/vllm-openai:nightly(
0.29.1rc1.dev422), DeepSeek-V4.1-Flash, this branch's files bind-mountedover the image one by one (mounting the whole package would hide the compiled
_Cthat ships in it).Server, on the head node's 4 GPUs:
Client, from the second node: 16 concurrent completions of ~19.5k tokens each
(311,744 tokens queried in total), then
/sleep?level=1with and without&clear_connector_cache=false, then/wake_up, then the same 16 promptsagain. Full script in the details block below.
clear_connector_cache=falsekv_offload_load_bytes_totalkv_offload_store_bytes_totalThe counters are the evidence: with the tier cleared the replay stores the same
1.31 GB all over again, with it kept it reads exactly that 1.31 GB back. The
latency column is indicative only — the two arms run in sequence against one
server and share prompts, so the second arm starts warmer.
Two things about this run that a reviewer should know:
config above additionally had
"pin_in_flight_chunks": true, which is thatPR's key and not part of this one. It cannot have moved these numbers: the
run stores 1.22 GiB into a 32 GiB tier, so nothing is ever evicted and
pinning has nothing to do. Reproducing this PR alone means dropping that key,
as the command above does.
--prompt-tokens 4096produces ~19.5k tokens per request. The absolute latencies depend onthat; the hit/miss counters do not.
Client script
Not a duplicate
_reset_caches/ the connector eviction on pause is untouched by any open PR.#57296 is KV event payloads, #56754 is request draining, #49789 is
graph/runtime state across weight reloads.
Test plan
The rest of
test_engine_core.pystarts a real engine and does not run on mybox (another process holds the GPU: "Free memory on device cuda:0 (92.93/139.8
GiB) ... less than desired GPU memory utilization"). CI covers it.
test_pause_clear_connector_cache_opt_outcovers both values throughEngineCore.pause_scheduler;test_pause_forwards_connector_flagcovers theimmediate and deferred
EngineCoreProcpaths and that an omitted argumentclears;
sleep_route_sends_connector_opt_out_only_when_requestedpins the Rustwire format.
No model-quality impact: sampling, weights and attention are untouched.
Review rounds
Adversarial automated review, several rounds.
Round 1 rejected the original design (removing the eviction outright):
block hashes exclude the weight version and weight updates do not reset
connectors, so that eviction is what prevents stale KV — hence the opt-out. It
then found a P1 in the Rust client, which sent three positional arguments
unconditionally and broke
/pauseand/sleepagainst an older engine.A later round found a P1 that this revision fixes, and it is the reason the
connector file is in this diff. Keeping the tier crashed the very case the
feature exists for:
_reset_cachespreempts the running requests, andpreempted_req_idsis onlyconsumed by the next
schedule(). Nothing is scheduled while the enginesleeps, so that set survives the sleep and lands on the step that resumes the
request — and with the tier kept, that step already carries the request's
resume load. The preemption flush asserted that a preempted request can
only hold stores, which is true when preemption and resumption are separate
steps, and false here.
The flush now takes the store jobs and leaves the load alone: a load reads no
freed block, so it is not what the flush is there to settle. Default clearing
never reached this, because with the tier gone there is no load to find —
which is also why the completed-request measurement above could not have
caught it.
test_preempted_request_resumed_in_the_same_stepcovers bothscheduling modes and fails on the previous revision.
The opt-out is now refused where it is not safe
The flag reaches whatever connector is configured, and the same-step resume is
a state every connector that tracks per-request transfers has to expect. Only
OffloadingConnectordoes, after the fix above.MooncakeStoreConnectoralsoimplements
reset_cache, also consumespreempted_req_ids, and popsload_specs[req_id]and_unfinished_requests[req_id]for them(
mooncake/store/scheduler.py:226-231) — the entries its new-request loopthen asserts on four lines later.
Rather than change a second connector I cannot exercise here, the opt-out is
fail-closed: connectors declare
supports_retained_cache_on_pause(defaultFalse,TrueonOffloadingConnector, the conjunction of its children onMultiConnector), and a pause that asks to keep a cache the connector cannotkeep is refused before anything is paused:
Making Mooncake handle it is a separate, testable change; until then its users
get an error at the call instead of a crash at the wake-up.
Two related notes:
This PR does not change the flush for stores, which is what protects against
it. It does extend how long a bad entry would survive if a connector ever
produced one, which is an argument for the default being "evict".
force-evicts the tier.
finish_weight_updatedoing so would remove thefootgun at no false-positive cost.
The same round also asked for, and this revision adds, the
/pauseopt-outwire test, a
sleep(level>=1)threading test, the one-line note on why thePython client may always send the argument (client and engine ship together,
so they cannot disagree), and the parameter's documentation on
EngineCoreProc.pause_scheduler, which is the docstring MP clients read.A further round, told about all of the above, checked for any path where the
flag defaults to
Falseby accident, the reverse gRPC skew (old frontend, newengine), whether the typed bool query parameter changes the accepted request
format (
False/false/1/0), and whether the deferredengine_idle_callbackclosure can run with a flag a later pause changed. Nodefects.
AI assistance
Developed with AI assistance (Claude). I reviewed every changed line, ran the
tests above, and I am the accountable submitter.
🤖 Generated with Claude Code