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

loss_detection_timer.update in recovery #3969

Closed
kazu-yamamoto opened this issue Jul 29, 2020 · 5 comments
Closed

loss_detection_timer.update in recovery #3969

kazu-yamamoto opened this issue Jul 29, 2020 · 5 comments
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.

Comments

@kazu-yamamoto
Copy link
Contributor

When a client is downloading a large contents from a server, the server sends STREAM repeatedly and the client sends ACK repeatedly. In this situation, time_of_last_ack_eliciting_packet is not updated in the client side. So, timeout - now()in loss_detection_timer.update can be a non-positive timeout value.

It seems to me that it is unclear how loss_detection_timer.update should behave when timeout value is non-positive.

@marten-seemann
Copy link
Contributor

SetLossDetectionTimer only sets a timer when there are ack-eliciting packets in flight. This is not the case on the client side in the scenario you describe, so you wouldn't set a timer in the first place.

@kazu-yamamoto
Copy link
Contributor Author

The same as #3967. Let's close. Thanks.

@kazu-yamamoto
Copy link
Contributor Author

I understand why timeout - now() can be negative. Suppose PTO for Initial is set. SetLossDetectionTimer is called even in Handshake but GetPtoTimeAndSpace returns Initial in this case. So, loss_detection_timer.update called in SetLossDetectionTimer adjusts the previous timer. It depends on timing (probably in thread programming), the value can be negative.

When I modify my code so that SetLossDetectionTimer takes packet space and add some logic, I don't see warning for minus anymore. I don't have a strong opinion to modify the current pseudocode but I'm pointing out this for record.

@kazu-yamamoto kazu-yamamoto reopened this Aug 4, 2020
@ianswett ianswett added the editorial An issue that does not affect the design of the protocol; does not require consensus. label Aug 4, 2020
@larseggert
Copy link
Member

Does anyone have a proposal for a change here, or is this a close-with-no-action?

@kazu-yamamoto
Copy link
Contributor Author

It seems to me that people want the current simple pseudocode rather than complex code with corner cases removed. So, let's close this hoping that new programmers would refer to this issue when necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

No branches or pull requests

5 participants