[alogfans]

Description

This PR addresses an RDMA endpoint lifetime race exposed after #2696 when multiple transfer workers are enabled. The previous worker/polling changes could still allow different paths to operate on the same peer endpoint lifecycle concurrently.

The change keeps the stable owner model for RDMA dispatch:

peer_nic_path -> owner worker -> owner CQ -> endpoint lifecycle

and adds a simple per-owner lifecycle gate so endpoint create/setup/post/delete paths for the same peer owner are serialized across transfer workers, passive handshake handling, completion failure handling, QP fatal events, and context-level endpoint disconnects.

Key changes:

• Centralize peer_nic_path -> owner worker -> CQ mapping in RdmaContext.
• Ensure RdmaContext::construct() creates at least one CQ per transfer worker.
• Remove the previous round-robin CQ fallback for endpoint creation.
• Add endpoint lifecycle locks keyed by owner worker.
• Use the same lifecycle gate from worker post/completion paths, passive setup, QP fatal deletion, and batch context disconnect paths.
• Keep worker thread creation, owner hashing, CQ mapping, and lifecycle lock sizing on the same constructed worker-count snapshot.

Module

• Deployment / Build
• Documentation
• Frontend
• Mooncake Store
• Transfer Engine (mooncake-transfer-engine)
• Python Bindings
• Other

Type of Change

• Bug fix
• New feature
• Refactor
• Performance improvement
• Documentation update
• Test-only change
• Build / CI
• Other

How Has This Been Tested?

Test commands:

git diff --check 9c411835570a9ce17458302bc363dcb7ce915cb8..HEAD
cmake --build build --target rdma_transport -j2
cmake --build build --target endpoint_store_test -j2
cmake --build build --target worker_pool_rail_state_test -j2
cmake --build build --target rdma_endpoint_state_test -j2
cmake --build build --target rdma_async_event_drain_test -j2
ctest --test-dir build -R '^(endpoint_store_test|worker_pool_rail_state_test|rdma_endpoint_state_test|rdma_async_event_drain_test)$' --output-on-failure

Hardware validation on local mlx5 devices:

ibv_devices
ibv_devinfo -d mlx5_1
ibv_devinfo -d mlx5_2
ibv_devinfo -d mlx5_3
ibv_devinfo -d mlx5_4
MC_METADATA_SERVER=P2PHANDSHAKE MC_WORKERS_PER_CTX=4 \
  build/mooncake-transfer-engine/tests/rdma_endpoint_reestablish_test \
  --gtest_filter='RDMAEndpointReestablishTest.InjectedSenderRnicDownKeepsFallbackTransfersSmoothForTwentySeconds:RDMAEndpointReestablishTest.InjectedReceiverRnicDownKeepsFallbackTransfersSmoothForTwentySeconds' \
  --gtest_repeat=5 --gtest_break_on_failure --gtest_color=no --logtostderr=1

mlx5_1, mlx5_2, mlx5_3, and mlx5_4 were present with active Ethernet RoCE ports. The repeated 4-RNIC injected fallback stress passed 10/10 subtests across 5 iterations. It completed 131,415 successful 4 MiB transfers, about 513.34 GiB total, while repeatedly injecting sender mlx5_2 and receiver mlx5_3 down/up events. Log scan found no failed tests, assertions, timeouts, crashes, or sanitizer/UAF messages.

Manual validation: verified whitespace checks, rebuilt the RDMA transport target, rebuilt the related test targets serially, and ran the related tests successfully.

Note: a full RDMAEndpointReestablishTest.* run on the same hardware passed 5/7 tests. The two failures were the AutoGID synthetic fault-injection expectations (getInjectedFailureCount() == 0), while the actual data transfer/re-establish steps completed successfully. The 4-RNIC fallback tests relevant to this endpoint lifecycle change passed.

Checklist

• I have performed a self-review of my code.
• My code follows the style guidelines of this project.
• I have run pre-commit run --files <changed files> locally. (pre-commit is not installed in this local environment.)
• I have added tests that prove my fix is effective or that my feature works.
• I have updated the documentation accordingly.
• I have added/updated RFC where needed.
• I used AI assistance for this PR and have reviewed the generated changes.

[Copilot]

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR fixes an RDMA endpoint lifetime race when multiple transfer workers are enabled by serializing endpoint create/setup/post/delete paths per peer “owner” worker and aligning CQ usage with that owner model.

Changes:

• Centralizes peer_nic_path -> owner worker -> CQ mapping in RdmaContext and routes worker posting/polling through it.
• Adds per-owner endpoint lifecycle locks and uses them from posting, completion handling, passive setup, and QP fatal event deletion paths.
• Updates endpoint construction to bind endpoints to a specific CQ (removing round-robin CQ selection) and adds a stress test for stale raw-pointer lookup/delete.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
mooncake-transfer-engine/tests/endpoint_store_test.cpp	Adds a stress test to ensure raw-pointer lookup/delete compares identity only and doesn’t dereference stale pointers.
mooncake-transfer-engine/src/transport/rdma_transport/worker_pool.cpp	Reworks posting/polling to be owner-thread based, adds lifecycle gating, and moves CQ polling to per-worker CQ selection.
mooncake-transfer-engine/src/transport/rdma_transport/rdma_transport.cpp	Ensures CQ count is at least workers_per_ctx during context initialization.
mooncake-transfer-engine/src/transport/rdma_transport/rdma_endpoint.cpp	Stores CQ on the endpoint for stable reconstruction; changes finishDestroy timeout behavior; adds peerNicPath() accessor.
mooncake-transfer-engine/src/transport/rdma_transport/rdma_context.cpp	Adds owner-thread/CQ mapping helpers and endpoint lifecycle locks; updates endpoint creation to take a CQ.
mooncake-transfer-engine/src/transport/rdma_transport/endpoint_store.cpp	Requires CQ for endpoint insertion and adds a test-only insertion helper.
mooncake-transfer-engine/include/transport/rdma_transport/worker_pool.h	Reflects queueing changes and adds completion-processing entry point.
mooncake-transfer-engine/include/transport/rdma_transport/rdma_endpoint.h	Adds CQ member, timeout logging flag, and peerNicPath() API.
mooncake-transfer-engine/include/transport/rdma_transport/rdma_context.h	Adds CQ-indexed endpoint creation, CQ selection helpers, and lifecycle lock APIs.
mooncake-transfer-engine/include/transport/rdma_transport/endpoint_store.h	Changes endpoint insertion API to accept a CQ; adds test-only endpoint insertion hook.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[catyans]

The new lifecycle gate is held across setupConnectionsByActive(), including its synchronous handshake RPC. That function deliberately releases RdmaEndPoint::lock_ around sendHandshake() to support simultaneous open, but the outer gate now remains held. If A and B both start an active handshake for the same NIC path, A holds its local owner gate while waiting for B's handler, and B holds its gate while waiting for A's handler. Each onSetupRdmaConnections() then blocks acquiring the gate held by its own active path, so both RPCs can wait on each other.

Could we avoid holding the lifecycle gate across the network RPC—for example, reserve/capture the endpoint generation under the gate, release it for sendHandshake(), then reacquire and validate the generation before setup/post continues? Alternatively, the passive simultaneous-open path needs to remain callable while the local active RPC is outstanding. A barrier-synchronized bidirectional active-handshake regression test would help cover this case.

[catyans]

Re-reviewed 0c900b0acb. Releasing the owner lifecycle gate around setupConnectionsByActive(), then reacquiring it and validating endpoint identity plus active/retired state, resolves the simultaneous-open wait cycle without allowing work to continue on a replaced endpoint. The barrier-synchronized bidirectional regression test covers the original scenario. Approved.

Please run the repository's clang-format step on rdma_endpoint_state_test.cpp before merge; Check code format (and therefore CI Gate) is currently failing only on formatting differences in the new test.

[catyans]

Re-reviewed the current head. The only delta since my previous approval is formatting in the logging and test code; the peer-owner endpoint lifecycle serialization behavior is unchanged. All checks are green. Approving this head.

[alogfans]

@staryxchen Would you like to review this PR? Thank you.

[staryxchen]

LGTM. Only one non-blocking concern: After this PR the owner thread is also the CQ poller, so sendHandshake() inside performPostSend() leaves that CQ unpolled (default connect timeout 5s). A dead peer can then overflow the CQ and fail every QP that shares it — could we drain this CQ around the RPC, or move handshake off the poll loop?