Re: [quicwg/base-drafts] MinRTT for the congestion control context (#2919)

[Jana Iyengar <[e-mail]>]

janaiyengar commented on this pull request.



> @@ -371,16 +371,23 @@ retain sufficient history is an open research question.
 
 ## Estimating min_rtt {#min-rtt}
 
-min_rtt is the minimum RTT observed over the lifetime of the connection.
-min_rtt is set to the latest_rtt on the first sample in a connection, and to the
-lesser of min_rtt and latest_rtt on subsequent samples.
+min_rtt is the minimum RTT observed for a given congestion control context.

min_rtt is only used for loss recovery in this doc. I think it's more appropriate to say RTT estimator context in general. 

> @@ -371,16 +371,23 @@ retain sufficient history is an open research question.
 
 ## Estimating min_rtt {#min-rtt}
 
-min_rtt is the minimum RTT observed over the lifetime of the connection.
-min_rtt is set to the latest_rtt on the first sample in a connection, and to the
-lesser of min_rtt and latest_rtt on subsequent samples.
+min_rtt is the minimum RTT observed for a given congestion control context.
+min_rtt is set to the latest_rtt on the first RTT sample, and to the lesser
+of min_rtt and latest_rtt on subsequent samples.  min_rtt is used to reject
+implausible RTT samples, but is not used directly in congestion control or

I would drop the rest of the text after "samples" and perhaps add "which would otherwise cause the smoothed_rtt estimate to be incorrectly low.". I understand what you're trying to say, but I think saying that it is not used directly in loss recovery makes it a bit confusing I think. It is used, and I think people will have different reads on what's indirect and what's 

>  
 An endpoint uses only locally observed times in computing the min_rtt and does
 not adjust for host delays reported by the peer ({{host-delay}}).  Doing so
 allows the endpoint to set a lower bound for the smoothed_rtt based entirely on
 what it observes (see {{smoothed-rtt}}), and limits potential underestimation
 due to erroneously-reported delays by the peer.
 
+The RTT for the path may change over time.  If the real path RTT decreases,

```suggestion
The RTT for a path may change over time.  If a path's actual RTT decreases,
```

>  
 An endpoint uses only locally observed times in computing the min_rtt and does
 not adjust for host delays reported by the peer ({{host-delay}}).  Doing so
 allows the endpoint to set a lower bound for the smoothed_rtt based entirely on
 what it observes (see {{smoothed-rtt}}), and limits potential underestimation
 due to erroneously-reported delays by the peer.
 
+The RTT for the path may change over time.  If the real path RTT decreases,
+it should be observed quickly.  If the real path RTT increases, it will not

```suggestion
the min_rtt will adapt immediately on the first low sample.  If the path's actual RTT increases,
```

>  
 An endpoint uses only locally observed times in computing the min_rtt and does
 not adjust for host delays reported by the peer ({{host-delay}}).  Doing so
 allows the endpoint to set a lower bound for the smoothed_rtt based entirely on
 what it observes (see {{smoothed-rtt}}), and limits potential underestimation
 due to erroneously-reported delays by the peer.
 
+The RTT for the path may change over time.  If the real path RTT decreases,
+it should be observed quickly.  If the real path RTT increases, it will not
+be observed, but the impact is limited to allowing some RTT samples that are

```suggestion
the min_rtt will not adapt to it, potentially causing future RTT samples that are
```

> @@ -410,11 +417,11 @@ endpoint:
   min_rtt.  This limits the underestimation that a misreporting peer can cause
   to the smoothed_rtt.
 
-On the first RTT sample in a connection, the smoothed_rtt is set to the
-latest_rtt.
+On the first RTT sample in a congestion control context, the smoothed_rtt is

RTT context

>  
 smoothed_rtt and rttvar are computed as follows, similar to {{?RFC6298}}.  On
-the first RTT sample in a connection:
+the first RTT sample in a congestion control context:

RTT context

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2919#pullrequestreview-264552498