Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: set an aggressive draining timeout when closed before established #1754

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LPardue
Copy link
Contributor

@LPardue LPardue commented Apr 8, 2024

It is possible that a server could close a connection without ever sending
an ACK frame. For example, when immeditaly closing a connection due to a
TLS-level reason.

Previously, we always would set the draining timer the same way when processing
CONNECTION_CLOSE frames, basing it on 3*PTO. In the situation where there
is no accurate RTT estimate, the PTO is based on the default initial RTT,
which comes out at 999ms. This leads to clients that honor the draining
period to have to wait 3 seconds for no real reason.

With this change, in the situation where a CONNECTION_CLOSE is received
before the connection is established, we'll just immediately timeout.

@LPardue LPardue requested a review from a team as a code owner April 8, 2024 20:46
It is possible that a server could close a connection without ever sending
an ACK frame. For example, when immeditaly closing a connection due to a
TLS-level reason.

Previously, we always would set the draining timer the same way when processing
CONNECTION_CLOSE frames, basing it on 3*PTO. In the situation where there
is no accurate RTT estimate, the PTO is based on the default initial RTT,
which comes out at 999ms. This leads to clients that honor the draining
period to have to wait 3 seconds for no real reason.

With this change, in the situation where a CONNECTION_CLOSE is received
before the connection is established, we'll just immediately timeout.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants