Subject: Re: [quicwg/base-drafts] Persistent Congestion Time Threshold (#2365)

From: Nick Banks <[email-masked]>
Date: Tue, 05 Feb 2019 15:09:52 -0800

@nibanks pushed 1 commit.

91481a34c75ac65a91e01b18031e4c256cf2fbd2  Merge branch 'master' into pr/persistent-congestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2365/files/ed4ad7ebbad7ba063d0dde2aad67e24fcccd8308..91481a34c75ac65a91e01b18031e4c256cf2fbd2

---

From: Nick Banks <[email-masked]>
Date: Tue, 05 Feb 2019 15:17:58 -0800

@ianswett / @janaiyengar anything else that needs to be fixed as part of this 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/2365#issuecomment-460843032

---

From: ianswett <[email-masked]>
Date: Wed, 06 Feb 2019 13:50:29 -0800

ianswett commented on this pull request.

> @@ -1282,15 +1287,27 @@ Invoked by loss detection from DetectLostPackets when new packets
 are detected lost.
 
 ~~~
+   InPersistentCongestion(oldest_loss_time):
+     congestion_period = Now() - oldest_loss_time

I think I concluded newest_lost_time - oldest_loss_time is the best option here.  Using Now() means that a single lost packet combined with some temporary ack loss causes this to trigger, which I do not believe is the intent.

-- 
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/2365#pullrequestreview-200823494

---

From: ianswett <[email-masked]>
Date: Wed, 06 Feb 2019 13:52:19 -0800

// Start a new congestion epoch if the last lost packet
      // is past the end of the previous recovery epoch.
-     CongestionEvent(largest_lost_packet.time_sent)
+     CongestionEvent(newest_lost_packet.time_sent)
+
+     // Collapse congestion window if persistent congestion
+     if (InPersistentCongestion(oldest_lost_packet.time_sent)):

I'd like the opinion of @janaiyengar here.  I think this is likely fairly good, but I think passing in newest_lost_time is easy and more correct, given they're easily available in the pseudocode.

-- 
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/2365#discussion_r254466754

---

From: Nick Banks <[email-masked]>
Date: Wed, 06 Feb 2019 14:10:32 -0800

nibanks commented on this pull request.

Let me propose an example. Assume smooth_rtted = 1 sec, rttvar = 0, max_ack_delay = 0 for simplicity's sake.
```
t=0 | Send Pkt #1 (App Data)
t=1 | Send Pkt #2 (PTO 1)
t=3 | Send Pkt #3 (PTO 2)
t=7 | Send Pkt #4 (PTO 3)
t=8 | Recv ACK of Pkt #4
```
At t=8 you'd get a congestion event for packets 1 & 2 being lost. If you use Now() for the persistent congestion calculation, this would be declared as persistent congestion. If you used newest_lost_time it wouldn't. Which behavior do we want?

-- 
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/2365#discussion_r254472723

---

From: ianswett <[email-masked]>
Date: Thu, 07 Feb 2019 11:24:40 -0800

If 4 is acked and the others are lost, then it should be persistent congestion.

But if 2, 3 and 4 were all acked at once(t=8), that should not be persistent congestion.

-- 
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/2365#discussion_r254833380

---

From: Nick Banks <[email-masked]>
Date: Thu, 07 Feb 2019 11:29:56 -0800

Hmm, well then we'd have to make two changes to achieve that behavior.
1. Use `newest_lost_packet - oldest_lost_packet` for the time window.
2. Make the threshold 2 PTO instead of 3.

Without the second part above, an ACK of pkt 4 would produce a loss window of only 3 seconds (3 - 0), right?

-- 
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/2365#discussion_r254835364

---

From: ianswett <[email-masked]>
Date: Thu, 07 Feb 2019 11:44:45 -0800

That sounds correct to me.  Can you add your example as an example to the text section as well?  This is clearly very confusing, so an example would help.

-- 
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/2365#discussion_r254840887

---

From: Nick Banks <[email-masked]>
Date: Thu, 07 Feb 2019 11:46:12 -0800

Sure. Do you want me to describe the example in just words, or put almost exactly what I have above with 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/2365#discussion_r254841436

---

From: ianswett <[email-masked]>
Date: Thu, 07 Feb 2019 19:49:05 +0000 (UTC)

I think how you have it laid out is great and an effort to convert it into text will probably make it less clear.  But I'm not familiar enough with markdown to know how to get the formatting right.

-- 
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/2365#discussion_r254842489

---

From: Nick Banks <[email-masked]>
Date: Thu, 07 Feb 2019 19:50:20 +0000 (UTC)

Ok. I'll give it a go.

-- 
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/2365#discussion_r254842942

---

From: Nick Banks <[email-masked]>
Date: Thu, 07 Feb 2019 12:10:30 -0800

731211fb037ee11bf63db705f7202bca60b4f82c  slight redesign and example

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2365/files/91481a34c75ac65a91e01b18031e4c256cf2fbd2..731211fb037ee11bf63db705f7202bca60b4f82c

---

From: Nick Banks <[email-masked]>
Date: Thu, 07 Feb 2019 12:11:56 -0800

d0254cc1e07bd3fbca1ae99134655864fb26e552  forgot to hit save first...

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2365/files/731211fb037ee11bf63db705f7202bca60b4f82c..d0254cc1e07bd3fbca1ae99134655864fb26e552

---

From: Nick Banks <[email-masked]>
Date: Thu, 07 Feb 2019 18:10:31 -0800

be6ce7203971eb1e5e26df20b051aa795aa83236  whitespace removal

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

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 07 Feb 2019 21:16:01 -0800

janaiyengar commented on this pull request.

> +  t=1 | Send Pkt #2 (PTO 1)
+  t=3 | Send Pkt #3 (PTO 2)
+  t=7 | Send Pkt #4 (PTO 3)
+  t=8 | Recv ACK of Pkt #4
+
+The first three packets are determined to be lost when the ACK of packet 4 is
+received at t=8.  The congestion period is calculated as the time between the
+oldest and newest lost packets: (3 - 0) = 3.  The duration for persistent
+congestion is equal to: (1 * ((2 ^ 2) - 1)) = 3.  Because the threshold was
+reached, the network is considered to have experienced persistent congestion.
+
+When persistent congestion is established, the sender's congestion window MUST
+be reduced to the minimum congestion window (kMinimumWindow).  This response of
+collapsing the congestion window on persistent congestion is functionally
+similar to a sender's response on a Retransmission Timeout (RTO) in TCP
+{{RFC5681}}.

```suggestion
{{RFC5681}} after Tail Loss Probes (TLP) {{TLP}}.
```

> @@ -1282,15 +1309,28 @@ Invoked by loss detection from DetectLostPackets when new packets
 are detected lost.
 
 ~~~
+   InPersistentCongestion(congestion_period):
+     pto = smoothed_rtt + 4 * rttvar + max_ack_delay
+     return
+       congestion_period >
+         pto * (2 ^ kPersistentCongestionThreshold - 1)

editorial nit: this looks a bit confusing because 'return' is on a line by itself. How about:

return congestion_period >
                    pto * (2 ^ kPersistentCongestionThreshold - 1)

>  When an ACK frame is received that establishes loss of all in-flight packets
-sent prior to a threshold number of consecutive PTOs (pto_count is more than
-kPersistentCongestionThreshold, see {{cc-consts-of-interest}}), the network is
-considered to be experiencing persistent congestion, and the sender's congestion
-window MUST be reduced to the minimum congestion window (kMinimumWindow).  This
-response of collapsing the congestion window on persistent congestion is
-functionally similar to a sender's response on a Retransmission Timeout (RTO) in
-TCP {{RFC5681}}.
+sent over a long enough period of time, the network is considered to be
+experiencing persistent congestion.  Commonly, this can be established by
+consecutive PTOs, but since the PTO timer is reset when a new ack-eliciting
+packet is sent, an explicit duration must be used to account for those cases
+where PTOs do not occur or are substantially delayed.  This duration is the
+equivalent of kPersistentCongestionThreshold consecutive PTOS, smoothed_rtt +
+4 * rttvar + max_ack_delay * ((2 ^ kPersistentCongestionThreshold) - 1).

Agreed that it seems odd to backoff on max_ack_delay. @ianswett : Let's do that separately.

-- 
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/2365#pullrequestreview-201433164

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 07 Feb 2019 21:18:14 -0800

are detected lost.
 
 ~~~

Yes, I agree. I had thought that Now() was best, but that does not account for pauses in sending... newest_lost_time - oldest_lost_time is quite good I think.

-- 
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/2365#discussion_r254957869

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 07 Feb 2019 21:19:16 -0800

Looks great! Thanks for doing the work, @nibanks - I like this new form of the algorithm much better. I think it's basically correct, and the text and example are both excellent.

-- 
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/2365#issuecomment-461693803

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 07 Feb 2019 21:19:28 -0800

janaiyengar 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/2365#pullrequestreview-201434461

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 07 Feb 2019 21:29:44 -0800

@janaiyengar pushed 1 commit.

4e4b4fbceaa9b71f65fa003cf1918c73bb1dde99  editorial nits

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2365/files/be6ce7203971eb1e5e26df20b051aa795aa83236..4e4b4fbceaa9b71f65fa003cf1918c73bb1dde99

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 07 Feb 2019 21:31:33 -0800

68ec0b84c338a3894557051e53ffe4d891f9e9e8  another nit

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2365/files/4e4b4fbceaa9b71f65fa003cf1918c73bb1dde99..68ec0b84c338a3894557051e53ffe4d891f9e9e8

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 07 Feb 2019 21:40:31 -0800

@ianswett: want to take a look?

-- 
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/2365#issuecomment-461696945

---

From: ianswett <[email-masked]>
Date: Fri, 08 Feb 2019 12:54:01 -0800

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/2365#pullrequestreview-201780837

---

From: Marten Seemann <[email-masked]>
Date: Fri, 08 Feb 2019 18:30:35 -0800

marten-seemann commented on this pull request.

> +  max_ack_delay = 0
+  kPersistentCongestionThreshold = 2
+
+If an eck-eliciting packet is sent at time = 0, the following scenario would
+illustrate persistent congestion:
+
+  t=0 | Send Pkt #1 (App Data)
+  t=1 | Send Pkt #2 (PTO 1)
+  t=3 | Send Pkt #3 (PTO 2)
+  t=7 | Send Pkt #4 (PTO 3)
+  t=8 | Recv ACK of Pkt #4
+
+The first three packets are determined to be lost when the ACK of packet 4 is
+received at t=8.  The congestion period is calculated as the time between the
+oldest and newest lost packets: (3 - 0) = 3.  The duration for persistent
+congestion is equal to: (1 * ((2 ^ 2) - 1)) = 3.  Because the threshold was

I had a hard time figuring out where this equation came from. Maybe it's more readable if you don't fill in the value for the `kPersistentCongestionThreshold`?

> +  rttvar = 0
+  max_ack_delay = 0
+  kPersistentCongestionThreshold = 2
+
+If an eck-eliciting packet is sent at time = 0, the following scenario would
+illustrate persistent congestion:
+
+  t=0 | Send Pkt #1 (App Data)
+  t=1 | Send Pkt #2 (PTO 1)
+  t=3 | Send Pkt #3 (PTO 2)
+  t=7 | Send Pkt #4 (PTO 3)
+  t=8 | Recv ACK of Pkt #4
+
+The first three packets are determined to be lost when the ACK of packet 4 is
+received at t=8.  The congestion period is calculated as the time between the
+oldest and newest lost packets: (3 - 0) = 3.  The duration for persistent

This doesn't match the description in the first paragraph, does it?
> When an ACK frame is received that establishes loss of all in-flight packets, sent over a long enough period of time, the network is considered to be experiencing persistent congestion.

With this algorithm, we would detect persistent congestion even if packet 2 and 4 were acked, right?

-- 
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/2365#pullrequestreview-201843084

---

From: Nick Banks <[email-masked]>
Date: Mon, 11 Feb 2019 07:42:42 -0800

+  max_ack_delay = 0

The text says `all in-flight packets` meaning a contiguous block of sent packets. The example doesn't call out that check for a contiguous block explicitly, but it would have to be made. So, in that case if packets 2 and 4 were acknowledged you wouldn't have that contiguous block of time that meets the requirements.

-- 
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/2365#discussion_r255562860

---

From: Nick Banks <[email-masked]>
Date: Mon, 11 Feb 2019 07:44:39 -0800

We pretty clearly state the equation for calculating the persistent congestion window above and state that `kPersistentCongestionThreshold` is assumed to be 2 for this example. I don't really want to make this section any more verbose.

-- 
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/2365#discussion_r255563786

---

From: ianswett <[email-masked]>
Date: Mon, 11 Feb 2019 11:51:08 -0800

+equivalent of kPersistentCongestionThreshold consecutive PTOS, and is computed

```suggestion
equivalent of kPersistentCongestionThreshold consecutive PTOs, and is computed
```

-- 
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/2365#pullrequestreview-202305133

---

From: ianswett <[email-masked]>
Date: Mon, 11 Feb 2019 11:54:17 -0800

+  max_ack_delay = 0

I think this example works, but the pseudocode below is not quite right for this reason.  The easiest way to fix it would be to assume no packet number gaps and check if the number of packets lost is equal to largest - smallest + 1.

-- 
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/2365#discussion_r255668357

---

From: Nick Banks <[email-masked]>
Date: Mon, 11 Feb 2019 12:00:10 -0800

08da54a8c2d828b84ae61b2280994ade3ee09158  Ian's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2365/files/68ec0b84c338a3894557051e53ffe4d891f9e9e8..08da54a8c2d828b84ae61b2280994ade3ee09158

---

From: Nick Banks <[email-masked]>
Date: Mon, 11 Feb 2019 12:05:22 -0800

+  max_ack_delay = 0

Do we need to really spell this out with a 100% complete pseudocode example? I can imagine a couple ways for implementations to achieve this and I don't think we need to call them all out.

-- 
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/2365#discussion_r255672542

---

From: ianswett <[email-masked]>
Date: Mon, 11 Feb 2019 12:12:02 -0800

+  max_ack_delay = 0

The text is normative and the pseudocode is an appendix.  I'm ok with the pseudocode not being perfect, especially for cases that are fairly unlikely in practice.

-- 
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/2365#discussion_r255674698

---

From: Marten Seemann <[email-masked]>
Date: Mon, 11 Feb 2019 17:56:56 -0800

+  max_ack_delay = 0

The point of the pseudo-code is not to tell implementations exactly what to do. You can still implement it any way you want.
I'd like to avoid inconsistencies between the text and the pseudo code. We had these before when we introduced packet number spaces, and it now turns out to be quite a lot of work to get things straight.

-- 
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/2365#discussion_r255773243

---

From: Marten Seemann <[email-masked]>
Date: Mon, 11 Feb 2019 17:59:01 -0800

Just saying, this was my experience when reading this paragraph for the first time. Would be interesting to hear what other people reading this text for the first time think.

-- 
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/2365#discussion_r255773610

---

From: Nick Banks <[email-masked]>
Date: Tue, 12 Feb 2019 02:08:25 +0000 (UTC)

Do you have a recommendation of some specific text to add or change that would make things more clear?

-- 
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/2365#discussion_r255775248

---

From: Marten Seemann <[email-masked]>
Date: Mon, 11 Feb 2019 18:12:31 -0800

`1 * ((2 ^ kPersistentCongestionThreshold) - 1)) = 3`

-- 
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/2365#discussion_r255775921

---

From: Nick Banks <[email-masked]>
Date: Mon, 11 Feb 2019 18:16:35 -0800

4857ee815dfa5387b32767402e791340785c5779  Marten's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2365/files/08da54a8c2d828b84ae61b2280994ade3ee09158..4857ee815dfa5387b32767402e791340785c5779

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 19 Feb 2019 19:17:41 -0800

+  max_ack_delay = 0

I think @marten-seemann is right, we don't want to have differences between the text and the example at least. We don't need the pseudo code to be descriptive in how the datastructures are used, but It can specify what needs to be done. The problem here is that the example and the pseudo code are suggesting that this is simpler than it actually is. 

I think we may need to slightly alter the mechanism anyway. It's not the time difference between the newest and the oldest that's interesting, it's that packets sent in the 3 x PTO duration prior to the newest lost packet were all marked as lost. So (i) the latest packet sent >= 3 x PTO before the newest lost packet should be the oldest packet that we are interested in (not the oldest packet that is marked as lost), and (ii) we need to then check that all packets between that packet and the newest are marked lost.

So, I propose the following changes:
1. Change the example to show packets that were sent earlier than 3xPTO and marked lost, but they aren't used in the detection of PersistentCongestion
2. Add to the text below (proposed below)
3. Add to the pseudocode (proposed below)

+
+congestion is equal to: (1 * ((2 ^ kPersistentCongestionThreshold) - 1)) = 3.
+Because the threshold was reached, the network is considered to have experienced

Suggestion: "Because the threshold was reached and because none of the packets between the oldest and the newest packets are acknowledged, the network is considered ..."

> @@ -1282,15 +1314,27 @@ Invoked by loss detection from DetectLostPackets when new packets
 are detected lost.
 
 ~~~

Suggested rework of this function:
```
InPersistentCongestion(newest_lost_packet):
    pto = smoothed_rtt + 4 * rttvar + max_ack_delay
    congestion_period = pto * (2 ^ kPersistentCongestionThreshold - 1)
    // Find the latest packet that was sent at least pto time before the newest lost packet
    oldest_sent_packet = FindPacketSentBefore(newest_lost_packet, congestion_period)
    // Determine if all packets in the window, including the  edges, are marked lost
    return IsWindowLost(oldest_sent_packet, newest_lost_packet)
```

+     if (InPersistentCongestion(

Change params to InPersistentCongestion to only have newest_lost_packet.

-- 
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/2365#pullrequestreview-205554955

---

From: Nick Banks <[email-masked]>
Date: Thu, 21 Feb 2019 09:10:38 -0800

are detected lost.
 
 ~~~

I feel like this overly complicates things (at least for the pseudocode). What if simplify it a bit, like the following:
```
InPersistentCongestion(newest_lost_packet):
    pto = smoothed_rtt + 4 * rttvar + max_ack_delay
    congestion_period = pto * (2 ^ kPersistentCongestionThreshold - 1)
    // Determine if all packets in the window before the newest lost packet,
    // including the edges, are marked lost
    return IsWindowLost(newest_lost_packet, congestion_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/pull/2365#discussion_r259027096

---

From: Nick Banks <[email-masked]>
Date: Thu, 21 Feb 2019 09:13:56 -0800

a8510b1b50b7a1b8354b934bc1558a49457295ed  Jana's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2365/files/4857ee815dfa5387b32767402e791340785c5779..a8510b1b50b7a1b8354b934bc1558a49457295ed

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 21 Feb 2019 20:35:42 -0800

This LGTM. Thanks, @nibanks!

-- 
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/2365#pullrequestreview-206655235

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 21 Feb 2019 20:36:57 -0800

@ianswett: Can you take a look? I'd like to merge this one.

-- 
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/2365#issuecomment-466270803

---

From: ianswett <[email-masked]>
Date: Mon, 25 Feb 2019 08:37:48 -0800

@ianswett pushed 1 commit.

d81b39d20aeb29f83193e4f6864bf26096393aac  Change back to largest_lost_packet

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

---

From: ianswett <[email-masked]>
Date: Mon, 25 Feb 2019 08:37:57 -0800

>     OnPacketsLost(lost_packets):
      // Remove lost packets from bytes_in_flight.
      for (lost_packet : lost_packets):
        bytes_in_flight -= lost_packet.size
-     largest_lost_packet = lost_packets.last()
+     newest_lost_packet = lost_packets.last()

Elsewhere we use largest(ie: largest_acked) and there are currently no uses of newest in the recovery draft, so I'm going to change this back to largest.  If we want to start using newest, let's do that in 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/2365#pullrequestreview-207496591

---

From: Nick Banks <[email-masked]>
Date: Mon, 25 Feb 2019 16:39:24 +0000 (UTC)

Fine with me. I might look at changing it later.

-- 
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/2365#discussion_r259912200

---

From: ianswett <[email-masked]>
Date: Mon, 25 Feb 2019 08:45:14 -0800

Merged #2365 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/2365#event-2162330556