[TransferEngine] Rebuild single Jetty on ACK timeout (status=9) - #33
Open
Connor-Matthew wants to merge 10 commits into
Open
[TransferEngine] Rebuild single Jetty on ACK timeout (status=9)#33Connor-Matthew wants to merge 10 commits into
Connor-Matthew wants to merge 10 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>
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绿）