kvcache-ai/Mooncake

Fork 0

mirror of https://github.com/kvcache-ai/Mooncake.git synced 2026-11-05 01:17:55 +00:00

[GH-ISSUE #4709] [Bug]: Potential incorrect metadata removal for shared-address multi-protocol registrations #1200

New issue

Open

opened 2026-10-29 21:23:52 +00:00 by bsakaguchi · 4 comments

bsakaguchi commented

2026-10-29 21:23:52 +00:00

Owner

Originally created by @xp2001-coder on GitHub (Oct 16, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4709

Bug Report

Description

I noticed a potential metadata consistency issue in the classic Mooncake Transfer Engine while investigating multi-protocol registration with the MACA backend on MetaX GPUs.

Under ENABLE_MULTI_PROTOCOL, multiple transport-specific BufferDesc entries may refer to the same memory address. However, TransferMetadata::removeLocalMemoryBuffer() currently identifies the descriptor to remove only by its address, without considering the transport protocol.

When multiple protocol descriptors share the same address, this may remove the wrong descriptor and potentially leave stale metadata for a transport that has already been unregistered.

Environment

The preliminary reproduction was performed with the following configuration:

Mooncake commit: a9983dcd600270fcc5c0cc6df92906b1e1785f8c
Transfer Engine: Classic
Build options: USE_MACA=ON, ENABLE_MULTI_PROTOCOL=ON
Hardware: 2 × MetaX C600-A GPUs (single node)
MACA version: 3.9.0.29
Driver version: 3.9.6
OS: Ubuntu 24.04

The observed metadata behavior was reproduced using synthetic buffer descriptors. A full hardware-backed same-address multi-protocol unregister scenario has not yet been validated.

Relevant Code

The current removal logic in TransferMetadata::removeLocalMemoryBuffer() essentially performs:

if (iter->addr == (uint64_t)addr) {
    new_segment_desc->buffers.erase(iter);
    break;
}

The removal interface does not accept a protocol identifier, so it cannot distinguish between descriptors belonging to different transports at the same address.

Additionally, TransferEngineImpl tracks local memory regions using an address-based index, while the multi-protocol unregister path invokes transport-specific unregister operations.

This raises a potential mismatch between per-transport registration ownership and address-based metadata cleanup.

Preliminary Reproduction

A synthetic metadata test was constructed with two BufferDesc entries sharing the same address, ordered as [tcp, maca].

The intended operation was to remove the MACA registration. However, the underlying metadata removal interface receives only the address, without any protocol information.

Observed behavior:

Initial descriptors:   [tcp, maca]
Intended removal:      maca
Actual descriptors:    [maca]
Expected descriptors:  [tcp]

Because the removal logic selects the first descriptor matching the address, it removes the TCP descriptor while retaining the MACA descriptor.

This demonstrates that address-only removal is insufficient to distinguish protocol-specific descriptors in the constructed multi-protocol scenario.

The reproduction isolates metadata removal behavior; it does not establish that the complete hardware-backed unregister path necessarily exhibits the same outcome.

I reviewed several related multi-protocol registration and unregister fixes:

#2724 / #2725: Addressed incorrect transport-specific descriptor selection when the same GPU memory was registered under both HIP and RDMA. This demonstrates a real shared-address multi-protocol scenario, but the fix focuses on transfer-time descriptor lookup, rather than metadata removal during unregister.
#4240 / #4243: Address partial unregister failures and recovery of per-transport registration state. PR #4243 introduces tracking of transport-specific registration and unregister progress, which is closely related to the ownership semantics discussed here. However, it does not explicitly establish protocol-aware BufferDesc removal as part of the reported fix.
#2962: Improved best-effort unregister behavior by attempting unregister operations across transports even when an individual transport reports an error.

The concern reported here is specifically about which protocol-specific metadata descriptor is removed when multiple descriptors share the same address, rather than transfer-time selection or recovery from partial unregister failures.

It would also be useful to clarify whether the ongoing ownership-tracking work in #4243 is expected to address this metadata consistency concern.

Questions for Maintainers

Is registering and publishing multiple transport-specific descriptors for the same memory address range an intended and supported use case across the available backends?
Should metadata removal identify the target descriptor using (address, transport protocol) rather than address alone?
Should TransferEngineImpl retain per-transport registration ownership in coordination with the metadata layer?
How should partial registration rollback and protocol-specific unregister operations maintain consistency with shared metadata, particularly in light of the ongoing work in #4243?

Validation Scope

This finding is based on source inspection and a synthetic metadata reproduction performed while investigating MACA multi-protocol behavior.

End-to-end validation of the same-address MACA + RDMA registration/unregistration flow on real hardware has not yet been completed. Therefore, the impact on actual multi-protocol deployments remains to be confirmed.

I would like to clarify the intended registration ownership and metadata cleanup semantics before proposing changes to the Transfer Engine implementation.

Before submitting...

Ensure you searched for relevant issues and read the [documentation]

Originally created by @xp2001-coder on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4709 ### Bug Report ## Description I noticed a potential metadata consistency issue in the classic Mooncake Transfer Engine while investigating multi-protocol registration with the **MACA backend on MetaX GPUs**. Under `ENABLE_MULTI_PROTOCOL`, multiple transport-specific `BufferDesc` entries may refer to the same memory address. However, `TransferMetadata::removeLocalMemoryBuffer()` currently identifies the descriptor to remove **only by its address, without considering the transport protocol**. When multiple protocol descriptors share the same address, this may remove the wrong descriptor and potentially leave stale metadata for a transport that has already been unregistered. ### Environment The preliminary reproduction was performed with the following configuration: - **Mooncake commit:** `a9983dcd600270fcc5c0cc6df92906b1e1785f8c` - **Transfer Engine:** Classic - **Build options:** `USE_MACA=ON`, `ENABLE_MULTI_PROTOCOL=ON` - **Hardware:** 2 × MetaX C600-A GPUs (single node) - **MACA version:** 3.9.0.29 - **Driver version:** 3.9.6 - **OS:** Ubuntu 24.04 The observed metadata behavior was reproduced using synthetic buffer descriptors. A full hardware-backed same-address multi-protocol unregister scenario has not yet been validated. ### Relevant Code The current removal logic in `TransferMetadata::removeLocalMemoryBuffer()` essentially performs: ```cpp if (iter->addr == (uint64_t)addr) { new_segment_desc->buffers.erase(iter); break; } ``` The removal interface does not accept a protocol identifier, so it cannot distinguish between descriptors belonging to different transports at the same address. Additionally, `TransferEngineImpl` tracks local memory regions using an address-based index, while the multi-protocol unregister path invokes transport-specific unregister operations. This raises a potential mismatch between **per-transport registration ownership** and **address-based metadata cleanup**. ### Preliminary Reproduction A synthetic metadata test was constructed with two `BufferDesc` entries sharing the same address, ordered as `[tcp, maca]`. The intended operation was to remove the MACA registration. However, the underlying metadata removal interface receives only the address, without any protocol information. **Observed behavior:** ```text Initial descriptors: [tcp, maca] Intended removal: maca Actual descriptors: [maca] Expected descriptors: [tcp] ``` Because the removal logic selects the first descriptor matching the address, it removes the TCP descriptor while retaining the MACA descriptor. This demonstrates that **address-only removal is insufficient to distinguish protocol-specific descriptors** in the constructed multi-protocol scenario. The reproduction isolates metadata removal behavior; it does not establish that the complete hardware-backed unregister path necessarily exhibits the same outcome. ### Related Issues and Prior Work I reviewed several related multi-protocol registration and unregister fixes: - **#2724 / #2725:** Addressed incorrect transport-specific descriptor selection when the same GPU memory was registered under both HIP and RDMA. This demonstrates a real shared-address multi-protocol scenario, but the fix focuses on **transfer-time descriptor lookup**, rather than metadata removal during unregister. - **#4240 / #4243:** Address partial unregister failures and recovery of per-transport registration state. PR #4243 introduces tracking of transport-specific registration and unregister progress, which is closely related to the ownership semantics discussed here. However, it does not explicitly establish protocol-aware `BufferDesc` removal as part of the reported fix. - **#2962:** Improved best-effort unregister behavior by attempting unregister operations across transports even when an individual transport reports an error. The concern reported here is specifically about **which protocol-specific metadata descriptor is removed when multiple descriptors share the same address**, rather than transfer-time selection or recovery from partial unregister failures. It would also be useful to clarify whether the ongoing ownership-tracking work in #4243 is expected to address this metadata consistency concern. ### Questions for Maintainers 1. Is registering and publishing multiple transport-specific descriptors for the same memory address range an intended and supported use case across the available backends? 2. Should metadata removal identify the target descriptor using **(address, transport protocol)** rather than address alone? 3. Should `TransferEngineImpl` retain per-transport registration ownership in coordination with the metadata layer? 4. How should partial registration rollback and protocol-specific unregister operations maintain consistency with shared metadata, particularly in light of the ongoing work in #4243? ### Validation Scope This finding is based on source inspection and a synthetic metadata reproduction performed while investigating MACA multi-protocol behavior. End-to-end validation of the same-address MACA + RDMA registration/unregistration flow on real hardware has not yet been completed. Therefore, the impact on actual multi-protocol deployments remains to be confirmed. I would like to clarify the intended registration ownership and metadata cleanup semantics before proposing changes to the Transfer Engine implementation. ### Before submitting... - [x] Ensure you searched for relevant issues and read the [documentation]

bsakaguchi added the

bug

label

2026-10-29 21:23:52 +00:00

bsakaguchi commented

2026-10-29 21:23:53 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Oct 16, 2026):

Thanks for opening this issue, @xp2001-coder!

Field	Value
Issue	#4709
GitHub user ID	`332348953`
Reporter	@xp2001-coder

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Oct 16, 2026): Thanks for opening this issue, @xp2001-coder! | Field | Value | |-------|-------| | **Issue** | #4709 | | **GitHub user ID** | `332348953` | | **Reporter** | @xp2001-coder | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi commented

2026-10-29 21:23:53 +00:00

Author

Owner

@Cross2pro commented on GitHub (Oct 16, 2026):

Withdrawing this claim.

@xp2001-coder, I see you already have open PR #4738 (Refs #4709) with the address + protocol match and regression tests. Leaving this for you; thanks for picking it up.

@Cross2pro commented on GitHub (Oct 16, 2026): Withdrawing this claim. @xp2001-coder, I see you already have open PR #4738 (`Refs #4709`) with the address + protocol match and regression tests. Leaving this for you; thanks for picking it up.

bsakaguchi commented

2026-10-29 21:23:53 +00:00

Author

Owner

@xp2001-coder commented on GitHub (Oct 17, 2026):

I'd like to take this.

@Cross2pro Thanks for the analysis and for offering to work on this!

I have completed an implementation with regression tests and pushed the commit here:

github.com/xp2001-coder/Mooncake@32bd1aac

It follows the same approach you outlined: matching by address and protocol, with synthetic tests ensuring that removing one protocol's descriptor preserves the other.

All seven new regression tests passed with multi-protocol support enabled. I'm opening a PR with the implementation and detailed validation results.

I'd appreciate your review, especially around backward compatibility, ambiguous matches, and the transport unregister paths. Thanks!

@xp2001-coder commented on GitHub (Oct 17, 2026): > I'd like to take this. @Cross2pro Thanks for the analysis and for offering to work on this! I have completed an implementation with regression tests and pushed the commit here: https://github.com/xp2001-coder/Mooncake/commit/32bd1aac It follows the same approach you outlined: matching by address and protocol, with synthetic tests ensuring that removing one protocol's descriptor preserves the other. All seven new regression tests passed with multi-protocol support enabled. I'm opening a PR with the implementation and detailed validation results. I'd appreciate your review, especially around backward compatibility, ambiguous matches, and the transport unregister paths. Thanks!

bsakaguchi commented

2026-10-29 21:23:53 +00:00

Author

Owner

@xp2001-coder commented on GitHub (Oct 17, 2026):

I've opened #4738 with the implementation, regression tests, and detailed validation results.

@Cross2pro Thanks again for your analysis and for offering to help! I'd appreciate your review, especially around backward compatibility, ambiguous matches, and the transport unregister paths.

I'll add cross-host validation results to the PR once available.

@xp2001-coder commented on GitHub (Oct 17, 2026): I've opened #4738 with the implementation, regression tests, and detailed validation results. @Cross2pro Thanks again for your analysis and for offering to help! I'd appreciate your review, especially around backward compatibility, ambiguous matches, and the transport unregister paths. I'll add cross-host validation results to the PR once available.