Subject: Re: [quicwg/base-drafts] Rework section on persistent congestion (#3961)

From: ianswett <[email-masked]>
Date: Sat, 01 Aug 2020 18:50:22 -0700

@ianswett approved this pull request.

> +
+This duration allows a sender to send as many packets, including some in
+response to PTO expiration, as TCP does with Tail Loss Probe ({{RACK}}), before
+establishing persistent congestion, as TCP does with a Retransmission Timeout
+({{?RFC5681}}).
+
+The RECOMMENDED value for kPersistentCongestionThreshold is 3, which is
+approximately equivalent to two TLPs before an RTO in TCP.
+
+This design does not use consecutive PTO events to establish persistent
+congestion, since application patterns impact PTO expirations. For example, a
+sender that sends small amounts of data with silence periods between them
+restarts the PTO timer every time it sends, potentially preventing the PTO timer
+from expiring for a long period of time, even when no acknowledgments are being
+received. The use of a duration enables a sender to establish persistent
+congestion without depending on the occurrence of PTOs.

We use expiry elsewhere, so maybe this is slightly clearer?

```suggestion
congestion without depending on the expiry of PTOs.
```

> +This design does not use consecutive PTO events to establish persistent
+congestion, since application patterns impact PTO expirations. For example, a
+sender that sends small amounts of data with silence periods between them
+restarts the PTO timer every time it sends, potentially preventing the PTO timer
+from expiring for a long period of time, even when no acknowledgments are being
+received. The use of a duration enables a sender to establish persistent
+congestion without depending on the occurrence of PTOs.
+
+### Establishing Persistent Congestion
+
+A sender establishes persistent congestion on receiving an acknowledgement if at
+least two ack-eliciting packets are declared lost, and:
+
+* a prior RTT sample existed when both packets were sent;
+
+* the duration between the send times of these two packets exceeds the

I think this is still ambiguous if more than 2 packets are lost, so I'm reusing language below.

```suggestion
* the duration between the send times of the oldest and newest lost packet exceeds the
```

-- 
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/3961#pullrequestreview-459613948

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 02 Aug 2020 14:22:19 -0700

@janaiyengar commented on this pull request.

That's still not correct. The point here is that there's a contiguous period that is long enough, and the duration has to be between two packets that were marked lost, not necessarily the oldest and the newest packets. For example, consider that 
- packets 1-10 were sent,
- all odd packets were marked lost: 1, 3, 5, 7, 9.
- the duration between 1 and 9 was greater than the persistent congestion duration.

According to the existing design, this should not be considered persistent congestion, but with your proposed text, it will be.

-- 
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/3961#discussion_r464124805

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 02 Aug 2020 14:25:42 -0700

@janaiyengar pushed 1 commit.

275d9bca78ef19750123bbbcd682b6f000f4cc4e  PTO expiration

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3961/files/b38fb27f7c14fbf01acc7164417882b51ee34f3e..275d9bca78ef19750123bbbcd682b6f000f4cc4e

---

From: ianswett <[email-masked]>
Date: Mon, 03 Aug 2020 09:19:25 -0700

@ianswett commented on this pull request.

There's a requirement that all packets between the largest and smallest are marked lost as well, so my suggested change would still not declare this as persistent congestion(assuming 2, 4, 6 or 8 were acknowledged).

I still think this is unclear if more than 2 packets are lost, so maybe you have a suggestion?

-- 
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/3961#discussion_r464518304

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 05 Aug 2020 22:35:17 -0700

We can't require all packets between largest and smallest be acked; see my comment: https://github.com/quicwg/base-drafts/pull/3961#discussion_r463907388.

-- 
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/3961#discussion_r466156457

---

From: ianswett <[email-masked]>
Date: Mon, 10 Aug 2020 17:25:30 -0700

One more suggestion

Sorry for the confusion, I now have what you have in mind.

If you put this bullet after the next one, I think this will be clearer.

-- 
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/3961#pullrequestreview-464670452

---

From: Mike Bishop <[email-masked]>
Date: Tue, 11 Aug 2020 13:23:58 -0700

@MikeBishop approved this pull request.

This many parenthetical phrases in commas is tough to parse.  Can we divide it up a little more?  Into multiple sentences, by using something other than commas, or by condensing?

```suggestion
This duration allows a sender to send as many packets before
establishing persistent congestion, including some in
response to PTO expiration, as TCP does with a Retransmission Timeout
({{?RFC5681}}) and a Tail Loss Probe ({{RACK}}).
```

Actually, I might move that third bullet to be first.  Conceptually, it frames the idea that they're the first and last packets of a 2+ packet sequence.

-- 
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/3961#pullrequestreview-465397169

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 11 Aug 2020 14:55:49 -0700

slightly rephrased, but taken.

-- 
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/3961#discussion_r468887005

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 11 Aug 2020 15:00:46 -0700

Yup, suggestions taken.

-- 
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/3961#discussion_r468889055

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 11 Aug 2020 15:01:17 -0700

f3baa0ec0fa78976e7f352061f5971c2217ad8c4  mike and ian comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3961/files/275d9bca78ef19750123bbbcd682b6f000f4cc4e..f3baa0ec0fa78976e7f352061f5971c2217ad8c4

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 20 Aug 2020 00:09:43 -0700

It would be necessary to use much resource to implement the detection of persistent congestion described here because of "across all packet spaces". So, let me ask a basic question. What is the motivation to define persistent congestion across all packet spaces? Isn't it good enough to limit persistent congestion only to 1-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/3961#issuecomment-677389067

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 20 Aug 2020 00:54:55 -0700

@kazu-yamamoto I'm not sure if the amount of state an endpoint needs to retain changes depending on if persistent congestion is a connection-wide event. FWIW, we had some discussion regarding the complexity in #3939, and my take is https://github.com/quicwg/base-drafts/issues/3939#issuecomment-662808337.

-- 
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/3961#issuecomment-677437685

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 20 Aug 2020 20:54:37 -0700

@kazuho 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/3961#pullrequestreview-472128288

---

From: Kazu Yamamoto <[email-masked]>
Date: Sun, 23 Aug 2020 17:19:21 -0700

I have implemented this algorithm based on  the pseudocode in #4010. As I said in https://github.com/quicwg/base-drafts/pull/4010#issuecomment-678469958, it was not difficult for Haskell quic to implement this since the data structure was able to be reused. However, I'm not sure if it is easy to implement this algorithm for all implementations and benefits outweighs its complexity.

-- 
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/3961#issuecomment-678845617

---

From: Martin Thomson <[email-masked]>
Date: Mon, 24 Aug 2020 17:55:07 -0700

@janaiyengar, let's merge this one.  Then we can talk about the pseudocode in #4010.

-- 
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/3961#issuecomment-679439789

---

From: Martin Thomson <[email-masked]>
Date: Mon, 24 Aug 2020 19:01:48 -0700

This resolves a design issue, so flagging it so that it doesn't get merged accidentally.

-- 
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/3961#issuecomment-679457674

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 24 Aug 2020 19:02:06 -0700

@martinthomson : This is part of the consensus call, so we need to wait for that to complete.

-- 
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/3961#issuecomment-679457735