Skip to content

fix(rdma): v2.6.1 hardening — revert unilateral, fix MR leak, drop per-request logs - #267

Merged
ketor merged 1 commit into
dingodb:mainfrom
ketor:fix/v2.6.1-hardening
Aug 6, 2026
Merged

fix(rdma): v2.6.1 hardening — revert unilateral, fix MR leak, drop per-request logs#267
ketor merged 1 commit into
dingodb:mainfrom
ketor:fix/v2.6.1-hardening

Conversation

@ketor

@ketor ketor commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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/BatchPut both route through CacheFromCacheMany (pipelined), so the advertised performance path did not match the actual call chain. Removes PostSendNotify + nbuf_/nmr_ entirely.

2. Fix per-QP notification MR/memory leak

Close() cleared nmr_/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

  • Every RDMA completion logged at INFO with global-mutex fprintf (546)
  • Three copies of the unilateral decoder existed in decode_request; two removed, the third removed now
  • reinterpret_cast<const uint64_t*> raw wire load removed

4. Remove dead reaper

StartReaper/StopReaper had zero callers, reaper_enabled_ unused, Close() never called StopReaper() — latent lifetime hazard.

5. Default idle back to 10 min

DFKV_RDMA_IDLE_MS default 600000; K8s launcher exports 30000 explicitly.

Verification (0064, B200, mlx5_0; 32GB recv segment, 32GB cap, 64KB slab granularity, credits=256)

Matrix fails
t32 PUT+GET 10k, batch 1/2/8/32 0
depth 1/4/8 (t32, b1, 10k) 0
1MB PUT+GET (t32, b1, 2k) 0
evictions 0

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*).

…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).
@ketor
ketor added this pull request to the merge queue Aug 6, 2026
Merged via the queue into dingodb:main with commit 301f587 Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant