Skip to content

fix(rdma): CacheMany posts one RECV per slot, not two - #266

Merged
ketor merged 1 commit into
dingodb:mainfrom
ketor:fix/cachemany-dual-recv
Aug 6, 2026
Merged

fix(rdma): CacheMany posts one RECV per slot, not two#266
ketor merged 1 commit into
dingodb:mainfrom
ketor:fix/cachemany-dual-recv

Conversation

@ketor

@ketor ketor commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Problem

PR#264's revert kept the unilateral-PUT layout in CacheMany: a PostRecv before Encode plus another PostRecv inside the WRITE_WITH_IMM guard. 4 slots x 2 RECVs = 8 RECVs against RQ depth depth+1=5. Server status SENDs consumed the first-posted RECVs (wr_id 0,0,1,1), so slots 2-3 never got completions: rbytes=[18,18,0,0], batch>1 PUT ~50% fails (t1/b4: 1000/2000 on both mlx5_0 and ib7s400p0, 32GB recv segment, 0 exhausted).

Fix

Restore v2.5.0 order: Encode first, then one PostRecv + one PostWriteImmScatter per slot.

Verification (0064 loopback, 32GB recv segment, 32GB cap, credits=256)

Test Before After
t1/b4 PUT 2000 1000 fails 0 fails
t1/b1 PUT 2000 0 fails 0 fails
t32/b1 PUT/GET 20000 - 0 fails
t32/b4 PUT/GET 20000 - 0 fails

Note: earlier reported batch>1 fails at high thread counts were two environment configs, not code: recv-segment/cap too small (slab evictions) and DFKV_RDMA_RAIL_CREDITS=64 default capping 32t x b4 = 128 credits.

PR#264's revert kept the unilateral-PUT layout: a PostRecv before
Encode plus another PostRecv in the WRITE_WITH_IMM guard. With 4 slots
that is 8 RECVs against an RQ depth of depth+1=5, so the server's
status SENDs consumed the first-posted RECVs (wr_id 0,0,1,1), leaving
slots 2-3 without a completion -> rbytes=[18,18,0,0] -> batch>1 PUT
~50% fails (t1/b4: 1000/2000).

Restore the v2.5.0 order: Encode first, then one PostRecv + one
PostWriteImmScatter per slot.

Verified on 0064 loopback (32GB recv segment, 32GB cap, credits=256):
- t1/b4 PUT 2000: 0 fails (was 1000/2000)
- t1/b1 PUT 2000: 0 fails
- t32/b1 PUT/GET 20000: 0 fails
- t32/b4 PUT/GET 20000: 0 fails
@ketor
ketor added this pull request to the merge queue Aug 6, 2026
Merged via the queue into dingodb:main with commit 3664172 Aug 6, 2026
yuhaijun999 pushed a commit to yuhaijun999/DingoCache that referenced this pull request Aug 7, 2026
…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).
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