Subject: [quicwg/base-drafts] recovery pseudocode enters "recovery" after "slow start" (#4045)

From: Kazu Yamamoto <[email-masked]>
Date: Mon, 24 Aug 2020 17:56:16 -0700

The spec says that "congestion avoidance" after "slow start". However, it seems to me that pseudocode enters "recovery" after "slow start".

When packets are lost, `OnCongestionEvent` is called:

```
OnCongestionEvent(sent_time):
  // Start a new congestion event if packet was sent after the
  // start of the previous congestion recovery period.
  if (!InCongestionRecovery(sent_time)):
    congestion_recovery_start_time = now()
    congestion_window *= kLossReductionFactor
    congestion_window = max(congestion_window, kMinimumWindow)
    ssthresh = congestion_window
    // A packet can be sent to speed up loss recovery.
    MaybeSendOnePacket()
```

This function sets `congestion_recovery_start_time` if the state is not "recovery", which means either "slow start" or "congestion avoidance". So, "recovery" starts after "slow start".

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

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 25 Aug 2020 16:01:27 -0700

@kazu-yamamoto : you are right; perhaps this could be made part of #3982? 

-- 
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/4045#issuecomment-680310390

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 25 Aug 2020 17:20:04 -0700

@janaiyengar Better definition of each state is welcome. But I'm talking about the pseudocode. My implementation based on the pseudocode enters "recovery" after "slow start". That's why I noticed this issue. We should fix the pseudocode somehow. (To fix this bug, I introduced a variable to hold the state explicitly in Haskell quic.)

-- 
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/4045#issuecomment-680344591

---

From: Martin Thomson <[email-masked]>
Date: Tue, 25 Aug 2020 17:26:29 -0700

Entering recovery after slow start is correct.  The language in the draft was confusing.  Hopefully #4055 helps there.

-- 
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/4045#issuecomment-680352463

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 25 Aug 2020 18:25:07 -0700

> The language in the draft was confusing. 

Indeed...

-- 
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/4045#issuecomment-680405573

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 25 Aug 2020 18:50:45 -0700

Closed #4045.

-- 
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/4045#event-3692599757

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 25 Aug 2020 18:50:46 -0700

After merging #4055, the pseudocode matches the language. Let's close 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/issues/4045#issuecomment-680412175

---

From: Martin Thomson <[email-masked]>
Date: Tue, 25 Aug 2020 18:59:10 -0700

Let's leave this open until #4055 merges then.

-- 
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/4045#issuecomment-680414395

---

From: Martin Thomson <[email-masked]>
Date: Tue, 25 Aug 2020 18:59:15 -0700

Reopened #4045.

-- 
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/4045#event-3692615137