Repository navigation
[KV offload] Add max_recompute_pending_tokens to skip waiting on short in-flight offloads - #57959
robertgshaw2-redhat wants to merge 2 commits into
Conversation
…t in-flight offloads A lookup that finds chunks whose GPU->CPU store is still in flight (HIT_PENDING) always deferred the request until the store completed, even when only a chunk or two were pending. Behind a backlog of serialized stores that wait can reach tens of seconds, which shows up as large vllm:kv_offload_lookup_async_delay_seconds observations. Add a kv_connector_extra_config option, max_recompute_pending_tokens, that bounds the wait: when the run of chunks from the first pending one to the end of the hit would gain at most this many tokens, the prefix lookup is cut before the first pending chunk so the request loads the ready prefix and recomputes the rest. The partial-tail lookup applies the same budget. RETRY results and pending chunks inside a sliding-window group's required window still defer. The default of 0 keeps the current always-wait behavior. Signed-off-by: robertgshaw2-redhat <robertgshaw2@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KGES2LbnNcLCHwQM9WNKzb
|
Documentation preview: https://vllm--57959.org.readthedocs.build/en/57959/ |
orozery
left a comment
There was a problem hiding this comment.
Thanks @robertgshaw2-redhat . LGTM in general.
Got 2 points from Claude (have not verified):
-
Split RETRY out of pending in the partial-tail loop (scheduler.py:1032). Right now boundary_pending is set by both HIT_PENDING and RETRY, so any non-zero budget makes RETRY stop deferring — contradicting your own doc row. Keep two flags; gate only the pending one.
-
Gate on the highest awaitable pending boundary, not max_boundary (scheduler.py:1050). max_boundary - complete_boundary is the best case at the top candidate, but the pending boundary can be far lower, so the feature under-triggers exactly where it's meant to fire. Also skip boundaries that had a MISS in another group — those can never become hits, so deferring for them buys nothing.
|
This pull request has merge conflicts that must be resolved before it can be |
Purpose
When a KV offload lookup hits chunks whose GPU->CPU store is still in flight (
HIT_PENDING), the scheduler always defers the request until the store completes, even if only a chunk or two are pending. Behind a backlog of serialized stores that wait can reach tens of seconds, which shows up as largevllm:kv_offload_lookup_async_delay_secondsobservations. Recomputing a handful of tokens would have been far cheaper.This PR adds a
kv_connector_extra_configoption,max_recompute_pending_tokens, that bounds the wait:RETRYresults still defer (the chunk's location is unknown), as do pending chunks inside a sliding-window group's required window.0, which keeps the current always-wait behavior, so this is a no-op unless opted in.The option is documented in
docs/features/kv_offloading_usage.md.Not a duplicate
Searched open PRs for
HIT_PENDING offload recomputeandmax_recompute_pending_tokens. The closest is #53087, which is complementary rather than overlapping: it adds a wall-clock deadline (hit_pending_timeout_s) insideTieringOffloadingManagerfor stalled primary-tier writes. This PR is a token-budget decision in the connector scheduler that applies to all offloading specs and avoids the wait up front when the pending run is small, rather than timing out after the fact.Test Plan
New tests:
test_recompute_budget_decides_whether_pending_chunks_are_awaited(end-to-end through the request runner): with chunks[HIT, HIT_PENDING, HIT], a budget covering the pending run loads the ready first chunk and records no async lookup delay; a smaller budget defers as before._maximal_prefix_lookupunit cases: short pending run is cut before the first pending chunk, a run one chunk over budget defers, a leading pending chunk within budget is a miss, andRETRYdefers regardless of budget.test_negative_recompute_budget_is_rejected.Test Result
No model eval or serving benchmark was run: the default (
0) leaves lookup behavior unchanged, and with the option enabled the request only recomputes tokens it would otherwise have loaded, so outputs are unaffected.AI assistance (Claude Code) was used to write this change.
🤖 Generated with Claude Code