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

kGranularity and max_ack_delay possibly should be added after exponential backoff #2393

Closed
ianswett opened this issue Jan 30, 2019 · 4 comments
Labels
-recovery design An issue that affects the design of the protocol; resolution requires consensus.

Comments

@ianswett
Copy link
Contributor

Right now, exponential backoff occurs last. But it's reasonable to consider adding/maxing them at the end after exponential backoff.

On the one hand, the current pseudocode provides true exponential backoff. On the other hand, it might be overly conservative in some cases. This only applies on subsequent PTOs, so maybe this is an academic question.

I'm opening this in the interest in discussing design issues on issues and not on PRs, but I'm also fine with it being closed as not being important and/or needing more information.

This came up in #2365

@ianswett ianswett added design An issue that affects the design of the protocol; resolution requires consensus. -recovery labels Jan 30, 2019
@nibanks
Copy link
Member

nibanks commented Jan 30, 2019

I also agree kGranularity and max_ack_delay should be applied after the exponential; especially kGranularity which is only there to account for clocks/timers that have a minimum time you can set. No need to compound that inaccuracy inside the exponent.

@ianswett
Copy link
Contributor Author

In an effort to align with TCP, kGranularity is now considered here:
PTO = smoothed_rtt + max(4*rttvar, kGranularity) + max_ack_delay

In TCP, is the exponential backoff done before kGranularity is considered?

@pravb
Copy link

pravb commented Mar 25, 2019

TCP does not use kGranularity because minRTO already accounts for it indirectly i.e. minRTO values have never been allowed to be set lower than the timer granularity. But yes in TCP minRTO is included in the exponential backoff so it makes sense to keep the equation as is. I think we are effectively using kGranularity as minRTO here.

In any case I think max_ack_delay is going to be the dominant value here in practice unless delayed ACKs are turned off. I wonder if another option is this:
PTO = smoothed_rtt + max(4*rttvar) + max(max_ack_delay, kGranularity)

@ianswett
Copy link
Contributor Author

As discussed in Prague, the feeling is the current formula is safe, correct, and follows RFC6298.

This formula was recently adjusted as a result of #2472

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

No branches or pull requests

3 participants