feat(kv-offload): import the server-allocated KV arena over CUDA IPC - #764
Open
xiaguan wants to merge 3 commits into
Open
feat(kv-offload): import the server-allocated KV arena over CUDA IPC#764xiaguan wants to merge 3 commits into
xiaguan wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b130f3abd9
ℹ️ 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".
Switch the external-PegaFlow client to the inverted-ownership contract (novitalabs/pegaflow feat/native-arena-ipc): registration sends the KV layout as per-layer offsets plus the total size, the SERVER allocates the fused arena on the target device, and the registration response carries a 64-byte CUDA IPC handle. OpenInfer imports the handle and builds its KvBuffer as a non-owning view over the mapping. Why: GPUDirect RDMA registration only works on memory the registering process owns, so the arena must live in PegaFlow. Compared with exporting client memory as a VMM fd, the IPC handle is plain bytes riding the existing gRPC response - the SCM_RIGHTS fd side-channel, its discovery via Health, and the fd-before-RPC ordering invariant all disappear, and with them ~200 lines of libc/cmsg client code. - external.rs: single round-trip registration returning the arena handle; no fd channel. The liveness stream now exits the process on loss: the server owns the memory this process has mapped, so a dead server means the KV arena is gone (no reconnect, by policy). - engine.rs: OffloadEngine::new(config, layout, num_blocks) registers first and returns the handle; Registration derives offsets from KvLayout instead of reconstructing them from device pointers. The in-process PegaEngine path is gone (external-only). - kernels: exportable.rs (VMM alloc/export) replaced by imported.rs (cuIpcOpenMemHandle/Close), ~100 lines smaller. - kv-cache: Backing::ImportedView is a non-owning view; the executor owns the ImportedKvArena and closes it after workers stop and before unregister lets the server free the allocation. - qwen3 executor: registers with PegaFlow before any KV allocation; Drop order is workers -> close mapping -> unregister. - glm52: keeps main's allocation and scheduling untouched; multi-arena offload fails fast client-side (one server-allocated arena per instance in native-arena-v1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: xiaguan <751080330@qq.com>
…ecks Post-review hardening of the arena-import client: - retire the dead-server watchdog before unregister: it aborts the process on stream loss and cannot tell a clean teardown from a crash; unregister now also treats 'instance not found' as success, since dropping the liveness stream may have already triggered the server's session cleanup - swap the (engine, arena) construction tuple so an executor-construction failure closes the IPC mapping before the engine's Drop unregisters and the server frees the allocation - executor Drop drains in-flight prefetch loads before closing the mapping - ImportedKvArena::open verifies the actual allocation size via cuMemGetAddressRange instead of trusting the client's own expectation - a server-side under-allocation now fails the import instead of turning into silent out-of-bounds writes - separate 120s data-plane deadline for save/load (hitting a deadline is fatal by policy, so it must sit far above worst-case DMA), config validation hoisted before server-side allocation, flush documented as a server-wide barrier Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: xiaguan <751080330@qq.com>
xiaguan
force-pushed
the
feat/pegaflow-arena-import
branch
from
July 26, 2026 08:40
b130f3a to
8120e38
Compare
Review feedback: with the native-arena contract rejecting multi-arena registration, a GLM5.2 launch with --kv-offload-server only failed inside with_arenas_on - after minutes of loading weights onto 8 GPUs. Validate at launch entry instead, next to the other kv_offload preconditions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: xiaguan <751080330@qq.com>
|
This pull request has been inactive for 14 days. It will be closed after another 30 days unless there is new activity. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Switches the external-PegaFlow KV offload client to the inverted-ownership contract (novitalabs/pegaflow#417): registration sends the KV layout (per-layer offsets + total size), the server allocates the fused GPU arena, and the registration response carries a 64-byte CUDA IPC handle that OpenInfer imports and uses as its KV buffer.
This supersedes #740 (client-allocated VMM + fd side-channel). The offload behavior — save/query/load/flush semantics, scheduler integration, write barriers — is unchanged.
Why
GPUDirect RDMA registration only works on memory the registering process owns, so the arena has to live in PegaFlow either way it is shared. With the server as allocator, the sharing primitive can be a legacy CUDA IPC handle — plain bytes in the existing gRPC response — instead of a VMM POSIX fd that needs a
SCM_RIGHTSUnix-socket side-channel. On this side that deletes:Health-advertised socket-path discovery, and the "fd must arrive before the register RPC" ordering invariant;VmmExportableBuffer(cuMemCreate/reserve/map/export, granularity rounding, GDR-flag probing) — replaced by a ~90-lineImportedKvArena(cuIpcOpenMemHandle/Close).Net: −83 lines vs main (vs #740's +176), with one fewer moving part per registration.
Changes
external.rs: single round-trip registration returning the arena handle. The liveness stream is now a dead-server detector that exits the process: the server owns the memory this process has mapped, so a lost server means the KV arena is gone. No reconnect, by design.engine.rs:OffloadEngine::new(config, layout, num_blocks)registers first and returns the handle;Registration::from_layoutderives per-layer offsets fromKvLayoutinstead of reconstructing them from device pointers.exportable.rs→imported.rs.Backing::ImportedViewis a non-owning view; the executor owns theImportedKvArenaand closes it after the workers stop and before unregister lets the server free the allocation (CUDA IPC does not defend an open importer mapping against the owner freeing).Droporder is workers → close mapping → unregister.main(this PR does not thread anexportable_kvflag through the rank workers); multi-arena offload fails fast client-side — one server-allocated arena per instance innative-arena-v1.Dependency
pegaflow-proto/pegaflow-core(types only, no storage/RDMA stack) pinned to the native-arena server branch; repin to the merged rev before this lands.Review hardening (second commit)
An adversarial review pass surfaced and fixed: the dead-server watchdog racing a clean shutdown (it is now retired before unregister, and unregister treats "instance not found" as success since dropping the liveness stream may already trigger the server's session cleanup); construction-failure drop order (arena now closes before the engine unregisters); executor
Dropdraining in-flight prefetch loads before closing the mapping;cuMemGetAddressRangeverification of the actual arena size at import (a server-side under-allocation fails the import instead of becoming silent out-of-bounds writes); a separate 120s data-plane deadline for save/load.Testing
On an RTX 5070 Ti against the torch-free (
--python-registry false) native-arena server:cpu_roundtrip: register → import → write per-(block,layer,segment) patterns → save → query (3/3 hits) → load into different block ids → byte-exact compare; untouched block stays zero; clean shutdown (server log confirms arena free on unregister).kv_offload_cpu_hit(Qwen3-4B live executor): pure CPU-hit and combined GPU+CPU-hit prefix restore, head-logprob drift within prefix-cache tolerance.cargo check/clippy --workspace --all-targetsgreen (including glm52 with the NCCL shim); kv-offload/server unit tests pass.Supersedes #740.