[Bugfix][Store] Support Dummy external buffer registration - #1
Open
zxpdemonio wants to merge 996 commits into
Open
[Bugfix][Store] Support Dummy external buffer registration#1zxpdemonio wants to merge 996 commits into
zxpdemonio wants to merge 996 commits into
Conversation
…-ai#3235) Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.79.3 to 1.82.1. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.79.3...v1.82.1) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-version: 1.82.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ache-ai#3166) Co-authored-by: Yuchen Kou <kouyuchen@approaching.ai>
…cache-ai#3171) (kvcache-ai#3184) * [ROCm] Add ROCm/HIP wheel build, CI, and release parity with CUDA Closes the ROCm CI parity gap (kvcache-ai#3171). Mooncake already supports AMD GPUs in the transfer engine (USE_HIP, hip_transport, dmabuf MR registration from kvcache-ai#2225), but there was no CI to build/test the wheel and no published mooncake-transfer-engine-rocm package. The transfer engine is torch-independent and has no device kernels, so the ROCm wheel is GPU-arch-agnostic (one wheel covers gfx942 and gfx950) and the ROCm runtime (libamdhip64/libhsa-runtime64) is auditwheel-excluded and bound at load time -- so the same wheel works in both the upstream vllm/vllm-openai-rocm and lmsysorg/sglang ROCm images. - scripts/build_wheel.sh: add HIP_BUILD variant -> mooncake-transfer-engine-rocm (ROCm runtime libs were already in the auditwheel exclude list). - .github/workflows/ci_rocm.yml: PR build check in the ROCm dev image (py3.10 + py3.12), mirroring ci_cu13.yml. No GPU needed to compile. - .github/workflows/release-rocm.yaml: tag-triggered build+publish, mirroring release-musa.yaml (self-contained container build), py3.10-3.13. - ci.yml: wire build-wheel-rocm into the CI graph and CI Gate. Validated on real hardware (build in rocm/dev-ubuntu-22.04:7.2.3-complete, installed into the target images): - MI300X gfx942 (Hotaisle): SGLang v0.5.16-rocm720-mi30x (cp310) and vLLM v0.26.0 (cp312) -- mooncake_master --version + tests/test_transfer_on_hip.py (2/2 sync read+write loopback transfers) pass on-device. - MI355X gfx950 (Crusoe): same images (mi35x variants), same tests. Signed-off-by: andyluo7 <andy.luo@amd.com>
…ache-ai#3210) * [TE] perf(efa): allow bounding the batch MR registration fan-out registerLocalMemoryBatch() and unregisterLocalMemoryBatch() spawn one std::async(std::launch::async) per buffer with no cap, which libstdc++ takes literally -- one fresh thread each. Kimi-K3 registers ~180 KV buffers per TP rank in a single batch, and with one TransferEngine per rank an 8-rank node peaks at ~1100 threads all inside fi_mr_reg / fi_mr_regattr. Replace both fan-outs with a fixed pool pulling from a shared index, sized by MC_MAX_CONCURRENT_REG_MR. The default of 0 means unbounded, so behavior is unchanged unless an operator sets the knob: with no cap the pool spawns count-1 threads and runs the caller as a worker, matching what std::async did. Error semantics are also unchanged -- every item is still attempted and the first non-zero return is propagated. The knob is opt-in with no built-in default because its effect depends on the order the caller passes buffers in, which this layer cannot see. Measured on p5.48xlarge (32 NICs) replaying K3's registration the way SGLang issues it -- 8 processes, one engine and one GPU each, 182 buffers of 2.5 KB to 391 MB per process, barrier-synchronized. Slowest rank, since nothing serves until all 8 finish: per-proc cap descending pool order (SGLang's) ascending unset 108/99/128 s 112 s 120 s 64 184/208 s 101 s 60 s 16 95/98/97 s 43 s 36/37 s 8 107 s -- 44 s 4 157 s -- -- A cap of 16 is worth 2.6x on the order SGLang actually uses, but only 1.1x on descending order, and the order alone swings the capped result by 2.7x. Unbounded is order-insensitive (99-128 s) because nothing queues. Largest-first being the worst order contradicts longest-processing-time scheduling and is still unexplained, so no sort is applied here yet. Two other platforms put the optimum elsewhere, which is the other reason not to compile in a default: p5 host memory (fi_mr_reg), 128 x 2 GiB of 4 KB pages, prefers cap 16 (59.1 s vs 274 s unbounded); a 2x p6-b300 K3 server run at 16 NICs/rank prefers cap 8 (20.7 s vs 138.7 s unbounded). A core-scaled default would be worse still on a large node -- where capping helps, the bottleneck is the provider lock, not CPU. So the code keeps its historical behavior and only exposes the pool size. * [Doc] condense the MC_MAX_CONCURRENT_REG_MR entry The bullet had grown into a multi-paragraph section with measurement tables, out of proportion to every other entry in this list. Keep the operationally relevant facts -- the cap is per process, the default is unbounded, a good value is platform- and order-dependent, do not core-scale it -- and leave the supporting measurements to the PR. * [TE] docs(efa): correct the MC_MAX_CONCURRENT_REG_MR guidance The comment and docs claimed the cap should not be core-scaled, on the grounds that the bottleneck was a shared provider lock rather than CPU. Both halves were wrong, and both came from a single-process benchmark where 48 registration threads on a 192-core node never contended. Measuring at the real topology instead -- one TransferEngine per TP rank, and varying both the rank count and the core budget -- shows the optimum is set by the CPU budget: 8 ranks/192 cores prefers cap 16, 4 ranks/192 cores prefers 32, and 8 ranks/64 cores prefers 8. All three are the same ~cores global thread count, so a good value is roughly cores/processes-per-node. The lock hypothesis is refuted separately by a 23x speedup from capping a serial baseline, which a size-proportional global lock cannot produce. Also drops the retracted single-process figures and shortens the comment. No functional change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…g the same batch key, lead to OBJECT_ALREADY_EXISTS/persist failed/INVALID_KEY (kvcache-ai#2967)
…ions (kvcache-ai#3230) Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
…i#3236) Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
…3066) (kvcache-ai#3073) Two bugs in the BatchLoad io_uring path could cause short reads to pass validation silently: 1. storage_backend.cpp: read_res was overwritten with the expected value before the downstream short-read check, making it dead code. 2. uring_file.cpp: collect() used io_uring_for_each_cqe which consumes all CQEs without user_data filtering, allowing stale CQEs from previous failed operations to contaminate byte counts. Fix: add min_required validation before pointer adjustment, and filter CQEs by monotonic op_id via sqe->user_data with peek-then-wait to avoid extra syscalls on the common path. Closes kvcache-ai#3066. Co-authored-by: Claude <noreply@anthropic.com>
…e-ai#3242) isGpuDirectRdmaSupported only recognized nvidia_peermem, so ROCm hosts never enabled gpu_to_gpu and VRAM RDMA transfers were rejected at transport select. Signed-off-by: staryxchen <staryxchen@tencent.com>
…ai#3248) Hardcoding "cuda:" skipped hip:N topology entries, so MC_ENABLE_HCA_PEER_AFFINITY never applied on ROCm builds. Signed-off-by: staryxchen <staryxchen@tencent.com>
…e-ai#3246) Remote reads of SSD-offloaded objects failed with '[RDMA] No rkey for MR access' because FileStorage::RegisterLocalMemory registered the client buffer pool (which backs offload read results) with remote_accessible=false. With remote_accessible=false the pool's BufferDesc is published with an empty rkey array, so any peer that RDMA READs the pool hits the rkey-range check in selectPeerDevice. Fix: register with remote_accessible=true, matching how DRAM segments are registered in MountSegmentAndGetId. No performance impact (MR access flags do not affect transfer rate; registration is a one-time startup cost). Signed-off-by: tan changzhi <544463199@qq.com>
Co-authored-by: Yuchen Kou <kouyuchen@approaching.ai>
* [Bugfix][TE] Fix dma-buf offsets for chunked GPU MRs Fix dma-buf offset calculation for chunked GPU memory regions by incorporating each chunk's relative displacement, and add a GPU loopback regression test. Signed-off-by: Ruiqing Feng <ruiqing.feng@broadcom.com> * [CI] Suppress libnuma.so LSan false positive in async event drain test Suppress LSan false positive from libnuma.so's static topology cache in rdma_async_event_drain_test, fixing process teardown exit code 8 in CI. Signed-off-by: Ruiqing Feng <ruiqing.feng@broadcom.com> --------- Signed-off-by: Ruiqing Feng <ruiqing.feng@broadcom.com>
…kvcache-ai#3196) * add option for USE_VRAM_SEGMENT --------- Co-authored-by: 沈冲 <hz_yongke.zyk@antgroup.com>
…ai#3268) * Restrict nightly workflow to upstream repository * Simplify nightly repository guard
* [FEAT] add SUPA (Biren GPU) support to transfer engine
Introduce gpu_vendor/supa.h CUDA-alike shim mapping cuda* runtime
APIs to supa* equivalents. Wire USE_SUPA option through common.cmake
(SUPA_ROOT / driver lib discovery) and transfer-engine CMakeLists.
Adapt rdma_context, rdma_transport, transfer_engine_bench, and
cuda_alike.h to build against SUPA runtime/driver when USE_SUPA is ON.
* [TEST] enable GPU mem backend under USE_SUPA in rdma_transport_test
rdma_transport_test.cpp's GPU-memory paths (pickBackend/pickDevId/validateBackend/
setBackendDevice/allocateMemoryPool/freeMemoryPool/copyFromHost/copyToHost and the
checkCudaError helper) were guarded by USE_CUDA||USE_MUSA||USE_HIP||USE_MLU||
USE_MACA||USE_HYGON||USE_COREX but omitted USE_SUPA, so under the SUPA build the test
fell back to mem_backend=cpu and validateBackend("gpu") exited with EXIT_FAILURE --
the GPU-mem RDMA case could not run.
Add USE_SUPA to each of those guards. cuda_alike.h already #includes gpu_vendor/supa.h
under USE_SUPA, which aliases cudaMalloc/cudaFree/cudaMemcpy/cudaSetDevice/
cudaError_t/cudaSuccess/cudaMemcpyHostToDevice/DeviceToHost to the SUPA runtime, so
no new call sites are needed. USE_CUDA branches are untouched.
Verified on FPGA 15.1+15.2 (BR208x2, mlx5_0, peermem=false): 15.2 target +
15.1 initiator with --mem_backend=gpu --protocol=rdma, shared etcd 6.6.6.2:2379.
initiator: 'Remote buffer location: supa:0' + 'RDMA compare: OK' rc=0;
target: 'Received RDMA ready ACK'. The SUPA alias path (supaMalloc + dmabuf MR
export via exportDmabuf/registerMemoryRegionInternal) is exercised in-test.
* [BUILD] resolve BIREN_HOME first in common.cmake USE_SUPA block
Replace envSUPA_PATH/SUPA_ROOT + SUPA_DRIVER_LIB_DIR two-var lookup with a
single BIREN_HOME-first lookup (env BIREN_HOME -> /usr/local/birensupa/all/latest),
then include ${BIREN_HOME}/supa/include and link ${BIREN_HOME}/supa/lib
${BIREN_HOME}/brumd/lib. message BIREN_HOME so the resolved root is visible
at configure time.
src/CMakeLists.txt unchanged (target_link_libraries supa supart via dirs above).
Verified on FPGA 15.2: cmake reconfigure prints 'BIREN_HOME: /home/ubuntu/fullstack/full-stack';
transfer_engine + rdma_transport_test build clean; ldd (envs sourced) resolves
libsupa.so.2 / libsupart.so.2 / libbebr.so.2 from brumd/lib + supa/lib.
* [CHORE] remove stale branch-A note from supa.h alias block
* [STYLE] clang-format/cmake-format only the SUPA MR-touched lines
Apply clang-format-20 and cmake-format to the lines this SUPA
transfer-engine MR actually added/modified; no whole-file reflow of
pre-existing style debt on adjacent code.
- supa.h (new file): collapse aligned #define macro columns to
single-space per repo .clang-format
- cuda_alike.h: pad the #if continuation line aligned with the
USE_SUPA clause added by this MR
- rdma_context.cpp: wrap the #if line that grew past ColumnLimit 80
after appending USE_SUPA
- rdma_transport_test.cpp: pad #if/#elif continuation lines aligned
with the USE_SUPA clause added by this MR
- common.cmake: reflow the set(... CACHE) calls inside the MR-added
USE_SUPA BIREN_HOME block
Verified in-place:
- clang-format-20 --dry-run --Werror: PASS on all 6 MR-touched cpp/h
- cmake-format --check: the MR-added CMake hunks are clean; remaining
cmake-format findings on these files are pre-existing style debt on
lines this MR did not touch and are intentionally left unchanged.
---------
Co-authored-by: e00977 <yqcai@birentech.com>
…nit (kvcache-ai#3263) Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
…cal NICs (kvcache-ai#3219) The EFA transport registers every single-chunk buffer on every NIC. That is incidental to the `chunks.size() <= 1` branch rather than deliberate: the multi-chunk paths already partition NICs, only the common case does not. For device memory the cost is not linear in the NIC count but worse. EFA charges a device-memory registration roughly in proportion to the device bytes already registered on that libfabric domain, and that cost is paid once per domain, so fan-out multiplies accumulation. Measured on p5.48xlarge (32 NICs, 8 GPUs), 48 x 391 MB GPU buffers registered serially: all NICs (32) 116.1 s topology-local (4) 14.9 s -- 7.8x An 8-rank replay of a Kimi-K3 KV registration (182 GPU buffers per rank, 14.3 GiB, one TransferEngine per rank as SGLang does it) at MC_MAX_CONCURRENT_REG_MR=16, in the buffer order SGLang actually passes: all NICs (32) 35.0 s wall 2981 CPU-s 85 cores busy topology-local 5.2 s wall 394 CPU-s 77 cores busy -- 6.7x / 7.6x This does not subsume MC_MAX_CONCURRENT_REG_MR. `local` cuts the work per registration, not the number of them in flight: the thread count is the buffer count, independent of the NIC set. Unbounded, a `local` batch still peaked at 1289 threads and 182 of 192 cores busy, versus 168 threads and 77 cores with the cap. The two are orthogonal and both are needed. Add MC_EFA_NIC_SELECTION=all|local. `local` narrows a single-chunk *device* buffer to the NICs the topology reports as closest to its GPU. No new PCI walk or hwloc dependency: Topology::discover() already computes exactly this. For a "cuda:N" entry, preferred_hca holds the HCAs at minimum PCI distance from that GPU within its NUMA node -- on p5.48xlarge the 4 EFA devices sharing the GPU's PCIe root complex, the same rail group NIXL's libfabric plugin derives from hwloc. buildLocalNicMap() just inverts that into context_list_ indices. Verified on hardware: each of the 8 GPUs resolves to its own 4 rails, and the timing above is symmetric across GPUs (14.9 s on GPU 0, 15.1 s on GPU 5), so it is per-GPU rather than always GPU 0's set. Partial registration needed no new plumbing: lkey/rkey are already pushed as 0 for unassigned NICs, and selectDevice() already retries past a NIC whose rkey is 0. Host memory is deliberately excluded. It has no per-domain accumulation to amplify, it is ~4x cheaper to register anyway, and a host buffer has no single owning device -- "cpu:N"'s preferred set is a NUMA node's NICs, measured as 16 of 32 here, half the machine rather than a rail group. Opt-in, defaulting to the historical behavior, because it trades away per-buffer bandwidth: fewer NICs can serve a transfer touching that buffer. The RDMA transport registers on every context unconditionally too, so defaulting `local` on would also put the EFA transport out of step with the rest of the codebase. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…vcache-ai#3265) Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
…truction (kvcache-ai#3584) * [TransferEngine] Fence stale RDMA handshakes after QP reconstruction Signed-off-by: He Zhou <zh541072@alibaba-inc.com> * [TransferEngine] Add stale RDMA handshake regression test Signed-off-by: He Zhou <zh541072@alibaba-inc.com> --------- Signed-off-by: He Zhou <zh541072@alibaba-inc.com> Co-authored-by: He Zhou <zh541072@alibaba-inc.com>
…vcache-ai#3684) An idle pooled connection only learns that its peer restarted when it is next used, and that first use fails. Discarding just that one client left every other connection pooled for the same peer equally stale, so the caller paid one failed call per stale connection. TcpTransport retries a bounded number of times (max_retry_count), so a peer that was already back up could still fail a transfer outright, and notifications, which have no retry above this layer, were dropped. Drop the whole pool for that address instead, so the next attempt starts from a fresh connection. This deliberately does not retry the failed call in place. Pin, Unpin, Delegate and Notify share this path and mutate remote state, and the error tells us nothing about whether the peer ran the handler: ylt reports both "never sent" and "ran, but the reply was lost" as io_error. Recovery stays with the layer that already owns it and knows whether its operation is safe to repeat. The flush is skipped when the failure came back from the peer as a reply - a handler exception, an unknown function id, a rejected argument. The connection carried a full round trip in those cases, so it is fine, and so are the ones other callers of the same address are holding. The pool also carries a generation. A client is handed out with the generation it came from, and a failure only flushes if that is still the current one. Several callers notice the same restart at once; without this the second one to fail would throw away the connections the first one has already re-established, and the pool would churn for as long as stale clients keep surfacing. rpc_reconnect_test: - StalePoolIsFlushedSoTheNextCallSucceeds - a concurrent burst parks two connections in the pool, the peer restarts on the same port, and the call after the first (unavoidable) failure has to succeed. Fails without the flush. - AFailedCallIsNotRetried - the peer runs a side effect and only then fails; it must have run exactly once. Fails against a version that retries, reporting two executions. - HandlerFailureLeavesThePoolAlone, DeadPeerStillFails, ConcurrentStaleCallsAllRecover - guards for behaviour that must not change. They pass either way and say so in their comments. tent suite: 31/31 passed. Co-authored-by: Frank <xiaodouzi6661@gmail.com>
…e-ai#3562) * [TransferEngine] Reuse TCP CUDA staging buffers across chunks Cache accelerator device detection once per request and reuse a growable staging buffer within TCP client and server sessions instead of allocating one for every chunk. Add CUDA regression coverage for multi-chunk READ/WRITE correctness and staging allocation/device-query counts. Refs kvcache-ai#3446 * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix ci problem --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
batchTransferSync returned -1 silently from three paths: openSegment failure, submitTransfer failure, and exhausting all retries. A failed KV/state transfer therefore surfaced to callers (e.g. sglang) as an opaque -1 with nothing in the engine log, which is what made the NVLink ordinal bug look like a mystery. Log each path with the target host, batch size, total length and retry context. Signed-off-by: staryxchen <staryxchen@tencent.com>
…3604) * fix(rdma): evict stale endpoints on port recovery (fixes kvcache-ai#3596) When an RDMA port goes down and comes back up, the IBV_EVENT_PORT_ACTIVE path only called context->resume(), flipping a DeviceStatus flag. QPs that entered IBV_QPS_ERR while the link was down were never destroyed or rebuilt. The endpoint store kept returning these stale endpoints, so work requests were posted into dead QPs and never completed. Fix: add EndpointStore::evictAll() to both FIFOEndpointStore and SIEVEEndpointStore. evictAll() moves every entry from the active map to the waiting list and calls beginDestroy() on each, transitioning QPs to ERR state so the hardware flushes inflight WRs to the CQ. RdmaContext::resume() now calls evictEndpoints() (which calls evictAll()) after successfully flipping status back to DEVICE_ENABLED. The next getOrInsert() call for each peer will create a fresh endpoint with a new QP, restoring full throughput. Fixes kvcache-ai#3596 Co-Authored-By: Claude <noreply@anthropic.com> * fix(rdma): apply clang-format to evictAll() in SIEVEEndpointStore The for-loop body on a single line exceeded the 88-column limit enforced by clang-format-20 in CI. Split it into a braced body. Co-Authored-By: Claude <noreply@anthropic.com> * fix(rdma): add missing evictAll() declaration in SIEVEEndpointStore SIEVEEndpointStore::evictAll() was implemented in endpoint_store.cpp but not declared in the header, leaving the class abstract and causing a compile error in tent-ci. Co-Authored-By: Claude <noreply@anthropic.com> * fix(rdma): add missing evictAll() declaration in FIFOEndpointStore FIFOEndpointStore::evictAll() was implemented in endpoint_store.cpp but not declared in the header, leaving the class abstract. Co-Authored-By: Claude <noreply@anthropic.com> * ci: trigger re-run Co-Authored-By: Claude <noreply@anthropic.com> * ci: trigger re-run Co-Authored-By: Claude <noreply@anthropic.com> * fix(rdma): insert into waiting_list_ under endpoint_map_lock_ in evictAll Publish evicted endpoints into waiting_list_ while holding the endpoint_map_lock_ write lock, instead of after releasing it. reclaim() iterates waiting_list_ under endpoint_map_lock_, so an insert outside the lock was an unsynchronized read/write on the unordered_set (alogfans review on kvcache-ai#3604). beginDestroy() still runs after the lock is released since it may call back into reclaim(). * ci: re-trigger CI * ci: re-trigger * ci: re-trigger CI --------- Co-authored-by: zhaoye <zhaoye@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: zhaoye <yzhao04@iquestlab.com>
…vcache-ai#3677) Metric updates took a mutex-protected map lookup per call, and because TENT labels come from small fixed enums, all updates for a given label landed on the same shard mutex. Add cached_metric.h with CachedDynamicCounter (resolve() hands out a stable per-label atomic cell; incCached() is a relaxed fetch_add) and CachedDynamicHistogram (bucket counters and the sum counter built from cached cells, replacing ylt's dynamic histograms and the parallel standalone sum counters). TentMetrics slots N=1 metrics by TransportType and N=2 attempt metrics by (transport, operation), resolving cells lazily with race-free publication; the rare failover counter keeps the locked path. Prometheus/JSON output is unchanged. Add a concurrency test asserting exact counts when multiple threads resolve and update overlapping label cells.
…vcache-ai#3678) NVLinkTransport::relocateSharedMemoryAddress called cudaSetDevice() on the peer BufferDesc's device index before cudaIpcOpenMemHandle. That ordinal is relative to the PEER's CUDA_VISIBLE_DEVICES: with per-instance CUDA_VISIBLE_DEVICES (e.g. GPUs partitioned between prefill/decode workers) it can be out of range locally or point at a different physical GPU, so every transfer to the peer's higher-numbered devices failed -- silently, since CHECK_CUDA returned without logging. Open the handle on the caller's current device instead: cudaIpcMemLazyEnablePeerAccess takes care of the mapping, and the copies already run cross-device via P2P. Remove the same device switch from MnnvlTransport (its driver calls are device-agnostic). Also: CHECK_CUDA/CHECK_CU now LOG(ERROR) before returning; NVLinkTransport::submitTransferTasks rolls back half-appended task entries on synchronous failure (no I/O has started yet). Test: tent_nvlink_transport_test exports a real IPC handle from a forked child and registers it with an out-of-range ordinal. Not registered in ctest (needs a real GPU; run manually on a GPU box). Signed-off-by: staryxchen <staryxchen@tencent.com>
…he scenario DSL (kvcache-ai#3656) * [Store] Migrate object lifecycle and tenancy MasterService tests to the scenario DSL Consume the typed DSL from kvcache-ai#3510 for the client-visible put, upsert, revoke, batch, remove, and regex flows plus object groups, tenant isolation, and quota admission listed in kvcache-ai#3572. Replace lease-expiry sleeps with deterministic ExpireAt. Keep implementation-only assertions as focused tests; SoftPinRequestValidation is trimmed to its internal zero-TTL remainder. Closes kvcache-ai#3572 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [Store] Restore lease-extension, combined-regex, and tenant key-count coverage Address review on kvcache-ai#3656: three coverage regressions in the migrated tests. - RemoveLeasedObject: add the deterministic ExpireAt -> KeyExists -> Remove(OBJECT_HAS_LEASE) -> ExpireAt sequence so ExistKey extending an existing (expired) lease is asserted, mirroring the GetReplicaList half of the scenario. - RemoveByRegexComplex: migrate the legacy /|\d$ case as a fifth scenario, asserting the five removed keys (two path keys, three trailing-digit keys) and the eight survivors exactly. - TenantScopedPutsAndRemovesUpdateGlobalKeyCount: focused direct test in the monolith covering the legacy GetKeyCount 2 -> 1 cross-tenant transition; GetKeyCount stays outside the DSL vocabulary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Opening the review against upstream kvcache-ai/Mooncake instead. |
github-actions
Bot
added
run-ci
store
Transfer Engine
PyTorch Backend
Mooncake EP
Installation
labels
* update readme * update readme * update readme --------- Co-authored-by: Ke Yang <yangke@approaching.ai>
Co-authored-by: Yuchen Kou <kouyuchen@approaching.ai>
zxpdemonio
force-pushed
the
codex/dummy-register-buffer-pr
branch
from
August 27, 2026 10:07
fba6f34 to
95fa49e
Compare
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
20 participants
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.
Description
DummyClient previously accepted only pointers inside its own shared-memory
segments. As a result,
register_buffer(ptr, size)rejected ordinary host,pinned-host, and CUDA allocations, and external addresses could not be used
safely by the Dummy read/write APIs.
This PR adds explicit external-buffer registration semantics:
(base, size);RPC boundary requires it.
The change is limited to DummyClient registration and generic external
pointer transfers. Tensor-specific
*_frommulti-buffer routing is coveredby PR-2.
Module
mooncake-transfer-engine)mooncake-store)mooncake-reshard)mooncake-ep)mooncake-integration)mooncake-wheel)mooncake-common)Type of Change
How Has This Been Tested?
Test commands:
Test results:
dummy_client_get_buffer_testpassed: 15 tests, latest run 77.5 s.size mismatch, overlap rejection, overflow rejection, partial transfer, and
exact unregister.
Review notes:
register_bufferitself performs registration bookkeeping only; it does notallocate a staging buffer or copy user data.
because the RPC handler runs against the Dummy-mapped address space.