Skip to content

Commit

Permalink
Some config values make test less flaky, updating
Browse files Browse the repository at this point in the history
Longer idle timeout has obvious effect of reducing connection failures
due to idle timeout. This idle timeout doesn't seem to be specified
anywhere, we are just doing it. 10s seemed reasonable, but when fighting
against tokio which is not evenly distributing tasks, it gets less
reasonable.

The retry timeout should be small enough that if it maxes out, there are
a few chances at reattempts before the idle timeout. Setting it to /4 of
the idle timeout means we should get at least 3 attempts (the 4th will
race with the idle).

Was seeing regular connection failures, even with the stress limiter. By
bumping up to 5 attempts, it seemed to effectively disappear. I don't
have a strong model for how often SYNs are dropped in this test. To get
an estimate of the allowed failure rate, making a silly-ish assumption
that each attempt has an independently random chance of failing, then to
have the test pass 99.9% of the time, we need each connection attempt to
succeed at least 75% of the time.
  • Loading branch information
carver committed Jun 10, 2023
1 parent 86d7392 commit 4244f87
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ pub struct ConnectionConfig {

impl Default for ConnectionConfig {
fn default() -> Self {
let max_idle_timeout = Duration::from_secs(10);
let max_idle_timeout = Duration::from_secs(30);
Self {
max_conn_attempts: 3,
max_conn_attempts: 5,
max_idle_timeout,
max_packet_size: congestion::DEFAULT_MAX_PACKET_SIZE_BYTES as u16,
initial_timeout: congestion::DEFAULT_INITIAL_TIMEOUT,
min_timeout: congestion::DEFAULT_MIN_TIMEOUT,
max_timeout: max_idle_timeout,
max_timeout: max_idle_timeout / 4,
target_delay: Duration::from_micros(congestion::DEFAULT_TARGET_MICROS.into()),
}
}
Expand Down

0 comments on commit 4244f87

Please sign in to comment.