[TENT] Retire stale endpoint on peer reconnect bootstrap

[guptaishaan]

Fixes #3173

When a peer drops its endpoint (endpoint store eviction, or resetConnection()
after a slice failure) and bootstraps again with a fresh QP set, the passive side
still holds an EP_READY endpoint under the same key. accept() saw the same
peer nic path but a different peer_desc.qp_num, logged "Endpoint already
established ... cannot accept new connection" and returned an error while leaving
the endpoint EP_READY. onSetupRdmaConnections() only drops endpoints in a
terminal state, so the stale one stayed in the store and rejected every later
bootstrap from that peer, wedging all further transfers to that instance.

The stale endpoint now calls beginDestroyNoLock() before returning the error.
Endpoints keep their unidirectional lifecycle, they are still never reset. The
caller's existing terminal-state branch removes the retired endpoint from the
store, and the peer's next bootstrap (the worker resubmits the slice) is served
by a newly constructed endpoint.

Testing:

• New unit test EndpointLifecycleTest.BootstrapWithNewPeerQpsRetiresEstablishedEndpoint.
  It fails before the change (endpoint stays EP_READY) and passes after.
• All 32 tent test binaries pass, including the endpoint store, rdma transport
  and failover suites.
• clang-format 20 clean on the touched files.

Not verified: no RDMA hardware was available, so the two-node handshake was not
run end to end. The test drives accept() on an endpoint with no RdmaContext, so
the verbs work inside beginDestroyNoLock() (QP transition to ERR, notification
QP unregistration) is not covered by it.

Thanks to @rookiedali for the report and the logs, which pinpointed the rejecting
branch.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[guptaishaan]

The two failing checks are not caused by this PR.

build-wheel-cu13 (3.10) does not compile anything this PR touches.
.github/workflows/ci_cu13.yml configures with -DBUILD_UNIT_TESTS=OFF and
never passes -DUSE_TENT=ON, and USE_TENT defaults to OFF
(mooncake-common/common.cmake:164), so mooncake-transfer-engine/tent/ is not
added to the build. All three files in this PR are under that directory.

Supporting evidence:

• The same job passed on the base commit 015a897 (main push run 30468416894) and
  on other PRs that ran around the same time (30480557815, 30478353884,
  30454990178).
• The failing "Build project" step ran for 30 seconds (20:04:43Z to 20:05:13Z).
  Successful runs of that step take 20 to 70 minutes, so it died before doing
  real compilation.
• build-wheel-cu13 (3.12) was cancelled by matrix fail-fast, not an
  independent failure.
• The jobs that do compile the changed files, tent-ci in cuda-off,
  cuda-off-metrics-on and cuda-on, all passed on this commit.
• CI Gate fails only because it aggregates the cu13 job result.

I could not read the raw job logs (log download needs repo admin), so I cannot
say what the 30 second failure actually was. Could someone with write access
re-run that job? I have left the branch as is. It still merges cleanly on
current main, and none of the commits landed since touch these files, so I am
happy to rebase instead if you would prefer a fresh run.

[rookiedali]

Thank you very much for the fix. I would like to additionally ask when this error is more likely to occur, due to network fluctuations?

[alogfans]

LGTM

[guptaishaan]

@rookiedali Network fluctuation is one way in, but it is not required. What the bug needs is asymmetric teardown: one side throws its endpoint away and bootstraps again with fresh QP numbers, while the other side still holds an EP_READY endpoint under the same key. Anything that makes only one side drop the endpoint will do it.

Three ways the active side gets there, all ending in a new bootstrap:

1. Any failed work completion on a slice. In Workers::asyncPollCq (mooncake-transfer-engine/tent/src/transport/rdma/workers.cpp), a failed completion calls disableEndpoint() in both branches, including the one where the retry count is still under max_retry_count and the slice is resubmitted. So a single transient error is enough, it does not have to be a persistent fault. This is the fluctuation-shaped trigger: retry-exceeded from a congested or lossy fabric, a link blip, or a peer that restarted.
2. A software transfer timeout on an inflight slice, same function.
3. Endpoint store eviction, with no network problem at all. Once a context holds endpoint_store_cap endpoints, SIEVEEndpointStore::evictOne() retires a live victim. Only the evicting side drops it, so the peer keeps its EP_READY endpoint and the next transfer in that direction bootstraps with new QPs. The shipped config/transfer-engine.json sets 256, and MC_MAX_EP_PER_CTX overrides it. This one gets more likely the more peers a single NIC talks to, which is why it tends to show up on larger clusters rather than under load.

The reason it looked permanent rather than like a blip: before this change the reject left the endpoint in EP_READY, so the caller's terminal-state branch never removed it from the store, and every later bootstrap from that peer hit the same reject. One transient event turned into a standing wedge for that peer pair until a restart. That matches the message repeating in your logs.

If you can still see the affected run, two things would tell us which trigger you hit: roughly how many peer endpoints that context held versus your MC_MAX_EP_PER_CTX, and whether the message repeated for a single peer or for many at once. Eviction pressure usually hits several peers over time, a fabric error usually starts with one. I have no RDMA hardware to reproduce it on, so that part is read off the code paths and your logs rather than measured.

[rookiedali]

@rookiedali Network fluctuation is one way in, but it is not required. What the bug needs is asymmetric teardown: one side throws its endpoint away and bootstraps again with fresh QP numbers, while the other side still holds an EP_READY endpoint under the same key. Anything that makes only one side drop the endpoint will do it.

Three ways the active side gets there, all ending in a new bootstrap:

1. Any failed work completion on a slice. In Workers::asyncPollCq (mooncake-transfer-engine/tent/src/transport/rdma/workers.cpp), a failed completion calls disableEndpoint() in both branches, including the one where the retry count is still under max_retry_count and the slice is resubmitted. So a single transient error is enough, it does not have to be a persistent fault. This is the fluctuation-shaped trigger: retry-exceeded from a congested or lossy fabric, a link blip, or a peer that restarted.
2. A software transfer timeout on an inflight slice, same function.
3. Endpoint store eviction, with no network problem at all. Once a context holds endpoint_store_cap endpoints, SIEVEEndpointStore::evictOne() retires a live victim. Only the evicting side drops it, so the peer keeps its EP_READY endpoint and the next transfer in that direction bootstraps with new QPs. The shipped config/transfer-engine.json sets 256, and MC_MAX_EP_PER_CTX overrides it. This one gets more likely the more peers a single NIC talks to, which is why it tends to show up on larger clusters rather than under load.

The reason it looked permanent rather than like a blip: before this change the reject left the endpoint in EP_READY, so the caller's terminal-state branch never removed it from the store, and every later bootstrap from that peer hit the same reject. One transient event turned into a standing wedge for that peer pair until a restart. That matches the message repeating in your logs.

If you can still see the affected run, two things would tell us which trigger you hit: roughly how many peer endpoints that context held versus your MC_MAX_EP_PER_CTX, and whether the message repeated for a single peer or for many at once. Eviction pressure usually hits several peers over time, a fabric error usually starts with one. I have no RDMA hardware to reproduce it on, so that part is read off the code paths and your logs rather than measured.

@guptaishaan Thank you very much for your explanation. I understand now.