Retransmit state transfers before the first progress ACK - #12
Open
Unchiga wants to merge 1 commit into
Open
Conversation
…its first ACK The sender's ACK-timeout path (rewind to peer_ack, resend the window) only armed once an ACK had made progress. Before that there was no timer at all, so a first burst that lost chunk 0, or whose chunks overtook BEGIN through reordering, left peer_ack at 0 with nothing ever resent: the receiver sat in receive at 0/N, the sender logged "0 acked" every half second, and the app gave up on the stall timeout. Reproduced with the link simulator at RNET_SIM_LATENCY_MS=35 RNET_SIM_JITTER_MS=15 (the guest memory-card upload never got past 0/131076) and with RNET_SIM_LOSS_PCT=5. The timer now runs from the transfer's start until the first progress ACK takes over; both runs complete. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YnTp36AZz1iswG9X2fadT
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.
A state transfer can stall forever at
0/N ackedwhen the first chunk is lost or chunks overtake the initial BEGIN. The retransmit timeout was armed only after a progress ACK, so those transfers never resent their first window.Start the timeout from the transfer start until a progress ACK arrives. Keep the existing rewind-to-contiguous-ACK and timeout pacing behavior.
Validation: reproduced in the Forbidden Memories loopback harness with 35 ms latency and 15 ms jitter. A deterministic local test against current upstream 46ef6ed drops either the first STATE_CHUNK or BEGIN, uses clocks starting at 0 and 1000 ms, and requires both peers to receive the identical 100000-byte payload. It fails without this change and passes with it. That additional test is committed locally but is not included in this PR because this work is under a no-push constraint.
The existing upstream rollback_episode_test has four failures both with and without this fix; the other 15 local CTest cases, including the new loss test, pass.