Simplify OutstandingData state machine#100
Merged
boivie merged 1 commit intowebrtc:mainfrom Mar 3, 2026
Merged
Conversation
boivie
force-pushed
the
feature-simplify-outstanding-data
branch
from
2bf4897 to
7958d9f
Compare
DanilChapovalov
previously approved these changes
Mar 2, 2026
boivie
force-pushed
the
feature-simplify-outstanding-data
branch
from
7958d9f to
4c7e7aa
Compare
DanilChapovalov
previously approved these changes
Mar 3, 2026
This change replaces the separate Lifecycle and AckState enums with a single ItemState enum. Previously, the use of two independent state machines allowed for invalid combinations and required storing variables like time_sent and nack_count on all items, even when they were not applicable. The new design makes use of Rust's algebraic data types to scope these variables strictly to the states that need them. This also fixes a bug where measure_rtt could return an incorrect and too large duration for fragments that were previously gap-acked. By removing the time_sent field from the new Acked state, the code now explicitly prevents RTT calculations on chunks that have already been acknowledged, avoiding false measurements when a gap-acked chunk is later cumulatively acked. The new states closely follow the chunk lifecycle: in flight, reported missing, queued for retransmission, and three terminal variants: Acked, Abandoned, and AbandonedAndAcked. This separation ensures that the "abandoned" status is preserved even if a SACK eventually arrives for that chunk. When the cumulative acknowledgment point advances and the chunk is removed, its lifecycle ID is correctly reported as abandoned rather than falsely reported as a successful delivery.
boivie
force-pushed
the
feature-simplify-outstanding-data
branch
from
4c7e7aa to
0add31a
Compare
DanilChapovalov
approved these changes
Mar 3, 2026
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
2 participants
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.
This change replaces the separate Lifecycle and AckState enums with a single ItemState enum. Previously, the use of two independent state machines allowed for invalid combinations and required storing variables like time_sent and nack_count on all items, even when they were not applicable. The new design makes use of Rust's algebraic data types to scope these variables strictly to the states that need them.
This also fixes a bug where measure_rtt could return an incorrect and too large duration for fragments that were previously gap-acked. By removing the time_sent field from the new Acked state, the code now explicitly prevents RTT calculations on chunks that have already been acknowledged, avoiding false measurements when a gap-acked chunk is later cumulatively acked.
The new states closely follow the chunk lifecycle: in flight, reported missing, queued for retransmission, acked, and abandoned. The only exception to a strict progression is the Acked state, which includes a "was_abandoned" boolean flag. This field is necessary because an item can be logically abandoned by the sender, for example due to a message timeout or reaching the maximum number of retransmissions, while still being successfully delivered to and acknowledged by the receiver.
Preserving the abandoned status within the Acked state ensures that when the cumulative acknowledgment eventually covers the chunk and it is removed from the queue, its lifecycle ID is correctly added to the abandoned lifecycle IDs list rather than being falsely reported as successfully delivered.