Deprecate gfe2_reloadable_flag_quic_sent_packet_manager_cleanup. gfe-relnote: n/a (Flag deprecation) PiperOrigin-RevId: 277057068 Change-Id: I7a9f46306476c905093b4ded8ce5da626884f608
diff --git a/quic/core/congestion_control/rtt_stats.h b/quic/core/congestion_control/rtt_stats.h index 997faa4..1c0466a 100644 --- a/quic/core/congestion_control/rtt_stats.h +++ b/quic/core/congestion_control/rtt_stats.h
@@ -73,11 +73,6 @@ QuicTime::Delta mean_deviation() const { return mean_deviation_; } - QuicTime::Delta max_ack_delay() const { - DCHECK(!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)); - return max_ack_delay_; - } - QuicTime last_update_time() const { return last_update_time_; } bool ignore_max_ack_delay() const { return ignore_max_ack_delay_; }
diff --git a/quic/core/congestion_control/rtt_stats_test.cc b/quic/core/congestion_control/rtt_stats_test.cc index 4d259d3..be5f4d6 100644 --- a/quic/core/congestion_control/rtt_stats_test.cc +++ b/quic/core/congestion_control/rtt_stats_test.cc
@@ -35,28 +35,17 @@ QuicTime::Zero()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.smoothed_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.max_ack_delay()); - } // Verify that a plausible ack delay increases the max ack delay. rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(400), QuicTime::Delta::FromMilliseconds(100), QuicTime::Zero()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.smoothed_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(100), - rtt_stats_.max_ack_delay()); - } // Verify that Smoothed RTT includes max ack delay if it's reasonable. rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(350), QuicTime::Delta::FromMilliseconds(50), QuicTime::Zero()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.smoothed_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(100), - rtt_stats_.max_ack_delay()); - } // Verify that large erroneous ack_delay does not change Smoothed RTT. rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(200), QuicTime::Delta::FromMilliseconds(300), @@ -64,10 +53,6 @@ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::FromMicroseconds(287500), rtt_stats_.smoothed_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(100), - rtt_stats_.max_ack_delay()); - } } TEST_F(RttStatsTest, SmoothedRttIgnoreAckDelay) { @@ -78,18 +63,12 @@ QuicTime::Zero()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.smoothed_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.max_ack_delay()); - } // Verify that a plausible ack delay increases the max ack delay. rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(300), QuicTime::Delta::FromMilliseconds(100), QuicTime::Zero()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.smoothed_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.max_ack_delay()); - } // Verify that Smoothed RTT includes max ack delay if it's reasonable. rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(300), QuicTime::Delta::FromMilliseconds(50), QuicTime::Zero()); @@ -102,9 +81,6 @@ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::FromMicroseconds(287500), rtt_stats_.smoothed_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.max_ack_delay()); - } } // Ensure that the potential rounding artifacts in EWMA calculation do not cause @@ -225,9 +201,6 @@ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.smoothed_rtt()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.min_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(0), rtt_stats_.max_ack_delay()); - } rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(300), QuicTime::Delta::FromMilliseconds(100), @@ -235,19 +208,12 @@ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.smoothed_rtt()); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.min_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(100), - rtt_stats_.max_ack_delay()); - } // Reset rtt stats on connection migrations. rtt_stats_.OnConnectionMigration(); EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.latest_rtt()); EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.smoothed_rtt()); EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.min_rtt()); - if (!GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.max_ack_delay()); - } } } // namespace test
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index 852f9d2..ff73abf 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -90,8 +90,6 @@ QuicTime::Delta::FromMilliseconds(kMinTailLossProbeTimeoutMs)), min_rto_timeout_( QuicTime::Delta::FromMilliseconds(kMinRetransmissionTimeMs)), - ietf_style_tlp_(false), - ietf_style_2x_tlp_(false), largest_mtu_acked_(0), handshake_confirmed_(false), peer_max_ack_delay_( @@ -137,29 +135,13 @@ if (config.HasClientSentConnectionOption(kMAD1, perspective)) { rtt_stats_.set_initial_max_ack_delay(peer_max_ack_delay_); } - if (GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - QUIC_RELOADABLE_FLAG_COUNT(quic_sent_packet_manager_cleanup); - if (config.HasClientSentConnectionOption(kMAD2, perspective)) { - // Set the minimum to the alarm granularity. - min_tlp_timeout_ = QuicTime::Delta::FromMilliseconds(1); - } - if (config.HasClientSentConnectionOption(kMAD3, perspective)) { - // Set the minimum to the alarm granularity. - min_rto_timeout_ = QuicTime::Delta::FromMilliseconds(1); - } - } else { - if (config.HasClientSentConnectionOption(kMAD2, perspective)) { - min_tlp_timeout_ = QuicTime::Delta::Zero(); - } - if (config.HasClientSentConnectionOption(kMAD3, perspective)) { - min_rto_timeout_ = QuicTime::Delta::Zero(); - } - if (config.HasClientSentConnectionOption(kMAD4, perspective)) { - ietf_style_tlp_ = true; - } - if (config.HasClientSentConnectionOption(kMAD5, perspective)) { - ietf_style_2x_tlp_ = true; - } + if (config.HasClientSentConnectionOption(kMAD2, perspective)) { + // Set the minimum to the alarm granularity. + min_tlp_timeout_ = QuicTime::Delta::FromMilliseconds(1); + } + if (config.HasClientSentConnectionOption(kMAD3, perspective)) { + // Set the minimum to the alarm granularity. + min_rto_timeout_ = QuicTime::Delta::FromMilliseconds(1); } if (GetQuicReloadableFlag(quic_enable_pto)) { @@ -952,12 +934,6 @@ return std::max(min_tlp_timeout_, srtt * 0.5); } } - if (ietf_style_tlp_) { - return std::max(min_tlp_timeout_, 1.5 * srtt + rtt_stats_.max_ack_delay()); - } - if (ietf_style_2x_tlp_) { - return std::max(min_tlp_timeout_, 2 * srtt + rtt_stats_.max_ack_delay()); - } if (!unacked_packets_.HasMultipleInFlightPackets()) { // This expression really should be using the delayed ack time, but in TCP // MinRTO was traditionally set to 2x the delayed ack timer and this
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index e6a5b6f..b56b999 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -554,10 +554,6 @@ QuicTime::Delta min_tlp_timeout_; // The minimum RTO. QuicTime::Delta min_rto_timeout_; - // Whether to use IETF style TLP that includes the max ack delay. - bool ietf_style_tlp_; - // IETF style TLP, but with a 2x multiplier instead of 1.5x. - bool ietf_style_2x_tlp_; // Vectors packets acked and lost as a result of the last congestion event. AckedPacketVector packets_acked_;
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc index e86d950..9567fc4 100644 --- a/quic/core/quic_sent_packet_manager_test.cc +++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -1956,11 +1956,8 @@ EXPECT_EQ(QuicTime::Delta::FromMicroseconds(100002), QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); - // Send two packets, and the TLP should be 2 us or 1ms. - QuicTime::Delta expected_tlp_delay = - GetQuicReloadableFlag(quic_sent_packet_manager_cleanup) - ? QuicTime::Delta::FromMilliseconds(1) - : QuicTime::Delta::FromMicroseconds(2); + // Send two packets, and the TLP should be 1ms. + QuicTime::Delta expected_tlp_delay = QuicTime::Delta::FromMilliseconds(1); SendDataPacket(1); SendDataPacket(2); EXPECT_EQ(expected_tlp_delay, @@ -1991,11 +1988,8 @@ QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_)); EXPECT_EQ(QuicTime::Delta::FromMicroseconds(100002), QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); - // Send two packets, and the TLP should be 2 us or 1ms. - QuicTime::Delta expected_tlp_delay = - GetQuicReloadableFlag(quic_sent_packet_manager_cleanup) - ? QuicTime::Delta::FromMilliseconds(1) - : QuicTime::Delta::FromMicroseconds(2); + // Send two packets, and the TLP should be 1ms. + QuicTime::Delta expected_tlp_delay = QuicTime::Delta::FromMilliseconds(1); SendDataPacket(1); SendDataPacket(2); EXPECT_EQ(expected_tlp_delay, @@ -2004,69 +1998,6 @@ QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); } -TEST_F(QuicSentPacketManagerTest, NegotiateIETFTLPFromOptionsAtServer) { - if (GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - return; - } - QuicConfig config; - QuicTagVector options; - - options.push_back(kMAD4); - QuicConfigPeer::SetReceivedConnectionOptions(&config, options); - EXPECT_CALL(*network_change_visitor_, OnCongestionChange()); - EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); - manager_.SetFromConfig(config); - // Provide an RTT measurement of 100ms. - RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); - rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(100), - QuicTime::Delta::Zero(), QuicTime::Zero()); - // Expect 1.5x * SRTT + 0ms MAD - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(150), - QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_)); - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(150), - QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); - // Expect 1.5x * SRTT + 50ms MAD - rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(150), - QuicTime::Delta::FromMilliseconds(50), QuicTime::Zero()); - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(100), rtt_stats->smoothed_rtt()); - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), - QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_)); - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), - QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); -} - -TEST_F(QuicSentPacketManagerTest, NegotiateIETFTLPFromOptionsAtClient) { - if (GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { - return; - } - QuicConfig client_config; - QuicTagVector options; - - options.push_back(kMAD4); - QuicSentPacketManagerPeer::SetPerspective(&manager_, Perspective::IS_CLIENT); - client_config.SetConnectionOptionsToSend(options); - EXPECT_CALL(*network_change_visitor_, OnCongestionChange()); - EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); - manager_.SetFromConfig(client_config); - // Provide an RTT measurement of 100ms. - RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); - rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(100), - QuicTime::Delta::Zero(), QuicTime::Zero()); - // Expect 1.5x * SRTT + 0ms MAD - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(150), - QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_)); - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(150), - QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); - // Expect 1.5x * SRTT + 50ms MAD - rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(150), - QuicTime::Delta::FromMilliseconds(50), QuicTime::Zero()); - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(100), rtt_stats->smoothed_rtt()); - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), - QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_)); - EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), - QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); -} - TEST_F(QuicSentPacketManagerTest, NegotiateNoMinRTOFromOptionsAtServer) { QuicConfig config; QuicTagVector options; @@ -2080,19 +2011,13 @@ RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); rtt_stats->UpdateRtt(QuicTime::Delta::FromMicroseconds(1), QuicTime::Delta::Zero(), QuicTime::Zero()); - QuicTime::Delta expected_rto_delay = - GetQuicReloadableFlag(quic_sent_packet_manager_cleanup) - ? QuicTime::Delta::FromMilliseconds(1) - : QuicTime::Delta::FromMicroseconds(1); + QuicTime::Delta expected_rto_delay = QuicTime::Delta::FromMilliseconds(1); EXPECT_EQ(expected_rto_delay, QuicSentPacketManagerPeer::GetRetransmissionDelay(&manager_)); EXPECT_EQ(expected_rto_delay, QuicSentPacketManagerPeer::GetRetransmissionDelay(&manager_, 0)); // The TLP with fewer than 2 packets outstanding includes 1/2 min RTO(0ms). - QuicTime::Delta expected_tlp_delay = - GetQuicReloadableFlag(quic_sent_packet_manager_cleanup) - ? QuicTime::Delta::FromMicroseconds(502) - : QuicTime::Delta::FromMicroseconds(2); + QuicTime::Delta expected_tlp_delay = QuicTime::Delta::FromMicroseconds(502); EXPECT_EQ(expected_tlp_delay, QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_)); EXPECT_EQ(expected_tlp_delay, @@ -2113,19 +2038,13 @@ RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); rtt_stats->UpdateRtt(QuicTime::Delta::FromMicroseconds(1), QuicTime::Delta::Zero(), QuicTime::Zero()); - QuicTime::Delta expected_rto_delay = - GetQuicReloadableFlag(quic_sent_packet_manager_cleanup) - ? QuicTime::Delta::FromMilliseconds(1) - : QuicTime::Delta::FromMicroseconds(1); + QuicTime::Delta expected_rto_delay = QuicTime::Delta::FromMilliseconds(1); EXPECT_EQ(expected_rto_delay, QuicSentPacketManagerPeer::GetRetransmissionDelay(&manager_)); EXPECT_EQ(expected_rto_delay, QuicSentPacketManagerPeer::GetRetransmissionDelay(&manager_, 0)); // The TLP with fewer than 2 packets outstanding includes 1/2 min RTO(0ms). - QuicTime::Delta expected_tlp_delay = - GetQuicReloadableFlag(quic_sent_packet_manager_cleanup) - ? QuicTime::Delta::FromMicroseconds(502) - : QuicTime::Delta::FromMicroseconds(2); + QuicTime::Delta expected_tlp_delay = QuicTime::Delta::FromMicroseconds(502); EXPECT_EQ(expected_tlp_delay, QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_)); EXPECT_EQ(expected_tlp_delay,