quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.76k stars 380 forks source link

ACK frequency tests are flaky due to randomized packet number skipping #1628

Closed Ralith closed 1 year ago

Ralith commented 1 year ago

PendingAcks::reordering_threshold defaults to 1. This causes an ACK to be sent immediately when a packet number is skipped:

https://github.com/quinn-rs/quinn/blob/8076ffe94d38813ce0220af9d3438e7bfb5e8429/quinn-proto/src/connection/spaces.rs#L607-L609

As of #1613, we can randomly skip packet numbers, especially early in a connection. This causes a few extra ACKs to be sent at unpredictable times. This causes tests which count the number of ACKs sent in an interval to fail. We should modify such tests to use a reordering threshold greater than 1 to prevent these extra ACKs from being sent, or perhaps expose the number of skipped packets to allow them to account for the induced ACKs.