[Feature]: Async scheduling support with KVConnector#27644
[Feature]: Async scheduling support with KVConnector#27644KevinCheung2259 wants to merge 2 commits intovllm-project:mainfrom
Conversation
- Add num_output_placeholders field to SchedulerOutput - Implement _get_num_output_placeholders() method for collecting placeholder info - AsyncScheduler overrides to populate actual placeholder counts - OffloadingConnector updated to handle placeholders correctly - Add comprehensive design documentation This enables external KV cache systems (e.g., LMCache) to correctly determine the actual computed token boundary in async scheduling mode by using: real_computed = num_computed_tokens - num_output_placeholders Changes are backward compatible with sync scheduling.
|
Documentation preview: https://vllm--27644.org.readthedocs.build/en/27644/ |
There was a problem hiding this comment.
Code Review
This pull request adds support for asynchronous scheduling with KVConnectors, a critical feature for improving concurrency and performance. The approach taken is sound, extending SchedulerOutput to carry placeholder information and using a new overridable method in the Scheduler class to populate it. My review includes a key suggestion to improve the type consistency of the new field in SchedulerOutput, which simplifies consumer logic and enhances maintainability. Overall, the changes are well-implemented and align with the goal of enabling async scheduling for KV cache connectors.
| # generated. KV connectors (e.g., LMCache) use this to determine the actual | ||
| # computed token boundary for caching. | ||
| # For sync scheduling: this dict is empty (all placeholders are 0). | ||
| num_output_placeholders: dict[str, int] | None = None |
There was a problem hiding this comment.
The type hint dict[str, int] | None and default value None are inconsistent with the comment on line 179 ("this dict is empty") and the implementation in scheduler.py, which always provides a dict. This ambiguity can lead to confusion and potential bugs. To improve clarity and type safety, this should be defined as a non-optional dictionary. This also allows for cleaner access patterns in consumers like OffloadingConnector.
| num_output_placeholders: dict[str, int] | None = None | |
| num_output_placeholders: dict[str, int] |
| num_placeholders = 0 | ||
| if scheduler_output.num_output_placeholders: | ||
| num_placeholders = scheduler_output.num_output_placeholders.get( | ||
| req_id, 0 | ||
| ) |
There was a problem hiding this comment.
Contingent on the suggested change in vllm/v1/core/sched/output.py to make num_output_placeholders a non-optional dict, this logic can be simplified. Since num_output_placeholders would always be a dictionary, you can directly use .get() which safely handles cases where a req_id is not present or the dictionary is empty. The if check becomes redundant because an empty dictionary is falsy, and .get() on an empty dictionary will correctly return the default value.
num_placeholders = scheduler_output.num_output_placeholders.get(
req_id, 0
)There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._update_after_schedule(scheduler_output) | ||
| return scheduler_output | ||
|
|
There was a problem hiding this comment.
Async placeholder counts captured before increment
The new num_output_placeholders snapshot is taken in Scheduler.schedule() before _update_after_schedule() runs, yet _update_after_schedule() immediately increments request.num_output_placeholders for every request that is about to emit a token. OffloadingConnectorScheduler._get_reqs_to_store() then uses the snapshot (scheduler_output.num_output_placeholders) together with the pre–schedule req.num_computed_tokens and adds new_tokens. In async decoding steps where a placeholder is added this cycle, the snapshot is missing that placeholder, so the connector subtracts only the old placeholders and still adds new_tokens, effectively planning to offload one token that has not been generated yet. This causes LMCache/offloading to store blocks that contain placeholder data instead of computed KV values, breaking cache correctness whenever async decoding is active.
Useful? React with 👍 / 👎.
Purpose
As demonstrated in our tests and discussed in issue #19970 , async scheduling is currently not supported with the KV connector. Both of these features are critical for LLM inference.
This PR support async scheduling mode when a KVConnector is configured. This unlocks better concurrency, reduces tail latency, and improves throughput for prefix-reuse and offloading scenarios.
Core Changes
Extended
SchedulerOutputdataclass (vllm/v1/core/sched/output.py)num_output_placeholders: dict[str, int] | Nonefield to track tokens scheduled but not yet generatedNone, populated only in async scheduling modeAdded
_get_num_output_placeholders()method (vllm/v1/core/sched/scheduler.py)Schedulerreturns empty dict (sync mode: no placeholders)SchedulerOutputto collect placeholder informationAsyncScheduleroverride (vllm/v1/core/sched/async_scheduler.py)_get_num_output_placeholders()to collect actual placeholder counts from each requestreal_computed_tokens = num_computed_tokens - num_output_placeholdersEssential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.