fix(rdma): v2.6.1 hardening — revert unilateral, fix MR leak, drop per-request logs - #267
Merged
Merged
Conversation
…r-request logs Release-blocking fixes from review: 1. Revert unilateral PUT (PR#263 RoundTrip path) to dual-sided WRITE_WITH_IMM. Unilateral never entered the standard client path: KVClient::Put/BatchPut both route through CacheMany (pipelined), so the advertised performance path did not match the call chain. Removes PostSendNotify + nbuf_/nmr_ entirely, which also eliminates the per-QP notification MR leak (Close() cleared nmr_/nbuf_ vectors BEFORE the dereg loops, so both cleanup loops were always empty). 2. Fix Close() ordering: dereg/free loops run before vector clear (smr_/rmr_/dmr_ kept; nbuf_/nmr_ gone with the unilateral revert). 3. Remove per-request INFO logging in server decode (every RDMA completion logged with global-mutex fprintf), the duplicate unilateral decoders (three copies existed), and the type-punned reinterpret_cast<const uint64_t*> wire load. 4. Remove dead reaper: StartReaper/StopReaper had no callers, reaper_enabled_ unused, Close() never called StopReaper(). 5. Default idle back to 10 min (DFKV_RDMA_IDLE_MS=600000); K8s launcher exports 30000 explicitly. Verified on 0064 (B200, mlx5_0, 32GB recv segment, 32GB cap, 64KB slab granularity, credits=256): t32 PUT+GET 10k @ batch 1/2/8/32, depth 1/4/8, 1MB size — all 0 fails, 0 evictions. Release claimed t32/b1 ~83% fails: root cause was double-PostRecv RQ misalignment (fixed in dingodb#266) plus test-env slab eviction, not shared-QP WR interleaving (Acquire is mutex-guarded).
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.
Release-blocking fixes (review of v2.6.0)
1. Revert unilateral PUT (RoundTrip) to dual-sided WRITE_WITH_IMM
Unilateral never entered the standard client path:
KVClient::Put/BatchPutboth route throughCacheFrom→CacheMany(pipelined), so the advertised performance path did not match the actual call chain. RemovesPostSendNotify+nbuf_/nmr_entirely.2. Fix per-QP notification MR/memory leak
Close()clearednmr_/nbuf_vectors BEFORE the dereg/delete loops, so both cleanup loops were always empty → every QP teardown leaked 64B/slot buffers + MRs. nbuf_/nmr_ removal eliminates the leak; remaining vectors now clear after their loops.3. Drop per-request INFO logging + duplicate decoders + type-punned load
reinterpret_cast<const uint64_t*>raw wire load removed4. Remove dead reaper
StartReaper/StopReaperhad zero callers,reaper_enabled_unused,Close()never calledStopReaper()— latent lifetime hazard.5. Default idle back to 10 min
DFKV_RDMA_IDLE_MSdefault 600000; K8s launcher exports 30000 explicitly.Verification (0064, B200, mlx5_0; 32GB recv segment, 32GB cap, 64KB slab granularity, credits=256)
Release-claimed t32/b1 ~83% fails root cause: double-PostRecv RQ misalignment (fixed in #266) + test-env slab eviction at 1MB granularity — NOT shared-QP WR interleaving (Acquire is mutex-guarded; no thread ever holds the same Conn*).