[Bugfix][Python] Preserve TENT failure decisions in synchronous transfers - #4228
Draft
jacklin78911-collab wants to merge 1 commit into
Draft
jacklin78911-collab wants to merge 1 commit into
jacklin78911-collab wants to merge 1 commit into
Conversation
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
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
When a TENT WRITE reaches the peer but its acknowledgement is lost, the Python synchronous wrapper submits the request again in a new batch. This discards TENT's non-replayable failure state: both the scalar and batch APIs can write twice and report success after the second attempt.
Let TENT own retry/failover by limiting the wrapper to one submission in TENT mode. Classic TE keeps its existing retry count. Add a loopback regression that forwards a real HP TCP WRITE to a real target, verifies its completion response, and then drops the first ACK. Run it in the CPU-only TENT CI leg.
Refs #3995 (the retry part of P1-sync-status). #4191 addresses asynchronous batch ownership; this change addresses synchronous re-submission. Cancellation handling and stale-handle eviction are outside this change. No new retry mechanism or configuration is added.
Module
mooncake-integration)Type of Change
How Has This Been Tested?
Built from main
2f13d400da4a3eb04d37263742866c1ce555d212, withWITH_TE=ON,USE_TENT=ON,USE_CUDA=OFF, andBUILD_UNIT_TESTS=ON.Baseline: both new subtests fail because the target receives two WRITEs after one ACK is dropped.
Fixed: both subtests pass; the target receives one WRITE, contains the committed payload, and the caller receives failure (
-1). All four Python/TENT CTest entries pass.Healthy scalar and batch READ/WRITE smoke checks pass for both classic TCP and TENT HP TCP (8 cases, with data verification).
Scoped pre-commit: C++ changed-line formatting, Python, YAML, spelling and whitespace pass.
cmake-formatalso rewrites an unrelated existing install block; that baseline formatting change is excluded. The added CMake test block is formatter-clean.Integration tests pass
Manual testing done as described above
Upstream CI on
3533c36f2: all four TENT matrix jobs pass, including the explicit Python TENT sync-failure test in the CUDA-off job. The overall Build & Test run is red because the separate Storefileread_worker_pool_testfails (215/216 general CTests pass): process thread count is 6 instead of 4, followed by the 1 s teardown-count timeout. This matches [CI] FilereadWorkerPoolTest.AcceptsTypedTrailingWhitespaceAndCaches is flaky on the 1s teardown window #4213; existing fixes [Bugfix][Store] Isolate fileread worker count assertions from runtime threads #4210 and [CI] Harden FilereadWorkerPool thread-count flake #4226 are still open. No Store test changes are included here. Rerunning failed upstream jobs was denied by GitHub (HTTP 403: repository admin rights required).Checklist
./scripts/code_format.shAI Assistance Disclosure