fix(rtc_engine): keep a retry window for reliable resume replay - #1341
Open
tarsyang wants to merge 1 commit into
Open
fix(rtc_engine): keep a retry window for reliable resume replay#1341tarsyang wants to merge 1 commit into
tarsyang wants to merge 1 commit into
Conversation
TxQueue::trim() takes a target size to keep, but the send loop passed the byte count flushed by the current OnBufferedAmountChange event, so the retry queue retained roughly the last flushed burst (typically one packet). A resume that needed anything older replayed nothing and logged "Wrong packet sequence while retrying: ... packets missing", permanently losing reliable packets that the retry mechanism exists to recover. Trim to the flushed bytes plus a floor of 1.25x the low threshold instead. This restores the retry-buffer floor from client-sdk-swift#737, which this mechanism implements (livekit#688): the retained window must cover at least the full backpressure amount, since everything below the send-gate threshold can be sitting undelivered in the transport when a connection dies.
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.
Summary
data_channel_taskkeeps recently sent reliable packets in aTxQueueso_enqueue_for_retry_fromcan replay them after a resume, starting from the server-provided last received sequence.TxQueue::trim(target)discards from the front until the retained size is at or belowtarget, and the send loop passessent(the byte count flushed by the currentOnBufferedAmountChangeevent) as that target. The queue therefore retains roughly the last flushed burst, typically one packet: a resume that needs anything older replays nothing, logs "Wrong packet sequence while retrying: ... packets missing", and those reliable packets are permanently lost even though the retry mechanism exists to recover exactly them.This mechanism was introduced by #688 as an implementation of the reliability improvements from livekit/client-sdk-js#1546 and livekit/client-sdk-swift#737. The Swift implementation sizes the same buffer as the flushed bytes plus a floor:
(
RetryBuffer.trim(toAmount:)keepstoAmount + minAmount.) The floor did not carry over. Its rationale applies unchanged here: everything below the send-gate threshold can be sitting in the transport undelivered when a connection dies, so the retained window must cover at least that amount.The fix trims to
sent + threshold + threshold / 4, using the live threshold: the 2 MiB default gives the same 2.5 MiB floor as Swift's constant, and the window scales if the threshold is adjusted at runtime.Verification
cargo test -p livekit --lib(76 tests) and the data channel e2e suite against a locallivekit-server --dev(includingtest_reliable_retry) pass;cargo fmt --checkis clean. Note that the existing e2e cannot discriminate the window size:SignalReconnectkeeps the SCTP association alive, so replay never needs more than the last burst there. The evidence for the fix is the reference semantics above andTxQueue::trim's documented keep-at-most contract.