[TransferEngine] Rebuild single Jetty on ACK timeout (status=9) - #33
Open
Connor-Matthew wants to merge 13 commits into
Open
[TransferEngine] Rebuild single Jetty on ACK timeout (status=9)#33Connor-Matthew wants to merge 13 commits into
Connor-Matthew wants to merge 13 commits into
Conversation
Avoid deleting the whole endpoint after a transient ACK timeout by draining the faulty Jetty, recreating it locally, and rebinding to the existing peer id. Falls back to deleteEndpoint on flush-done timeout or rebuild failure. Co-authored-by: Cursor <cursoragent@cursor.com>
github-actions
Bot
added
documentation
Mark the jetty rebuild design doc as Sphinx orphan and apply clang-format to the UrmaEndpoint changes so PR checks pass. Co-authored-by: Cursor <cursoragent@cursor.com>
Defer endpoint deletion until after poll depth accounting to avoid UAF, deliver flush completions during rebuild, and isolate stale CQEs with per-slot jetty epochs. Align disconnect teardown with ERROR flush fence constraints and document the updated rebuild accounting model. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Extend the mock URMA provider so CI can drive the status=9 rebuild path without real hardware: record each posted WR's jetty local id, script the next poll status (URMA_CR_ACK_TIMEOUT_ERR), inject a FLUSH_ERR_DONE fence, return WR_FLUSH_ERR from urma_flush_jetty, and optionally fail the next urma_create_jetty. Add gtest cases TC-1..TC-3 covering the happy-path rebuild, flush-CR delivery, and stale-epoch drop, and wire them into the tent-ci (ub-mock) job as a scoped ctest. Co-authored-by: Cursor <cursoragent@cursor.com>
TC-1 segfaulted on CI because postOneSlice left slice->ub.r_seg null while processWrCompletion's failure log dereferences it. Import a real mock target segment in the fixture and attach it to each posted slice. Co-authored-by: Cursor <cursoragent@cursor.com>
…h test TC-2 failed on CI because the single-JFC/single-slot fixture drained both posted WRs in the first poll (s1 failed with status 9, s2 succeeded), leaving nothing for the rebuild flush loop to deliver. Add a withhold hook to the mock so a posted WR stays outstanding until urma_flush_jetty completes it, and post s2 with that hook so the flush path is genuinely exercised. Co-authored-by: Cursor <cursoragent@cursor.com>
The rebuild flush loop delivers each outstanding WR through processWrCompletion and increments poll's resolved_wr_count, so delivering s2 via flush yields resolved == 1 (the fence marker itself is not counted). Fix the test to assert that instead of 0. Co-authored-by: Cursor <cursoragent@cursor.com>
Rebuild failure (urma_endpoint.cpp, urma_endpoint.h): when urma_delete_jetty fails mid-rebuild, keep the old jetty handle and mark the slot REBUILDING_FAILED instead of nulling it, so deconstruct can retry the delete rather than leak the handle. The new state is never selected for posting. Drain timeout (urma_endpoint.cpp): when the flush-done fence never arrives, checkDrainTimeout now flushes the jetty and delivers each residual WR through processWrCompletion (endpoint still alive) so stuck slices are failed/retried and depth accounting returns to zero before the endpoint is deferred for deletion. disconnectUnlocked keeps its drain-only flush (endpoint is being torn down), with a comment clarifying why CRs are not delivered there. Add TC-4 (rebuild failure keeps the handle, deferred delete) and TC-5 (drain timeout delivers residual WRs) to urma_jetty_rebuild_test. Co-authored-by: Cursor <cursoragent@cursor.com>
TC-4 was driving rebuild failure via fail_next_create_jetty, which fails in recreateJettyUnlocked after the old jetty was already deleted — so the handle is legitimately null and the slot stays REBUILDING. The handle-preservation fix targets urma_delete_jetty failure instead. Add a fail_next_delete_jetty hook to the mock and point TC-4 at that branch, which is where the old handle must be kept (REBUILDING_FAILED) for deconstruct to retry. Co-authored-by: Cursor <cursoragent@cursor.com>
Explain how the ub-mock CI drives the status=9 rebuild path: the mock-layer injection design, the error9 trigger sequence, the TC-1..TC-5 to fix mapping, and the verification boundaries (what CI proves vs. what needs real hardware), plus explicit out-of-scope items (token registry, per-jetty injection, peer-id reuse assumption). Cross-link from the plan and rebuild-plan docs. Co-authored-by: Cursor <cursoragent@cursor.com>
checkJettyDrainTimeouts() previously ran inside UrmaContext::poll() and appended recovered slices to the failed vector without counting them in the return value. performPoll computes num_success = resolved - failed, so a single drain timeout made num_success negative; a negative success_nr_polls permanently disables the RNIC-dead protection (failed_nr_polls > 32 && !success_nr_polls). Move recovery out of poll(): checkJettyDrainTimeouts is now a virtual method on UbContext (default no-op, override in UrmaContext), invoked once per worker round after all poll() calls. Its failures are counted via the same handle_failed path but never subtracted from poll()'s resolved count. Recovery still runs before jetty_depth_set accounting and deferred endpoint deletes. Add TC-6 (DrainTimeoutRecoveryExcludedFromPoll): poll() must deliver nothing for an already-timed-out draining jetty; the separate recovery pass delivers the residual WR.
TC-3 (StaleEpochCompletionDropped) previously only asserted fixture state (slice->ub.jetty_epoch != current epoch) without ever delivering a stale completion to poll, so the epoch-drop guard that prevents double-completing a slice had no coverage. Rewrite it to exercise the real path: post a withheld WR so the rebuild's flush loop does not consume it, drive the status=9 rebuild, release the WR, and let poll deliver its SUCCESS CQE stamped with the pre-rebuild epoch. Assert the drop: resolved == 0, no failed delivery, no depth accounting, slice left POSTED and owned by the test. Add mock_urma_unwithhold_all() to the mock control API: urma_poll_jfc permanently skips withheld WRs, so a withheld entry that survives a rebuild could otherwise never be polled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
status=9（
URMA_CR_ACK_TIMEOUT_ERR）时，不再很快把整条 endpoint 删掉，而是对本端故障 Jetty 做：modify(ERROR)排空FLUSH_ERR_DONEflush→ unbind/unimport → delete → create → 本端 re-import/rebind 原对端 jetty id纯本地恢复，不引入对端同步协议。flush-done 超时（默认 3s）或重建失败则回退
deleteEndpointByPtr。在此之上，本 PR 还包含两个安全性加固与一套 mock 注入 CI 测试：
生产加固
urma_delete_jetty失败时不再丢 handle，保留旧指针 + 新增REBUILDING_FAILED态，deconstruct可重试 delete，避免泄漏。Mock 注入 CI 测试（无 URMA 硬件即可覆盖 status=9 路径）
mock_urma.cpp增强：记录每个 WR 的local_id，支持脚本化注入 poll status=9 / FLUSH_ERR_DONE fence / flush 返回 WR_FLUSH_ERR / create·delete 失败 / 滞留 WR。mock_urma_test_ctrl.h测试控制接口（注入点只在 mock 层，生产代码零改动、不读任何环境变量）。urma_jetty_rebuild_test（TC-1～TC-5），挂到tent-ci (ub-mock)的 scoped ctest。设计说明：
docs/source/design/transfer-engine/jetty-single-rebuild-plan.mddocs/source/design/transfer-engine/jetty-rebuild-mock-test-plan.mddocs/source/design/transfer-engine/jetty-rebuild-mock-test-verification.mddocs/source/design/transfer-engine/jetty-ack-timeout-rebuild.mdModule
mooncake-transfer-engine)mooncake-store)mooncake-reshard)mooncake-ep)mooncake-pg)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-common)mooncake-rl)Type of Change
How Has This Been Tested?
CI（
tent-ci (ub-mock)，无 URMA 硬件，确定性注入 status=9）全绿：AckTimeoutTriggersRebuildHappyPath：status=9 → DRAINING → FLUSH_ERR_DONE → rebuild → 回 ACTIVE、epoch+1、新旧 jetty id 不同FlushCompletionsDeliveredOnRebuild：rebuild 内 flush 把残留 WR 交付出来（计数正确）StaleEpochCompletionDropped：旧代 CQE 被 epoch 守卫丢弃RebuildFailureKeepsJettyHandle：delete 失败保留 handle +REBUILDING_FAILED+ 延迟删除DrainTimeoutDeliversResidualWriters：drain 超时收敛残留 WR（不悬挂、计数归零）仍待真机验证（mock 无法覆盖 provider 语义/时序）：
Checklist
./scripts/code_format.sh（CICheck code format绿）