Skip to content

[Bugfix][Store] Report the mounted segment's protocol in buffer descriptors - #4045

Open
NUABO wants to merge 1 commit into
kvcache-ai:mainfrom
NUABO:fix-rdma
Open

[Bugfix][Store] Report the mounted segment's protocol in buffer descriptors#4045
NUABO wants to merge 1 commit into
kvcache-ai:mainfrom
NUABO:fix-rdma

Conversation

@NUABO

@NUABO NUABO commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

/query_key reported protocol_: "tcp" for replicas on an RDMA-mounted segment. The descriptor's protocol comes from AllocatedBuffer::protocol, which defaults to "tcp" and was only ever written by change_to_cxl(), so it never carried the protocol of the segment the buffer was allocated from.

Carry the protocol on the allocator and stamp it when a buffer is constructed:

  • BufferAllocatorBase::getTransferProtocol() (defaults to "tcp", deliberately not pure so DummyBufferAllocator and out-of-tree subclasses keep compiling).
  • CachelibBufferAllocator / OffsetBufferAllocator keep the protocol they were created with; Create / CreateBufferAllocator / Import* take it as a trailing defaulted parameter so existing callers (tests, benchmarks) are unchanged.
  • The AllocatedBuffer ctor moves out of line (BufferAllocatorBase is incomplete in the header) and stamps the protocol from the owning allocator.
  • Production call sites pass it through: the segment mount path, the region driver open/adopt paths, and the HA remount / standby-restore imports, which now carry it in RegionResourceSpec.protocol.

"cxl" is never stamped from an allocator. Unlike a transport it marks an address already offset-encoded by change_to_cxl(), which rewrites the pointer in the same call, so it must stay a per-buffer invariant: the CXL allocators are created without a protocol. Taking it from the allocator would let a buffer that has not been offset-encoded advertise "cxl", and deallocate() would then free ptr + DEFAULT_CXL_BASE — reachable by mounting a segment as "cxl" on a master with CXL disabled, which falls through to the regular allocator path.

NoF segments keep the transfer default: NoFSegment has no protocol field (NoF replicas are addressed by endpoint alone). The HA snapshot format carries no protocol either, so allocators rebuilt from it keep the default.

Tests: AllocatedBufferCarriesSegmentProtocol,
CxlSegmentProtocolNotStampedOnFreshBuffers,
ImportedBuffersCarrySegmentProtocol.

Description

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Reshard (mooncake-reshard)
  • Mooncake EP (mooncake-ep)
  • Mooncake PG (mooncake-pg)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • Common (mooncake-common)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Performance improvement
  • Other

How Has This Been Tested?

Test commands:

# Example: bash scripts/run_ci_test.sh

Test results:

  • Unit tests pass
  • Integration tests pass (if applicable)
  • Manual testing done (describe below)

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (if applicable)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

  • No AI tools were used
  • AI tools were used (specify below)

…iptors

/query_key reported protocol_: "tcp" for replicas on an RDMA-mounted segment.
The descriptor's protocol comes from AllocatedBuffer::protocol, which defaults
to "tcp" and was only ever written by change_to_cxl(), so it never carried the
protocol of the segment the buffer was allocated from.

Carry the protocol on the allocator and stamp it when a buffer is constructed:

- BufferAllocatorBase::getTransferProtocol() (defaults to "tcp", deliberately
  not pure so DummyBufferAllocator and out-of-tree subclasses keep compiling).
- CachelibBufferAllocator / OffsetBufferAllocator keep the protocol they were
  created with; Create / CreateBufferAllocator / Import* take it as a trailing
  defaulted parameter so existing callers (tests, benchmarks) are unchanged.
- The AllocatedBuffer ctor moves out of line (BufferAllocatorBase is incomplete
  in the header) and stamps the protocol from the owning allocator.
- Production call sites pass it through: the segment mount path, the region
  driver open/adopt paths, and the HA remount / standby-restore imports, which
  now carry it in RegionResourceSpec.protocol.

"cxl" is never stamped from an allocator. Unlike a transport it marks an address
already offset-encoded by change_to_cxl(), which rewrites the pointer in the
same call, so it must stay a per-buffer invariant: the CXL allocators are
created without a protocol. Taking it from the allocator would let a buffer that
has not been offset-encoded advertise "cxl", and deallocate() would then free
ptr + DEFAULT_CXL_BASE — reachable by mounting a segment as "cxl" on a master
with CXL disabled, which falls through to the regular allocator path.

NoF segments keep the transfer default: NoFSegment has no protocol field (NoF
replicas are addressed by endpoint alone). The HA snapshot format carries no
protocol either, so allocators rebuilt from it keep the default.

Tests: AllocatedBufferCarriesSegmentProtocol,
CxlSegmentProtocolNotStampedOnFreshBuffers,
ImportedBuffersCarrySegmentProtocol.

Signed-off-by: tan changzhi <544463199@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant