-
Notifications
You must be signed in to change notification settings - Fork 2k
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
TCP Margin calcuation for subscription liveness check. #12009
Comments
Also note the maximum idle timeout computation in #11795 is not compliant with the current spec, which has jitter and exponential backoff. |
copy from #11795, As an aside, this PR's computation for maximum idle-state MRP timeout is not spec compliant. |
we need to discuss here for the suitable timeout value |
The suggestion in #12009 (comment) might be pretty reasonable.... |
@bzbarsky-apple @yunhanw-google - does this need to be fixed for 1.0? |
Going to redirect to @msandstedt |
I feel like the code @yunhanw-google has in place is pretty reasonable for 1.0, and particularly so if TCP is out of scope. This is how we compute the liveness timeout:
And this is GetAckTimeout:
So because we don't have a good way to know link conditions for TCP, we are just adding 30-seconds margin. But I don't believe we are going to be supporting TCP in 1.0. |
Agree, thanks @msandstedt . Moving to post V1.0 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
@pidarped You might be interested in this one... |
TCP isn't instant though. RTO * retries can be hundreds of seconds if a link exhibits a high round-trip time. Even a single lost packet could lead to a breakage for TCP here, whereas this code might accommodate such a thing for UDP.
I don't see that it makes sense to branch like this PR attempts to. In this context, the only thing that is special about UDP+MRP as compared to TCP is that we have immediate access to retry parameters with MRP, whereas these are likely obfuscated for TCP. But they still absolutely exist.
Originally posted by @msandstedt in #11795 (comment)
The text was updated successfully, but these errors were encountered: