Re: [quicwg/base-drafts] Recovery uses both time and packet thresholds (#1974)

[Subodh Iyengar <[e-mail]>]

siyengar commented on this pull request.



> @@ -875,7 +841,7 @@ Pseudocode for OnLossDetectionTimeout follows:
        RetransmitUnackedCryptoData()
        crypto_count++
      else if (loss_time != 0):
-       // Early retransmit or Time Loss Detection
+       // Time threshold loss Detection

I don't think this is only early retransmit @janaiyengar in this current form. loss_time is always set here

> @@ -907,14 +872,8 @@ Pseudocode for DetectLostPackets follows:
 DetectLostPackets(largest_acked):
   loss_time = 0
   lost_packets = {}
-  delay_until_lost = infinite
-  if (kUsingTimeLossDetection):
-    delay_until_lost =
-      (1 + time_reordering_fraction) *
-          max(latest_rtt, smoothed_rtt)
-  else if (largest_acked.packet_number == largest_sent_packet):
-    // Early retransmit timer.
-    delay_until_lost = 9/8 * max(latest_rtt, smoothed_rtt)
+  delay_until_lost = (1 + time_reordering_fraction) *

this looks like it will always set the loss time to be  1 + 1/8 rtt now. However this might be super aggressive for regular cases. This is even lower than 1/4 rtt for tlp. So tlp will never fire.

it seems like @janaiyengar  is right here. Early retransmit and time based loss detection are 2 separate mechanisms with different time_reordering_fractions.

something like:

```
 delay_until_lost = (1 + time_ordering_fraction) * max(lrtt, srtt)
  if (largest_acked.packet_number == largest_sent_packet):
     delay_until_lost = 9/8 * max(lrtt, srtt)
```

-- 
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/1974#pullrequestreview-177063937