Subject: [quicwg/base-drafts] Recovery RFCEditor #3: Why reset min_rtt (#4891)

From: ianswett <[email-masked]>
Date: Thu, 06 May 2021 07:50:08 -0700

3) [rfced] Does the following improve the readability of the paragraph?  

Current:
   Endpoints SHOULD set the min_rtt to the newest RTT sample after 
   persistent congestion is established. This is to allow a connection 
   to reset its estimate of min_rtt and smoothed_rtt after a 
   disruptive network event (Section 5.3), and because it is possible 
   that an increase in path delay resulted in persistent congestion 
   being incorrectly declared.

Perhaps:
   Endpoints SHOULD set the min_rtt to the newest RTT sample after 
   persistent congestion is established because it is possible that 
   an increase in path delay resulted in persistent congestion being 
   incorrectly declared. This also allows a connection to reset its 
   estimate of min_rtt and smoothed_rtt after a disruptive network 
   event (Section 5.3). 
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/4891

-- Commit Summary --

  * Recovery RFCEditor #3: Why reset min_rtt

-- File Changes --

    M rfc9002.md (10)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/4891.patch
https://github.com/quicwg/base-drafts/pull/4891.diff

-- 
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/4891

---

From: ianswett <[email-masked]>
Date: Thu, 06 May 2021 07:51:10 -0700

I find this easier to read, but I think it may be subtly different, so I put it into a separate PR.

-- 
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/4891#issuecomment-833585585

---

From: Mike Bishop <[email-masked]>
Date: Thu, 06 May 2021 07:57:44 -0700

@MikeBishop commented on this pull request.

It doesn't change the meaning, but it does feel like the first sentence is trying to span too many topics.  I like the reordering, but I'd split it out.

> +congestion is established because it is possible that an increase in path delay
+resulted in persistent congestion being incorrectly declared. This also allows

```suggestion
congestion is established. This helps when an increase in path delay
results in persistent congestion being incorrectly declared. This also allows
```

-- 
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/4891#pullrequestreview-653526048

---

From: ianswett <[email-masked]>
Date: Thu, 06 May 2021 11:05:39 -0700

@ianswett pushed 1 commit.

3f744380bdc5ae2a0987a288912cc88c0b5c5a4d  Update rfc9002.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4891/files/b9b6bee4b1eb729138c0033b49f5e67673215026..3f744380bdc5ae2a0987a288912cc88c0b5c5a4d

---

From: Martin Thomson <[email-masked]>
Date: Thu, 06 May 2021 17:04:48 -0700

@martinthomson commented on this pull request.

Careful with these, not to pick up bad stuff.

> @@ -345,11 +345,11 @@ path's actual RTT increases however, the min_rtt will not adapt to it, allowing
 future RTT samples that are smaller than the new RTT to be included in
 smoothed_rtt.
 
-Endpoints SHOULD set the min_rtt to the newest RTT sample after persistent
-congestion is established. This is to allow a connection to reset its estimate
-of min_rtt and smoothed_rtt ({{smoothed-rtt}}) after a disruptive network event,
-and because it is possible that an increase in path delay resulted in persistent
-congestion being incorrectly declared.
+Endpoints SHOULD set the min_rtt to the newest RTT sample after persistent

```suggestion
Endpoints SHOULD set the min_rtt to the newest RTT sample after persistent
```

> @@ -345,11 +345,11 @@ path's actual RTT increases however, the min_rtt will not adapt to it, allowing
 future RTT samples that are smaller than the new RTT to be included in
 smoothed_rtt.
 
-Endpoints SHOULD set the min_rtt to the newest RTT sample after persistent
-congestion is established. This is to allow a connection to reset its estimate
-of min_rtt and smoothed_rtt ({{smoothed-rtt}}) after a disruptive network event,
-and because it is possible that an increase in path delay resulted in persistent
-congestion being incorrectly declared.
+Endpoints SHOULD set the min_rtt to the newest RTT sample after persistent
+congestion is established. This helps when an increase in path delay
+results in persistent congestion being incorrectly declared. This also allows
+a connection to reset its estimate of min_rtt and smoothed_rtt after a
+disruptive network event (Section 5.3).

```suggestion
disruptive network event; see {{smoothed-rtt}}.
```

-- 
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/4891#pullrequestreview-653983966

---

From: ianswett <[email-masked]>
Date: Fri, 07 May 2021 06:23:39 -0700

f0e9ddfc4d6ae6551c2aee04be7716987539ec31  Update rfc9002.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4891/files/3f744380bdc5ae2a0987a288912cc88c0b5c5a4d..f0e9ddfc4d6ae6551c2aee04be7716987539ec31

---

From: ianswett <[email-masked]>
Date: Fri, 07 May 2021 06:24:06 -0700

c346701d64405ceaef330bb7e02471cdaf51d8a7  Update rfc9002.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4891/files/f0e9ddfc4d6ae6551c2aee04be7716987539ec31..c346701d64405ceaef330bb7e02471cdaf51d8a7

---

From: ianswett <[email-masked]>
Date: Fri, 07 May 2021 06:24:39 -0700

@ianswett commented on this pull request.

These two lines look identical, but I expect they're not and I copied over a non-breaking space or something?

-- 
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/4891#discussion_r628207169

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 07 May 2021 10:28:59 -0700

@janaiyengar commented on this pull request.

+results in persistent congestion being incorrectly declared. This also allows
+a connection to reset its estimate of min_rtt and smoothed_rtt after a
+disruptive network event; see {{smoothed-rtt}}.

```suggestion
congestion is established. This helps a connection deal when an increase
in path delay results in persistent congestion being incorrectly declared.
This also resets a connection's estimates of min_rtt and smoothed_rtt after
a disruptive network event; see {{smoothed-rtt}}.
```

-- 
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/4891#pullrequestreview-654709368

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 07 May 2021 10:30:37 -0700

@janaiyengar approved this pull request.

This is good, thank you !

-- 
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/4891#pullrequestreview-654710642

---

From: Martin Thomson <[email-masked]>
Date: Fri, 07 May 2021 15:53:49 -0700

> @@ -346,10 +346,10 @@ future RTT samples that are smaller than the new RTT to be included in
 smoothed_rtt.

Helps what?

The key is that it avoids repeatedly entering persistent congestion if the path RTT has genuinely increased.

-- 
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/4891#pullrequestreview-654914726

---

From: ianswett <[email-masked]>
Date: Tue, 11 May 2021 06:05:12 -0700

```suggestion
congestion is established. This avoids repeatedly declaring persistent congestion
incorrectly when the RTT increases. This also allows
```

-- 
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/4891#pullrequestreview-656680052

---

From: ianswett <[email-masked]>
Date: Tue, 11 May 2021 06:05:17 -0700

55f6d5300bae5cd444face295e01342df2577413  Update rfc9002.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4891/files/c346701d64405ceaef330bb7e02471cdaf51d8a7..55f6d5300bae5cd444face295e01342df2577413

---

From: ianswett <[email-masked]>
Date: Tue, 11 May 2021 06:06:15 -0700

b786b535f4eec93a44d25e97195ab5f1829902f2  Update rfc9002.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4891/files/55f6d5300bae5cd444face295e01342df2577413..b786b535f4eec93a44d25e97195ab5f1829902f2

---

From: ianswett <[email-masked]>
Date: Tue, 11 May 2021 06:06:52 -0700

smoothed_rtt.

Fair point, PTAL

-- 
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/4891#discussion_r630149041

---

From: Martin Thomson <[email-masked]>
Date: Tue, 11 May 2021 14:35:09 -0700

@martinthomson approved this pull request.

-- 
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/4891#pullrequestreview-657251511

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 11 May 2021 15:09:18 -0700

Merged #4891 into master.

-- 
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/4891#event-4731979530