-
Notifications
You must be signed in to change notification settings - Fork 204
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
Replace alarm with timer #1559
Replace alarm with timer #1559
Conversation
Fixes #1383
draft-ietf-quic-recovery.md
Outdated
@@ -111,8 +111,8 @@ of frames contained in a packet affect recovery and congestion control logic: | |||
to as "retransmittable" below. | |||
|
|||
* Long header packets that contain CRYPTO frames are critical to the | |||
performance of the QUIC handshake and use shorter timers for acknowledgement | |||
and retransmission. | |||
performance of the QUIC handshake and use shorter timeouts for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo this change -- shorter timeouts not timers.
draft-ietf-quic-recovery.md
Outdated
When CRYPTO frames are sent, the sender SHOULD set a timer for the handshake | ||
timeout period. Upon timeout, the sender MUST retransmit all unacknowledged | ||
CRYPTO data by calling RetransmitAllUnackedHandshakeData(). On each | ||
consecutive firing of the handshake timer without receiving an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/firing/expiration/
draft-ietf-quic-recovery.md
Outdated
To reduce latency, it is RECOMMENDED that the sender set and allow the TLP alarm | ||
to fire twice before setting an RTO alarm. In other words, when the TLP alarm | ||
To reduce latency, it is RECOMMENDED that the sender set and allow the TLP timer | ||
to fire twice before setting an RTO timer. In other words, when the TLP timer | ||
fires the first time, a TLP packet is sent, and it is RECOMMENDED that the TLP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should just do s/fires/expires/ through the doc
draft-ietf-quic-recovery.md
Outdated
When the last TLP packet is sent, an alarm is scheduled for the RTO period. When | ||
this alarm fires, the sender sends two packets, to evoke acknowledgements from | ||
the receiver, and restarts the RTO alarm. | ||
When the last TLP packet is sent, a timer is scheduled for the RTO period. When |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/a timer is scheduled/a timer is set/
draft-ietf-quic-recovery.md
Outdated
if (smoothed_rtt == 0): | ||
alarm_duration = 2 * kInitialRtt | ||
timer_duration = 2 * kInitialRtt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/timer_duration/timeout/
Thanks Jana, this PR got a bit larger, PTAL |
I don't know why circle is still showing breakage with the old qpack draft. I'll merge this. |
@ianswett I think you missed the transport doc (migration section) |
Should be done now in PR #1590 |
You're right, thanks for the PR! |
Fixes #1383