[Bugfix][Store] Support Dummy external buffer registration - #1
Open
zxpdemonio wants to merge 1045 commits into
Open
[Bugfix][Store] Support Dummy external buffer registration#1zxpdemonio wants to merge 1045 commits into
zxpdemonio wants to merge 1045 commits into
Conversation
…ache-ai#3296) The EFA submit path paces outstanding operations with two counters, wr_depth_ (vs max_wr_depth_) and EfaCq::outstanding (vs max_cqe). Neither ceiling was ever handshaken with the queue it paces: * max_wr_depth_ was set from GlobalConfig::max_wr. hints_->tx_attr->size is never set, so fi_endpoint() takes its depth from whatever the provider chose, and the counter was free to disagree. Contrast the RDMA transport, where the same number is passed to ibv_create_qp() and verbs rejects a mismatch at QP creation. * max_cqe came from GlobalConfig::max_cqe (4096), but the EFA RDM provider raises every CQ to MAX(rx_attr->size + tx_attr->size, FI_EFA_CQ_SIZE) regardless of the request (efa_domain.c: rdm_cq_size, applied in efa_rdm_cq.c), which is 12288 on p5. Both are per-device hardware attributes, so no compiled-in default can be right on every instance type: tx_attr->size is 4096 on p5.48xlarge and 2048 on p6-b300.48xlarge (the provider derives it from the device's max_sq_wr). Both directions of disagreement are real transfer failures, and both were seen in production on p6-b300: counter < queue (the 256 default, 8x too shallow): the counter saturates while the queue is mostly empty, so submitters spin in the credit-wait loop and give up -- "timed out waiting for CQ drain (wr_depth=256, max=256)" -- with 1792 slots the NIC would have accepted. counter > queue (MC_MAX_WR=16384, 8x too deep): the counter hands out credit the queue cannot honor, so fi_write returns -FI_EAGAIN with wr_depth pinned at exactly 2048 (the provider's real depth) and 14336 credits nominally free -- "1024 consecutive FI_EAGAIN waves posted nothing". EFA has no transport-level retransmit, so these slices are reported up as FAILED. Take both depths from what the provider actually gave us. For the transmit queue that is fi_info_->tx_attr->size; with no MC_MAX_WR it is adopted verbatim, which makes the out-of-the-box configuration correct on instance types nobody has measured. An explicit MC_MAX_WR below that depth is still honored as a per-NIC throttle; above it is clamped with a warning (LOG_FIRST_N: it is one process-wide mistake, not one per NIC). The new max_wr_from_env flag distinguishes the two cases and leaves the RDMA transport, which passes max_wr to ibv_create_qp(), untouched. For the CQ, read cq_attr.size back after fi_cq_open() rather than recomputing the provider's floor here. The formula folds in FI_EFA_CQ_SIZE (default 8192), so a local copy would be wrong on any host where the operator has set it, and would drift silently if a future provider changes the rule. fi_cq_open() reports the depth it chose in place -- verified on p5, where requests of 64 / 1024 / 4096 / 8192 all come back as 12288 while 20000 is granted as asked. Deliberately not done by setting hints_->tx_attr->size before fi_getinfo(): measured on p5, a hint of 16384 makes fi_getinfo() fail with -FI_ENODATA, turning a mis-set env var from a performance problem into a total init failure. Reading back what the provider chose is the safe order. The supported way to deepen the real queue is FI_EFA_TX_SIZE, which this change tracks automatically (measured: FI_EFA_TX_SIZE=16384 -> tx 16384, CQ 24576, both counters following). The startup log now reports all three numbers so the effective depth is visible without a probe: EFA device (libfabric): rdmap79s0, ... (shared endpoint, max_wr=4096, provider tx queue=4096, max_cqe=12288) Docs: the EFA transport page carried several env-var claims that do not hold. Corrected the MC_MAX_WR guidance to say the default is already right, and documented FI_EFA_TX_SIZE as the knob that does work. Removed the knobs that have no effect on EFA rather than describing them there -- mentioning a no-op knob on the EFA page only invites someone to try it: MC_MAX_CQE_PER_CTX (the provider raises every CQ to its own floor regardless of the request), MC_NUM_CQ_PER_CTX (the shared endpoint binds cq_list_[0] only, so extra CQs never receive completions), MC_SLICE_SIZE (the EFA transport issues one slice per request), and MC_EFA_STRIPING_THRESHOLD, which does not exist in the codebase at all. Also dropped a recommendation to raise MC_EFA_CQ_THREADS: the pollers busy-wait, so extra threads burn whole cores, and they cannot fix FI_EAGAIN, which means the provider is refusing work rather than completions going unreaped. MC_EFA_CQ_THREADS itself was missing from the runtime-options list and is now documented there. Tested on p5.48xlarge (32 EFA NICs, provider tx=4096 rx=8192): * config_test: 44/44 pass, including 5 new MaxWrEnvTest cases covering default / valid override / rejected values (a typo must not be treated as a deliberate override). * efa_transport_test: 10/10 pass unset, at MC_MAX_WR=16384 (clamped to 4096, one warning) and at MC_MAX_WR=1024 (honored). No FI_EAGAIN waves and no CQ-drain timeouts in any config. CQ depth read back as 12288 on every one of the 32 contexts. The CXI transport has the identical pattern in cxi_context.cpp (max_wr_depth_ from max_wr, cq_limit from globalConfig().max_cqe). Left alone here: no Slingshot hardware to verify the fix against. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…he-ai#3264) Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
kvcache-ai#3259) * [TENT] Fix double-free in HttpMetaStore under concurrent workers: a shared CURL* is not thread-safe * [TENT] http: use request handle for curl_easy_escape instead of nullptr * [TENT] http: fix clang-format * ci: retrigger checks (flaky graceful_shutdown_test) --------- Co-authored-by: jiayuzailiu <jiayuzailiu@tencent.com>
* [CI/Build] Simplify wheel workflows * [CI/Build] Preserve host build parallelism
…vcache-ai#3308) * [PG] Build the device worker with C++17 for CMake 3.22 compatibility * fix(musa): build decoupled PG device runtime --------- Co-authored-by: Xun Sun <UNIDY2002@outlook.com>
…he-ai#2962) TransferEngineImpl::unregisterLocalMemory returned on the first transport whose unregister failed, leaving the region registered on the remaining transports and skipping the local bookkeeping erase. Collect the first error but attempt every transport, mirroring the batch path made best-effort in kvcache-ai#2869. Refs kvcache-ai#2869 Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
…he-ai#2955) * [TransferEngine] Skip the CUDA pointer probe on GPU-less hosts getMemoryLocation() probes cudaPointerGetAttributes for every registered buffer in a CUDA-enabled build. On a GPU-less host (e.g. an RDMA-only real-client sidecar) the call fails with no device and logs an ERROR per buffer, drowning the real signal, before falling back to the host path. Detect device presence once via cudaGetDeviceCount and skip the probe entirely when no device exists, logging a single WARNING. Hosts with a GPU are unaffected. Refs kvcache-ai#2937 Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * Extract the CUDA device presence probe into a named helper Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * clang-format: join the device-count condition onto one line --------- Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
* fix(ci): resolve nightly build failures * ci: preserve nightly build environment on install * fix(efa): correct configuration comment
…-ai#3282) Co-authored-by: Le1zyCatt <2490162471@qq.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stmatengss <11641725+stmatengss@users.noreply.github.com>
…ket (kvcache-ai#3289) Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
…#3300) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stmatengss <11641725+stmatengss@users.noreply.github.com>
…cache-ai#3301) MasterMetricManager uses yalantinglibs dynamic_gauge_1t for per-segment metrics (mem_allocated_size_per_segment_, mem_total_capacity_per_segment_, nof_allocated_size_per_segment_, nof_total_capacity_per_segment_). When a segment is unmounted via CommitUnmountSegment, dec_total_mem_capacity() and dec_allocated_nof_size() only decrement the gauge value to 0 but do not remove the label entry from the gauge's internal map. This causes stale 0-value entries to persist indefinitely in Prometheus output. The problem is especially visible after a master restart with snapshot restore: the restored segments carry old client IDs, the reaper eventually expires those clients and calls CommitUnmountSegment, which decrements capacity to 0 but leaves the label behind. After clients remount with new IDs, the old segment names linger as capacity=0 entries. Fix: add remove_segment_metrics() and remove_nof_segment_metrics() that call remove_label_value() on the per-segment gauges, and invoke them in: - ScopedSegmentAccess::CommitUnmountSegment (memory segments) - ScopedNoFSegmentAccess::CommitUnmountSegment (NoF segments) - SegmentManager::releaseCapacityMetrics() (HA teardown) - ~MasterService() standby allocated-size cleanup (HA teardown) Signed-off-by: leonzzhu <leonzzhu@tencent.com>
Fix three categories of compiler warnings in mooncake-transfer-engine: 1. -Wpointer-arith in ibgda/os.h: void* pointer arithmetic in read_all() and write_all() — cast to char* before arithmetic. 2. -Wsign-compare in mlx5gda.cpp: size_t offset variables compared with int literal -1 — use (size_t)-1 to match the variable type. 3. -Wmissing-field-initializers in ibgda_device_transport.cpp: designated initializer for mlx5gda_qp_devctx omitted mutex, bf_offset, wq_head, wq_tail — add explicit zero initializers. Signed-off-by: leonzzhu <leonzzhu@tencent.com>
* [wheel] support jagged NestedTensor transfer * [wheel] optimize nested tensor deserialization
Restore the PR2671 flat-dict GET fast path for encoded non-tensor rollout data. Flat dict reads now avoid the DataProto object-array round trip, msgpack ragged values use streaming decode again, and encoded payload members are materialized by stage. Add regression coverage for dict-vs-DataProto output shape and direct-copy typed-ragged payload tests.
…i#3325) * [Fix] clean up partially initialized TENT RDMA contexts * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * [Fix] prevent CQ leak on construction failure * [Fix] simplify cleanup of partially initialized TENT RDMA contexts * [Fix] hold mr_set_mutex_ and correct cleanup comment in RdmaContext --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…#3339) Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
Co-authored-by: Yuchen Kou <kouyuchen@approaching.ai>
…#3344) These parameterless inc/dec overloads were declared in the header but never implemented or called; keep the segment-keyed APIs that actually exist. Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Schatten <czhengt@qq.com>
…reads (kvcache-ai#3767) TCP bulk copies ran inline on the RPC io_context, serializing concurrent transfers and stalling Probe/Bootstrap. Offload the handlers and default rpc_server_threads higher when TCP is enabled so attachments can be read in parallel. Signed-off-by: staryxchen <staryxchen@tencent.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Register the wrapper module with CTest when unit tests are enabled so Go test failures propagate through the existing test path. Signed-off-by: Miguel Garcia <miguelgarciaroman8@gmail.com>
ci.yml listened for every labeled event, and auto-labeler already applies run-ci, so adding run-e2e-ci cancelled in-progress PR CI and reran every job. Keep Build & Test on open/push and move same-SHA retrigger to a workflow that only reacts to a human-applied run-ci label. Signed-off-by: staryxchen <staryxchen@tencent.com> Co-authored-by: Cursor <cursoragent@cursor.com>
* [Store] Introduce stateful region resource drivers * [Store] Refine region driver recovery boundaries * [Store] Validate CacheLib slab count * [Store] Clarify region driver ownership contracts
* [CI/Build] Keep non-CUDA wheels CUDA-free * [CI/Build] Run wheel smoke test under bash for CUDA scan The non-CUDA CUDA-dependency scan uses process substitution (< <(...)), which sh (dash) rejects at parse time, failing every wheel build variant regardless of VARIANT_FLAG. Run the smoke test step under bash.
* Fix data copy while not on same device * Format code * Format code * Fix code address comment by @staryxchen --------- Co-authored-by: shawnding <shawnding@tencent.com>
…che-ai#3790) * Map MOONCAKE_LOCAL_HOSTNAME to TENT rpc_server_hostname * code format --------- Co-authored-by: ruanzhao <ruanzhao@kingsoft.com>
…vcache-ai#3777) * [TENT] Prefer the LAG-effective port speed from ibv_query_port_speed * [TENT] Let tests inject verbs into RdmaContext and cover the effective-speed path * [TENT] Hold the last effective speed over transient query failures and count them --------- Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
* [Store] Add batch OpLog snapshot coordinator * [Bugfix][Store] Fix batch snapshot coordinator races --------- Co-authored-by: Yuchen Kou <kouyuchen@approaching.ai>
…ai#3785) Signed-off-by: Schatten <czhengt@qq.com>
* perf(store): remove redundant per-file deletion delay * test(store): make concurrent remove test deterministic
…on (kvcache-ai#3782) Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
The ghfast fallback used raw git fetch, which skipped checkout's pull_request_target fork check and ran untrusted PR heads on the privileged self-hosted job. Rewrite github.com via insteadOf and retry the same action so network failures still use the mirror. Signed-off-by: staryxchen <staryxchen@tencent.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…ner (kvcache-ai#3718) * [Bugfix] Refactor RDMA slice dispatch and endpoint reclaim * Improve impl based on comments * [TransferEngine] Avoid RDMA lifecycle gate during active handshake * Reformat
…cache-ai#3468) * feat: Add strict_local_numa to hard-exclude cross-NUMA RDMA rails * Fix code address comment by @alogfans Conflicts: mooncake-transfer-engine/tent/include/tent/transport/rdma/quota.h mooncake-transfer-engine/tent/src/transport/rdma/quota.cpp * Fix code address comment by @staryxchen * fix ci --------- Co-authored-by: shawnding <shawnding@tencent.com>
…i#3582) * [TENT] Preserve ownership after failed memory free * fix(tent): warn when local memory free fails --------- Co-authored-by: codex <codex@local.invalid>
zxpdemonio
force-pushed
the
codex/dummy-register-buffer-pr
branch
from
September 1, 2026 11:58
bc4ebab to
d3c4c9f
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
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.