Subject: [quicwg/base-drafts] eliminate the difference between when having no RTT sample and having one (#3525)

From: Kazuho Oku <[email-masked]>
Date: Sat, 14 Mar 2020 23:53:24 -0700

As pointed out in #3524, I think that we should assume same amount of RTT variance when there is no RTT sample and when there is one sample, because variance is unknown in both cases.

This PR makes that change while keeping the initial PTO being the same (i.e. 1 second), and slightly simplifies the timeout logic.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * eliminate the different between when having no RTT sample and having one

-- File Changes --

    M draft-ietf-quic-recovery.md (37)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/3525.patch
https://github.com/quicwg/base-drafts/pull/3525.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/3525

---

From: ianswett <[email-masked]>
Date: Sun, 15 Mar 2020 17:26:24 -0700

ianswett commented on this pull request.

I think this is a nice simplification, and one I've considered before, but I think we need to be careful to ensure an actual RTT measurement always erases any historical or default values.

> @@ -353,14 +353,17 @@ endpoint:
 On the first RTT sample for a network path, the smoothed_rtt is set to the
 latest_rtt.
 
-smoothed_rtt and rttvar are computed as follows, similar to {{?RFC6298}}.  On
-the first RTT sample for a network path:
+smoothed_rtt and rttvar are computed as follows, similar to {{?RFC6298}}.  When
+there is less than one sample for a network path:

```suggestion
there are no prior samples for a network path:
```

>  ~~~
 
+where rtt_sample is either the initial RTT when there is no sample, or the
+latest RTT when there is one.
+
 On subsequent RTT samples, smoothed_rtt and rttvar evolve as follows:

I'm concerned this section is now potentially ambiguous.  I believe we want to maintain the property that the first actual RTT measurement replaces the initialized values, and I don't think that's clear enough here.

> @@ -353,14 +353,17 @@ endpoint:
 On the first RTT sample for a network path, the smoothed_rtt is set to the
 latest_rtt.

Moving this down above "On subsequent RTT samples," and adding that rttvar is also set on the first RTT sample  would help clarify this.

-- 
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/3525#pullrequestreview-374825664

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 15 Mar 2020 18:52:56 -0700

@kazuho pushed 1 commit.

fbefa4ae4fadf520918de418d4ea555c23851700  to be clear, split paragraphs that discuss no rtt sample vs. 1 rtt sample

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

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 15 Mar 2020 18:54:21 -0700

@kazuho pushed 1 commit.

a56598572fd8656b30a2c800eabacac24b75b180  typo

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

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 15 Mar 2020 18:55:14 -0700

@ianswett Thank you for the review. Would you ming checking if the updated text has what you have suggested?

-- 
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/3525#issuecomment-599303073

---

From: ianswett <[email-masked]>
Date: Mon, 16 Mar 2020 06:11:41 -0700

ianswett approved this pull request.

+where rtt_sample is the initial RTT.
+
+On the first RTT sample for the network path, smoothed_rtt and rttvar are set to
+the values using the formula above, using that first RTT sample as rtt_sample.

```suggestion
the values using the formula above, using that first RTT sample as rtt_sample.
This ensures a measurement erases the history of any persisted or default value.
```

-- 
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/3525#pullrequestreview-375196876

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 16 Mar 2020 12:21:30 -0700

@kazuho pushed 1 commit.

9c28244e70183fa35ee5690dc94b68f8deb28c98  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3525/files/a56598572fd8656b30a2c800eabacac24b75b180..9c28244e70183fa35ee5690dc94b68f8deb28c98

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 16 Mar 2020 12:22:01 -0700

kazuho commented on this pull request.

Thank you for the suggestion. Applied.

-- 
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/3525#discussion_r393259522

---

From: ianswett <[email-masked]>
Date: Mon, 16 Mar 2020 13:09:35 -0700

-- 
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/3525#pullrequestreview-375547788

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 17 Mar 2020 15:17:25 -0700

janaiyengar approved this pull request.

Thank you, @kazuho, this is a good fix. This also fixes the case where the 2 x initialRtt is smaller than granularity.

+where rtt_sample is the initial RTT.
+
+On the first RTT sample for the network path, smoothed_rtt and rttvar are set to
+the values using the formula above, using that first RTT sample as rtt_sample.
+This ensures a measurement erases the history of any persisted or default value.

```suggestion
This ensures that the first measurement erases the history of any persisted or default value.
```

-- 
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/3525#pullrequestreview-376442220

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 17 Mar 2020 16:27:35 -0700

@kazuho pushed 1 commit.

c4ed9aa8173340359ce52a5030141aef624e562d  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3525/files/9c28244e70183fa35ee5690dc94b68f8deb28c98..c4ed9aa8173340359ce52a5030141aef624e562d

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 17 Mar 2020 16:31:15 -0700

@kazuho pushed 1 commit.

31f5b8e9815a8d572f21f203ce2a6416765da13d  wordwrap

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3525/files/c4ed9aa8173340359ce52a5030141aef624e562d..31f5b8e9815a8d572f21f203ce2a6416765da13d

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 17 Mar 2020 16:32:03 -0700

Thank you for the suggestion. Applied in c4ed9aa.

-- 
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/3525#discussion_r394028844

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 18 Mar 2020 14:43:33 -0700

Just a few editorial suggestions.

> +smoothed_rtt and rttvar are computed as follows, similar to {{?RFC6298}}.  When
+there are no samples for a network path:

```suggestion
smoothed_rtt and rttvar are computed as follows, similar to {{?RFC6298}}.

When there are no samples for a network path, and on the first RTT sample for
the network path:
```

+
+On the first RTT sample for the network path, smoothed_rtt and rttvar are set to
+the values using the formula above, using that first RTT sample as rtt_sample.
+This ensures that the first measurement erases the history of any persisted or
+default value.

```suggestion
Before any RTT samples are available, the initial RTT is used as rtt_sample. 
On the first RTT sample for the network path, that sample is used as rtt_sample.
This ensures that the first measurement erases the history of any persisted or
default values.
```

> +When no previous RTT is available, the initial RTT SHOULD be set to 333ms,
+resulting in a 1 second initial timeout as recommended in {{?RFC6298}}. Resumed
+connections over the same network SHOULD use the previous connection's final
+smoothed RTT value as the resumed connection's initial RTT.

```suggestion
Resumed
connections over the same network SHOULD use the previous connection's final
smoothed RTT value as the resumed connection's initial RTT. When no previous
RTT is available, the initial RTT SHOULD be set to 333ms, resulting in a 1 second
initial timeout, as recommended in {{?RFC6298}}.
```

-- 
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/3525#pullrequestreview-377253942

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 18 Mar 2020 17:22:32 -0700

@kazuho pushed 1 commit.

512a5a4399191b11bc207443cce2cbd75943bbee  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3525/files/31f5b8e9815a8d572f21f203ce2a6416765da13d..512a5a4399191b11bc207443cce2cbd75943bbee

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 18 Mar 2020 17:27:01 -0700

Splitting the text when having no samples and having one is intentional, see the changes suggested in https://github.com/quicwg/base-drafts/pull/3525#pullrequestreview-374825664.

I'm personally fine with the changes proposed here, but I would like to see what @ianswett thinks. Maybe it's OK, as the we have two paragraphs right after, discussing about each of the two conditions.

-- 
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/3525#discussion_r394720545

---

From: ianswett <[email-masked]>
Date: Sat, 21 Mar 2020 12:11:27 -0700

I think Jana's suggestions LGTM.

I think @janaiyengar suggestion seems good and slightly clearer than before, though maybe line 361("where rtt_sample is the initial RTT." needs to be updated too?

> @@ -517,12 +521,11 @@ data.
 
 ### Handshakes and New Paths
 
-The initial probe timeout for a new connection or new path SHOULD be
-set to twice the initial RTT.  Resumed connections over the same network
-SHOULD use the previous connection's final smoothed RTT value as the resumed
-connection's initial RTT.  If no previous RTT is available, the initial RTT
-SHOULD be set to 500ms, resulting in a 1 second initial timeout as recommended
-in {{?RFC6298}}.
+Resumed

nit: reflow the text

-- 
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/3525#pullrequestreview-378943350

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 19:38:04 -0700

@kazuho pushed 1 commit.

d887d3bd18b3638f73703885db0e93f32120345a  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3525/files/512a5a4399191b11bc207443cce2cbd75943bbee..d887d3bd18b3638f73703885db0e93f32120345a

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 19:38:20 -0700

@kazuho pushed 1 commit.

22a3071287a58755789df4115eec579b86198a52  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3525/files/d887d3bd18b3638f73703885db0e93f32120345a..22a3071287a58755789df4115eec579b86198a52

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 19:46:18 -0700

@kazuho pushed 1 commit.

3c7a903cfd8b86f9fe81522cd81c967f7edba919  wordwrap

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3525/files/22a3071287a58755789df4115eec579b86198a52..3c7a903cfd8b86f9fe81522cd81c967f7edba919

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 19:48:07 -0700

@janaiyengar @ianswett Thank you for the suggestions. I've applied them.

-- 
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/3525#issuecomment-606366287

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 30 Mar 2020 19:57:13 -0700

@janaiyengar approved this pull request.

LGTM - thanks, @kazuho! Can you resolve the merge conflict?

-- 
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/3525#pullrequestreview-384369728

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 21:59:17 -0700

@kazuho pushed 1 commit.

7bc36d9a8e26ed505dab7082fb0da172839088c6  Merge branch 'master' into kazuho/initial-timeouts

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3525/files/3c7a903cfd8b86f9fe81522cd81c967f7edba919..7bc36d9a8e26ed505dab7082fb0da172839088c6

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 23:02:30 -0700

@kazuho pushed 1 commit.

12b817e737f4e3ccd4e9a45db40036496f7bd0c5  wordwrap

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3525/files/7bc36d9a8e26ed505dab7082fb0da172839088c6..12b817e737f4e3ccd4e9a45db40036496f7bd0c5

---

From: ianswett <[email-masked]>
Date: Tue, 31 Mar 2020 08:52:10 -0700

@ianswett 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/3525#pullrequestreview-384872046