[KV Offload] Reshape the transfer data model: per group specs and offloaded side alignment offset - #44865
[KV Offload] Reshape the transfer data model: per group specs and offloaded side alignment offset#44865hickeyma wants to merge 4 commits into
Conversation
|
Thanks @hickeyma ! Why are we coupling the offloaded medium to use BTW I think there's a chance that it this PR may be simplified if we first tackle another road map item: |
|
Thanks @orozery for the feedback.
You are right.
Ok, I'll push a PR for Task 7 and we can iterate on that. |
|
This pull request has merge conflicts that must be resolved before it can be |
489be86 to
aaf05f6
Compare
49549f8 to
b97824e
Compare
118bcde to
0be6fd8
Compare
@orozery Fixed |
| @@ -199,8 +221,10 @@ def prepare_load( | |||
| req_context: per-request context (e.g. kv_transfer_params). | |||
|
|
|||
| Returns: | |||
| A LoadStoreSpec that can be used by a worker to locate and load | |||
| the actual offloaded KV data. | |||
| One LoadStoreSpec per KV cache group, positionally aligned with | |||
| kv_cache_groups. Groups with no matching keys get an empty spec. | |||
| The caller stamps the per-group alignment offset via | |||
| set_gpu_block_offset() on each returned spec. | |||
There was a problem hiding this comment.
This forces the OffloadingManager to be group-aware.
I suggest a different approach, which also aligns well with the P2P tier work by @liranschour and @ronensc:
Goal
Keep OffloadingManager completely group-unaware. The scheduler — which already iterates per group — handles the splitting.
API changes to OffloadingManager
| Method | Signature | Purpose |
|---|---|---|
prepare_store |
(keys, req_context) -> Collection[OffloadKey] | None |
Allocate space, return accepted keys (or None if full). No spec, no evicted_keys. |
prepare_load |
(keys, req_context) -> None |
Pin blocks for reading (protect from eviction). No spec returned. |
get_spec (new) |
(keys) -> LoadStoreSpec |
Return medium-specific addressing for the given keys. Called per group by the scheduler. |
There was a problem hiding this comment.
Thanks @orozery, this seems a better approach.
You're right that the per group return value pulls group awareness into the manager where it doesn't belong. prepare_write([key], ...) ends up returning a list of empty specs with a single non-empty one and the caller has to index back in with get_offload_group_idx(key) to find the one that matters. The manager is handling something that is really the scheduler's job as it already walks per group. Pulling get_spec out as its own thing reads much cleaner.
It also helps that this lines up with the P2P tier work, so I'd rather we converge on one manager API than have it drift. A couple of things I'd like to nail down with you (and @liranschour / @ronensc) before I refactor so we are all on the same path:
evicted_keys: dropping it fromprepare_storesounds right since the CPU manager already emits eviction events viatake_events(). I want to double check that nothing downstream still relies on the returned list before removing it?get_specgranularity: called once per group with that group's keys returning a singleLoadStoreSpec. Does that match what you and the P2P side need, or do you want it to take a batch?
If that all sounds good I'll update this PR rather than leaving the manager half migrated. Happy to break it out into a follow-up instead if you'd prefer to keep this one focused on the descriptor reshape. I supoose whatever works best for the P2P timeline. Let me know what you think.
the worker. The data model change does not alter how blocks are actually copied. It changes the shape of the descriptor that flows across the scheduler/worker boundary. Three improvements: 1. One spec per KV cache group. GPULoadStoreSpec no longer packs all groups into a single flat block_ids array with parallel group_sizes and block_indices arrays. The scheduler now emits one GroupTransfer (gpu_spec + offload_spec pair) per group, so the structure of the transfer matches the physical reality rather than requiring the worker to re-slice it on every store and load. 2. Alignment offset moves to the offloaded side. The per group first block GPU offset (needed to skip leading sub-blocks when offloaded blocks are larger than GPU blocks) used to live on GPULoadStoreSpec even though it only applies to the offloaded side. It now lives on BlockIDsLoadStoreSpec via set_gpu_block_offset(), stamped by the scheduler when building each GroupTransfer. 3. Explicit direction. TransferSpec changes from an anonymous (src, dst) tuple to a dataclass with groups and is_store, so the direction of a transfer is readable from the spec itself rather than inferred from the spec type and a separate gpu_to_cpu flag. The two runtime asserts that policed the packing condition (sum(group_sizes) == len(block_ids)) and the parallel-array condition (len(block_indices) == len(group_sizes)) are deleted because the new structure makes them structurally impossible to violate. Partial vllm-project#33689 Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
…ker API introduced in vllm-project#45053 submit_store and submit_load now accept groups: Sequence[GroupTransfer] directly, replacing the (src_spec, dst_spec) pair that no longer exists after vllm-project#45053 removed the old OffloadingHandler dispatch layer. The intermediate worker/worker.py file which contained the TransferSpec dataclass and the medium routing dispatcher is deleted. gpu_block_offset is moved to the base LoadStoreSpec class so that any offload medium can carry the per group alignment offset, not just block ID based backends. Public return types for prepare_load and PrepareStoreOutput.store_specs are widened to list[LoadStoreSpec] accordingly. The copy kernels and scheduler block tracking logic are unchanged. Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
4e9a571 to
caa1d91
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
Purpose
This PR reshapes how a single KV cache transfer is described between the scheduler (which decides what to move) and the worker (which performs the copy). It does NOT change how data is copied, stored, or sent. It instead changes the shape of the descriptor that flows between them.
What is changed:
block_idsarray which was confusing with a parallelgroup_sizesarray. We now emit one spec per KV cache group.block_indices) moves off the GPU spec and onto the offloaded (CPU) spec viaset_gpu_block_offset(...). Offloaded is the side that the offset actually describes.GroupTransfer+TransferSpec) replacing the anonymous(src, dst)tuple.The result is a descriptor whose structure matches the physical reality of the transfer, removing two kinds of conditions that must always hold and were previously enforced by runtime
asserts and recomputed in frequently executed code.Partial #33689
Tasks from #33689:
submit_store()/submit_load()methods onOffloadingWorker, replacing the need foris_storeflag ([KV Offload] Replace OffloadingHandler with OffloadingWorker #45053)How KV offloading transfers work
vLLM v1 can offload KV cache blocks from GPU to CPU RAM & storage. The control flow:
The transfer spec is the contract crossing that boundary. The scheduler builds it and the worker's handler consumes it to issue the actual block copies.
The issue that makes the spec non-trivial "block size mismatch"
Offloaded blocks can be larger than GPU blocks. Defined by
block_size_factor = offloaded_block_size / gpu_block_size(an integer ≥ 1). One offloaded CPU block therefore holdsblock_size_factorGPU sized sub-blocks.When a request's first offloadable GPU block does not land on an offloaded block boundary, the worker must skip the leading sub-blocks of the first offloaded block:
With hybrid / HMA models there are multiple KV cache groups (e.g. a full-attention group and a sliding-window group), and each group has its own first block offset. So the skip is inherently per group. It is because of this that this redesign is crucial.
Problem: the old data model packed unrelated things together
Before
A single
GPULoadStoreSpecflattened every group into oneblock_idsarray, then used two parallel arrays to recover the structure:group_sizes[i]— how many blocks belong to groupi(the packing).block_indices[i]— the logical GPU-block offset of groupi's first block (the alignment skip source).Why this is a problem
group_sizesto re-slice the flat array back into per group views on every transfer (src_offset/dst_offset). It then needs to assertsrc_offset == num_src_blocksto prove it walked it correctly. The structure is discarded at construction and rebuilt at consumption.sum(group_sizes) == len(block_ids)andlen(block_indices) == len(group_sizes)are structural truths that anassertcan only catch. They are not preventable.block_indiceslive on the wrong objectblock_size_factor == 1, so its skip is always 0). Yet the offset was stored on the GPU spec, forcing the worker to reach intogpu_spec.block_indicesand compute the offloaded side skip from it. The data described one side but lived on the other.(src, dst)carried no label. The worker inferred direction from a separategpu_to_cpuflag on itself and from which side happened to be aGPULoadStoreSpec. Reading the spec alone could not tell you store vs load.Design: a per group, direction tagged descriptor
After
Direction is now expressed by which method is called on
OffloadingWorkerintroduced in #45053:The same example from above is restructured (the parallel arrays are gone) and each group is a self contained pair. The offset rides on the offloaded spec it describes:
Mapping old to new
group_sizes[i]len(groups[i].gpu_spec.block_ids)block_indices[i]groups[i].offload_spec.gpu_block_offsetTransferSpec = (src, dst)tuplegroups: Sequence[GroupTransfer]passed tosubmit_store/submit_loadgpu_to_cpuinferred from spec types + flagsubmit_storevssubmit_load)block_idsblock_idson each specgpu_block_offsetonBlockIDsLoadStoreSpecgpu_block_offseton baseLoadStoreSpec. Any medium can carry itWhat is the value?
sum(group_sizes) == len(block_ids)) and the parallel array condition (len(block_indices) == len(group_sizes)) no longer exist and hence remove potential issues. The runtime asserts that policed them are deleted.gpu_block_offsetnow lives on the spec it describes (offloaded side). A reader no longer has to know "the skip is on the GPU spec but only applies to the CPU side."gpu_to_cpuflag) and makes logs/repr/debugging legible.gpu_block_offsetlives on the baseLoadStoreSpecso that file or object based backends can carry it without inheriting fromBlockIDsLoadStoreSpec.Test Plan
Test Result