[TTThanos]

Description

Problem
Issue #3523 defect C. A TENT RDMA context that was paused by IBV_EVENT_PORT_ERR only ever leaves DEVICE_PAUSED from one place: the IBV_EVENT_PORT_ACTIVE branch of Workers::applyContextEvent(). Nothing revalidates the port afterwards, so if that one event is missed the context stays paused for the rest of the process' life, Workers::getEndpoint() rejects every transfer on that NIC, and no log line ever explains why.

That event is easy to miss. joinNonblockingPollList() registers the async fd with EPOLLIN | EPOLLET, while monitorThread() calls epoll_wait(..., maxevents=1, 100) and handleContextEvents() consumed exactly one ibv_async_event per wakeup. Under edge-triggered semantics epoll does not report readiness again until a new event arrives, so anything queued behind the event that woke us is stranded. Bursts are routine — IBV_EVENT_COMM_EST fires once per connection — so a PORT_ACTIVE sitting behind one waits for an unrelated future event that may never come. The classic engine hit exactly this and fixed it in WorkerPool::doProcessContextEvents(); TENT still had the one-event-per-wakeup read.

Fix
Workers::handleContextEvents() now drains the queue: loop until EAGAIN/EWOULDBLOCK, retry on EINTR, and propagate a genuine read error instead of spinning on a broken fd. Mirrors the classic engine.
Workers::resumePausedContexts() is a safety net on monitorThread()'s existing 1 Hz tick: for contexts in DEVICE_PAUSED it reads the port state and, when the hardware reports IBV_PORT_ACTIVE, runs the same recovery the event path runs. Intermediate states (INIT/ARMED/ACTIVE_DEFER) are left alone and re-checked on the next tick.
The recovery actions (resume() + re-seed bandwidth + setDeviceAvailable) are factored into Workers::activateContext() so the event path and the poll cannot drift apart.
New RdmaContext::queryPortState() reads the port state without touching the cached speed/width, because refreshLinkSpeed() diffs against them — refreshing there would hide a renegotiated link rate from the selector.

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?

Tests
New tent/tests/rdma_async_event_drain_test.cpp (5 cases), scripting the event source with -Wl,--wrap=ibv_get_async_event, so it needs no RDMA device: full drain of a burst, a handled event queued behind unhandled ones, empty queue is not an error, EINTR is retried, a real read error aborts. All 5 fail on main and pass with this change.
tent/tests/rdma_transport_test.cpp: RecoveryPollLeavesInertContextsAlone (the poll must not hand an inert slot back to the selector), QueryOnInertContextIsRejected, and PollActivatesPausedContextWithLivePort — the regression itself, on a real NIC (skipped when no device is present).

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)

I use AI tools to help summarise the PR content, assist and review on my code.

[he-yufeng]

Read the full diff and chased the invariants I was worried about; this is a careful fix for a real defect class.

Verified on my read:

• The starvation mechanism is real: joinNonblockingPollList registers the async fd EPOLLET and the old loop consumed one event per wakeup, so anything queued behind it only surfaces when a new event arrives. COMM_EST bursts make that routine, and a stranded PORT_ACTIVE is a silent permanent outage for that NIC.
• The drain loop gets the errno handling right for a non-blocking fd (EAGAIN/EWOULDBLOCK drains, EINTR retries, anything else propagates).
• Stale-event resurrection is not possible: the queue is FIFO per device, so a PORT_ACTIVE queued before a later PORT_ERR is always processed in that order, and resume() is CAS-guarded from DEVICE_PAUSED, so a double activation is a no-op rather than a crash or a state rewrite.
• Poll and event handling both run on monitorThread, so the 1 Hz net and the drain cannot race each other.
• queryPortState leaving the cached speed/width alone is the right call; refreshLinkSpeed diffs against the cache to detect renegotiation, and refreshing at query time would hide exactly that.
• The wrap-based test is honestly constructed: no device needed, the Apple linker guard is correct, and scripting the drained errno covers EAGAIN/EINTR/real-error plus handled/unhandled event mix.

One operational nit, not a blocker: every drained event still logs at WARNING, and before this change most COMM_EST events were never dequeued at all (which was the bug, but also meant they were never logged). After the drain, a connection burst produces a WARNING line per event, so on a busy cluster the log will get loud fast. Worth demoting COMM_EST to INFO or VLOG in a follow-up.

Scope note: read-only review against the PR head; I did not build or run the tests here. Leaving the formal approve to a second pair of eyes since this is the RDMA context path.

[alogfans]

LGTM. However I advise to avoid mass logs from high-frequent async events such as IBV_EVENT_COMM_EST.

[catyans]

LGTM. The edge-triggered async-event queue is now drained with the correct EAGAIN/EINTR handling, and the recovery poll only reactivates paused contexts whose port is fully active while preserving link-speed re-seeding. The regression coverage matches these paths.

The remaining MasterServiceSSDSnapshotTest.EvictObject failure appears unrelated to this RDMA-only change; please rerun it before merge.

[catyans]

LGTM. Re-reviewed the current head; the new commit only demotes the high-frequency IBV_EVENT_COMM_EST message to VLOG(1), while preserving event handling and acknowledgement behavior.

The PR is currently conflicting with the base branch and only the triage check is present, so please resolve the conflict and run the full CI before merge.

[alogfans]

It seems OK but you may check your code format.

[catyans]

issue: the merge resolution dropped the closing brace for RdmaContext::queryPortState() after return 0;, so queryEffectiveSpeed() now starts inside that function and the current head cannot compile. Please restore the } before the next member-function definition.

[catyans]

Thanks, this addresses my concern. LGTM. The current head restores the missing closing brace after queryPortState(); the remaining incremental changes are formatting-only, and the async-event drain/recovery logic is unchanged. CI is green.