Repository: quic-go/quic-go
Pull request #3852 (2023-05-22 15:29:49)

rttstats: don't set initial RTT after obtaining a measurement

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/3852?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#3852](https://app.codecov.io/gh/quic-go/quic-go/pull/3852?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (4c99f29) into [master](https://app.codecov.io/gh/quic-go/quic-go/commit/cb3453db2508eedf4ea3ce49f86bbe029de5c1c9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (cb3453d) will **increase** coverage by `0.19%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master    #3852      +/-   ##
==========================================
+ Coverage   83.94%   84.13%   +0.19%     
==========================================
  Files         146      142       -4     
  Lines       14307    14222      -85     
==========================================
- Hits        12009    11965      -44     
+ Misses       1858     1818      -40     
+ Partials      440      439       -1     
```


| [Impacted Files](https://app.codecov.io/gh/quic-go/quic-go/pull/3852?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [internal/utils/rtt\_stats.go](https://app.codecov.io/gh/quic-go/quic-go/pull/3852?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-aW50ZXJuYWwvdXRpbHMvcnR0X3N0YXRzLmdv) | `94.74% <100.00%> (+4.35%)` | :arrow_up: |

... and [7 files with indirect coverage changes](https://app.codecov.io/gh/quic-go/quic-go/pull/3852/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)



Patch:
diff --git a/internal/utils/rtt_stats.go b/internal/utils/rtt_stats.go
index 527539e1..2cd9a191 100644
--- a/internal/utils/rtt_stats.go
+++ b/internal/utils/rtt_stats.go
@@ -103,8 +103,12 @@ func (r *RTTStats) SetMaxAckDelay(mad time.Duration) {
 // SetInitialRTT sets the initial RTT.
 // It is used during the 0-RTT handshake when restoring the RTT stats from the session state.
 func (r *RTTStats) SetInitialRTT(t time.Duration) {
+	// On the server side, by the time we get to process the session ticket,
+	// we might already have obtained an RTT measurement.
+	// This can happen if we received the ClientHello in multiple pieces, and one of those pieces was lost.
+	// Discard the restored value. A fresh measurement is always better.
 	if r.hasMeasurement {
-		panic("initial RTT set after first measurement")
+		return
 	}
 	r.smoothedRTT = t
 	r.latestRTT = t


Test patch:
diff --git a/internal/utils/rtt_stats_test.go b/internal/utils/rtt_stats_test.go
index 29892c6f..eae31926 100644
--- a/internal/utils/rtt_stats_test.go
+++ b/internal/utils/rtt_stats_test.go
@@ -154,4 +154,14 @@ var _ = Describe("RTT stats", func() {
 		Expect(rttStats.SmoothedRTT()).To(Equal(200 * time.Millisecond))
 		Expect(rttStats.MeanDeviation()).To(Equal(100 * time.Millisecond))
 	})
+
+	It("doesn't restore the RTT if we already have a measurement", func() {
+		const rtt = 10 * time.Millisecond
+		rttStats.UpdateRTT(rtt, 0, time.Now())
+		Expect(rttStats.LatestRTT()).To(Equal(rtt))
+		Expect(rttStats.SmoothedRTT()).To(Equal(rtt))
+		rttStats.SetInitialRTT(time.Minute)
+		Expect(rttStats.LatestRTT()).To(Equal(rtt))
+		Expect(rttStats.SmoothedRTT()).To(Equal(rtt))
+	})
 })