From a6de3e42b3f1b2059b00ec72757d311e24c23f92 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 2 Nov 2025 12:53:30 -0500 Subject: [PATCH] ackhandler: fix qlogging of RTT values (#5418) --- internal/ackhandler/sent_packet_handler.go | 22 ++++++++++++++++++++++ internal/utils/rtt_stats.go | 4 ++++ internal/utils/rtt_stats_test.go | 6 ++++++ 3 files changed, 32 insertions(+) diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go index 274accd27..2c41f7f7f 100644 --- a/internal/ackhandler/sent_packet_handler.go +++ b/internal/ackhandler/sent_packet_handler.go @@ -333,6 +333,28 @@ func (h *sentPacketHandler) SentPacket( func (h *sentPacketHandler) qlogMetricsUpdated() { var metricsUpdatedEvent qlog.MetricsUpdated var updated bool + if h.rttStats.HasMeasurement() { + if h.lastMetrics.MinRTT == nil || *h.lastMetrics.MinRTT != h.rttStats.MinRTT() { + metricsUpdatedEvent.MinRTT = pointer(h.rttStats.MinRTT()) + h.lastMetrics.MinRTT = pointer(h.rttStats.MinRTT()) + updated = true + } + if h.lastMetrics.SmoothedRTT == nil || *h.lastMetrics.SmoothedRTT != h.rttStats.SmoothedRTT() { + metricsUpdatedEvent.SmoothedRTT = pointer(h.rttStats.SmoothedRTT()) + h.lastMetrics.SmoothedRTT = pointer(h.rttStats.SmoothedRTT()) + updated = true + } + if h.lastMetrics.LatestRTT == nil || *h.lastMetrics.LatestRTT != h.rttStats.LatestRTT() { + metricsUpdatedEvent.LatestRTT = pointer(h.rttStats.LatestRTT()) + h.lastMetrics.LatestRTT = pointer(h.rttStats.LatestRTT()) + updated = true + } + if h.lastMetrics.RTTVariance == nil || *h.lastMetrics.RTTVariance != h.rttStats.MeanDeviation() { + metricsUpdatedEvent.RTTVariance = pointer(h.rttStats.MeanDeviation()) + h.lastMetrics.RTTVariance = pointer(h.rttStats.MeanDeviation()) + updated = true + } + } if h.lastMetrics.CongestionWindow == nil || *h.lastMetrics.CongestionWindow != int(h.congestion.GetCongestionWindow()) { metricsUpdatedEvent.CongestionWindow = pointer(int(h.congestion.GetCongestionWindow())) h.lastMetrics.CongestionWindow = pointer(int(h.congestion.GetCongestionWindow())) diff --git a/internal/utils/rtt_stats.go b/internal/utils/rtt_stats.go index fab1de8c2..275313073 100644 --- a/internal/utils/rtt_stats.go +++ b/internal/utils/rtt_stats.go @@ -115,6 +115,10 @@ func (r *RTTStats) UpdateRTT(sendDelta, ackDelay time.Duration) { } } +func (r *RTTStats) HasMeasurement() bool { + return r.hasMeasurement +} + // SetMaxAckDelay sets the max_ack_delay func (r *RTTStats) SetMaxAckDelay(mad time.Duration) { r.maxAckDelay.Store(int64(mad)) diff --git a/internal/utils/rtt_stats_test.go b/internal/utils/rtt_stats_test.go index 75c94886c..b7504de13 100644 --- a/internal/utils/rtt_stats_test.go +++ b/internal/utils/rtt_stats_test.go @@ -10,14 +10,17 @@ import ( func TestRTTStatsDefaults(t *testing.T) { rttStats := NewRTTStats() + require.False(t, rttStats.HasMeasurement()) require.Equal(t, DefaultInitialRTT, rttStats.MinRTT()) require.Equal(t, DefaultInitialRTT, rttStats.SmoothedRTT()) } func TestRTTStatsSmoothedRTT(t *testing.T) { rttStats := NewRTTStats() + require.False(t, rttStats.HasMeasurement()) // verify that ack_delay is ignored in the first measurement rttStats.UpdateRTT(300*time.Millisecond, 100*time.Millisecond) + require.True(t, rttStats.HasMeasurement()) require.Equal(t, 300*time.Millisecond, rttStats.LatestRTT()) require.Equal(t, 300*time.Millisecond, rttStats.SmoothedRTT()) // verify that smoothed RTT includes max ack delay if it's reasonable @@ -122,11 +125,13 @@ func TestRTTStatsResetForPathMigration(t *testing.T) { rttStats.SetMaxAckDelay(42 * time.Millisecond) rttStats.UpdateRTT(time.Second, 0) rttStats.UpdateRTT(10*time.Second, 0) + require.True(t, rttStats.HasMeasurement()) require.Equal(t, time.Second, rttStats.MinRTT()) require.Equal(t, 10*time.Second, rttStats.LatestRTT()) require.NotZero(t, rttStats.SmoothedRTT()) rttStats.ResetForPathMigration() + require.False(t, rttStats.HasMeasurement()) require.Equal(t, DefaultInitialRTT, rttStats.MinRTT()) require.Equal(t, DefaultInitialRTT, rttStats.LatestRTT()) require.Equal(t, DefaultInitialRTT, rttStats.SmoothedRTT()) @@ -135,6 +140,7 @@ func TestRTTStatsResetForPathMigration(t *testing.T) { require.Equal(t, 42*time.Millisecond, rttStats.MaxAckDelay()) rttStats.UpdateRTT(10*time.Millisecond, 0) + require.True(t, rttStats.HasMeasurement()) require.Equal(t, 10*time.Millisecond, rttStats.SmoothedRTT()) require.Equal(t, 10*time.Millisecond, rttStats.LatestRTT()) }