Subject: [quicwg/base-drafts] Persistent congestion threshold is unreliable during the early stages of a connection (#3875)

From: Kazuho Oku <[email-masked]>
Date: Wed, 08 Jul 2020 03:45:49 -0700

Consider the following case:
* endpoint sends Initial
* after 1 second, endpoint sends Initial (PTO)
* after 1ms, the endpoint receives an ACK for Initial (PTO)

At this point, the endpoint would declare persistent congestion because the duration of congestion (1 seconds) is greater than the threshold (i.e. `(smoothed_rtt + 4 * rttvar + max_ack_delay) * kPersistentCongestionThreshold` where smoothed_rtt is 1ms, rttvar is 1ms, and max_ack_delay is zero).

The problem is that determining persistent congestion based on SRTT is not a good idea until the RTT estimate becomes stable.

I think the easiest fix (which is beneficial at this late stage) would be to disable persistent congestion until the handshake concludes.

Spawning off from #3831 as a dedicated issue.

-- 
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/issues/3875

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Jul 2020 17:52:02 -0700

My interpretation here is that the single packet loss does not result in persistent congestion as it spans precisely one packet.  That changes of course if there are two packets sent on PTO and the first of those is lost as the losses span multiple packets over a longer period.

-- 
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/issues/3875#issuecomment-655831740

---

From: Marten Seemann <[email-masked]>
Date: Wed, 08 Jul 2020 18:54:23 -0700

@martinthomson is right. The example should be:

* endpoint sends Initial (PN 1)
* after 1 second, endpoint sends two Initials on a PTO (PN 2 and PN 3)
* after 1ms, the endpoint receives an ACK for PN 3

Now we have a loss spanning over 1s, which is >> PTO.

-- 
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/issues/3875#issuecomment-655848798

---

From: ianswett <[email-masked]>
Date: Thu, 09 Jul 2020 16:10:45 -0700

I don't think persistent congestion can be enabled until there is an RTT sample, because smoothed_rtt and rttvar don't have measured values.

I wrote an editorial PR to that effect, so assuming that's sufficient, this is an editorial issue.

-- 
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/issues/3875#issuecomment-656394099

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 09 Jul 2020 16:46:26 -0700

@ianswett Thank you for the PR. As I've stated in the PR review comment, per my understanding, the problem is about applying the first RTT sample to the probes sent *before* obtaining that RTT sample.

Because the difference between the first RTT sample and initial RTT could be huge, using the first RTT sample to estimate how many probes have been sent based on the initial RTT does not work well.

-- 
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/issues/3875#issuecomment-656403944

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 12 Jul 2020 19:35:29 -0700

I agree that the PR doesn't solve this problem. Conservatively, you could avoid using losses of any packets that were sent before an RTT sample was received as legitimate for persistent congestion, and this might not be particularly hard to implement. You need one variable that is set on the first RTT measurement (largest PN sent for each space), and ensure that all PNs considered when detecting persistent congestion are greater than that number (per PN space).

Not doing persistent congestion before the handshake would be quite an aggressive departure from what our intent is.

-- 
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/issues/3875#issuecomment-657326999

---

From: ianswett <[email-masked]>
Date: Tue, 14 Jul 2020 12:40:00 -0700

I agree with Jana that disabling persistent congestion before the handshake would be quite aggressive and quite different from TCP, so I'd prefer to avoid that.

I was thinking persistent congestion was a timer that would be started when a packet is sent, but it's a time interval, so I agree the PR is not sufficient as is.  I'll update it along the lines of Jana's suggestion, which is what I intended.

-- 
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/issues/3875#issuecomment-658374513

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 14 Jul 2020 19:58:04 -0700

So, I understand the disconnect now after discussing with @kazuho. This issue is not just about before an initial RTT sample, it's about the case where the network RTT were to similarly reduce later in the connection. In such a case, when loss also happens, the sender could retroactively declare persistent congestion based on the new RTT estimate, while the previous RTT estimate might have caused fewer than 3 PTOs to fire. Specifically, the sender might have sent fewer than 3 packets as PTO responses into the network before marking the network as having persistent congestion.

This seems like a corner case to me that may not be necessary to fix (beyond what is being fixed in #3889). But I want to hear others' opinions.

-- 
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/issues/3875#issuecomment-658515935

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jul 2020 21:02:13 -0700

@janaiyengar Thank you for summarizing the issue.

Yes, my point was that the current definition of persistent congestion is inaccurate *if* it based on the rationale that the endpoint would have had occasions to send 3 probe packets, because the endpoint might have had a larger RTT estimate before receiving the ACK that triggers the persistent congestion check.

That said, in the current computation, we do have the guarantee that the endpoint would have sent at least 2 probe packets, because PTO never decreases by more than 25% when obtaining just one RTT sample. If we think that is enough, I'm fine with going with what we have now in #3889.

-- 
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/issues/3875#issuecomment-658532149

---

From: ianswett <[email-masked]>
Date: Wed, 15 Jul 2020 13:13:41 -0700

Thanks for reading the updated PR.

Assuming we go with something along the lines of #3889, I think this ends up as a design change, unless others think this is just a clarification of existing 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/issues/3875#issuecomment-658984532

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 15 Jul 2020 23:47:30 -0700

I think that this issue is a design issue and also that the current state of #3889 is a design change.

-- 
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/issues/3875#issuecomment-659195020

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 16 Jul 2020 18:54:31 -0700

This is a design issue.

As the design issue here is about whether 3 x PTO is about time, about packets, or both. That needs consensus.

-- 
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/issues/3875#issuecomment-659783855

---

From: ianswett <[email-masked]>
Date: Fri, 17 Jul 2020 08:48:31 -0700

My understanding was the point of changing from PTOs to persistent congestion was to make it about time.

If we think the change in RTT caused by the ACK which triggers the persistent congestion check is an issue, we should ignore that sample when doing the determination.  But I'm not convinced the extra complexity is worth it.

-- 
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/issues/3875#issuecomment-660181584

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 17 Jul 2020 17:49:24 -0700

@janaiyengar @ianswett Thank you for sharing the rationale behind the design. I am convinced that the fix in #3889  is fine.

-- 
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/issues/3875#issuecomment-660395787

---

From: ianswett <[email-masked]>
Date: Wed, 29 Jul 2020 06:05:08 -0700

Closed #3875 via #3889.

-- 
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/issues/3875#event-3598946641

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 29 Jul 2020 12:25:50 -0700

#3889 is now merged, but I realized that it has a slight problem. The text and the pseudo-code don't agree. Since this has gone through consensus, I'm continuing this discussion on #3972.

-- 
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/issues/3875#issuecomment-665856068