False deprecate --gfe2_restart_flag_quic_enable_sending_bandwidth_estimate_when_network_idle_v2. PiperOrigin-RevId: 566393408
diff --git a/quiche/quic/core/http/quic_server_session_base.cc b/quiche/quic/core/http/quic_server_session_base.cc index 350502b..fc39b90 100644 --- a/quiche/quic/core/http/quic_server_session_base.cc +++ b/quiche/quic/core/http/quic_server_session_base.cc
@@ -86,12 +86,6 @@ << "Failed to disable resumption"; } - enable_sending_bandwidth_estimate_when_network_idle_ = - GetQuicRestartFlag( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2) && - version().HasIetfQuicFrames() && - ContainsQuicTag(config()->ReceivedConnectionOptions(), kBWID); - // Enable bandwidth resumption if peer sent correct connection options. const bool last_bandwidth_resumption = ContainsQuicTag(config()->ReceivedConnectionOptions(), kBWRE); @@ -134,31 +128,10 @@ } } -void QuicServerSessionBase::OnBandwidthUpdateTimeout() { - if (!enable_sending_bandwidth_estimate_when_network_idle_) { - return; - } - QUIC_DVLOG(1) << "Bandwidth update timed out."; - const SendAlgorithmInterface* send_algorithm = - connection()->sent_packet_manager().GetSendAlgorithm(); - if (send_algorithm != nullptr && - send_algorithm->HasGoodBandwidthEstimateForResumption()) { - const bool success = MaybeSendAddressToken(); - QUIC_BUG_IF(QUIC_BUG_25522, !success) << "Failed to send address token."; - QUIC_RESTART_FLAG_COUNT_N( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2, 2, 3); - } -} +// TODO(b/296840230) Remove this virtual function. +void QuicServerSessionBase::OnBandwidthUpdateTimeout() {} void QuicServerSessionBase::OnCongestionWindowChange(QuicTime now) { - // Sending bandwidth is no longer conditioned on if session does bandwidth - // resumption. - if (GetQuicRestartFlag( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2)) { - QUIC_RESTART_FLAG_COUNT_N( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2, 3, 3); - return; - } if (!bandwidth_resumption_enabled_) { return; } @@ -357,55 +330,30 @@ sent_packet_manager.GetRttStats()->min_rtt().ToMilliseconds()); } - if (enable_sending_bandwidth_estimate_when_network_idle_) { - const SendAlgorithmInterface* send_algorithm = - sent_packet_manager.GetSendAlgorithm(); - if (send_algorithm != nullptr && - send_algorithm->HasGoodBandwidthEstimateForResumption()) { - cached_network_params.set_bandwidth_estimate_bytes_per_second( - BandwidthToCachedParameterBytesPerSecond( - send_algorithm->BandwidthEstimate())); - QUIC_CODE_COUNT(quic_send_measured_bandwidth_in_token); - } else { - const quic::CachedNetworkParameters* previous_cached_network_params = - crypto_stream()->PreviousCachedNetworkParams(); - if (previous_cached_network_params != nullptr && - previous_cached_network_params - ->bandwidth_estimate_bytes_per_second() > 0) { - cached_network_params.set_bandwidth_estimate_bytes_per_second( - previous_cached_network_params - ->bandwidth_estimate_bytes_per_second()); - QUIC_CODE_COUNT(quic_send_previous_bandwidth_in_token); - } else { - QUIC_CODE_COUNT(quic_not_send_bandwidth_in_token); - } - } - } else { - // Populate bandwidth estimates if any. - if (bandwidth_recorder != nullptr && bandwidth_recorder->HasEstimate()) { - const int32_t bw_estimate_bytes_per_second = - BandwidthToCachedParameterBytesPerSecond( - bandwidth_recorder->BandwidthEstimate()); - const int32_t max_bw_estimate_bytes_per_second = - BandwidthToCachedParameterBytesPerSecond( - bandwidth_recorder->MaxBandwidthEstimate()); - QUIC_BUG_IF(quic_bug_12513_1, max_bw_estimate_bytes_per_second < 0) - << max_bw_estimate_bytes_per_second; - QUIC_BUG_IF(quic_bug_10393_1, bw_estimate_bytes_per_second < 0) - << bw_estimate_bytes_per_second; + // Populate bandwidth estimates if any. + if (bandwidth_recorder != nullptr && bandwidth_recorder->HasEstimate()) { + const int32_t bw_estimate_bytes_per_second = + BandwidthToCachedParameterBytesPerSecond( + bandwidth_recorder->BandwidthEstimate()); + const int32_t max_bw_estimate_bytes_per_second = + BandwidthToCachedParameterBytesPerSecond( + bandwidth_recorder->MaxBandwidthEstimate()); + QUIC_BUG_IF(quic_bug_12513_1, max_bw_estimate_bytes_per_second < 0) + << max_bw_estimate_bytes_per_second; + QUIC_BUG_IF(quic_bug_10393_1, bw_estimate_bytes_per_second < 0) + << bw_estimate_bytes_per_second; - cached_network_params.set_bandwidth_estimate_bytes_per_second( - bw_estimate_bytes_per_second); - cached_network_params.set_max_bandwidth_estimate_bytes_per_second( - max_bw_estimate_bytes_per_second); - cached_network_params.set_max_bandwidth_timestamp_seconds( - bandwidth_recorder->MaxBandwidthTimestamp()); + cached_network_params.set_bandwidth_estimate_bytes_per_second( + bw_estimate_bytes_per_second); + cached_network_params.set_max_bandwidth_estimate_bytes_per_second( + max_bw_estimate_bytes_per_second); + cached_network_params.set_max_bandwidth_timestamp_seconds( + bandwidth_recorder->MaxBandwidthTimestamp()); - cached_network_params.set_previous_connection_state( - bandwidth_recorder->EstimateRecordedDuringSlowStart() - ? CachedNetworkParameters::SLOW_START - : CachedNetworkParameters::CONGESTION_AVOIDANCE); - } + cached_network_params.set_previous_connection_state( + bandwidth_recorder->EstimateRecordedDuringSlowStart() + ? CachedNetworkParameters::SLOW_START + : CachedNetworkParameters::CONGESTION_AVOIDANCE); } if (!serving_region_.empty()) {
diff --git a/quiche/quic/core/http/quic_server_session_base.h b/quiche/quic/core/http/quic_server_session_base.h index a06cb14..f70e15a 100644 --- a/quiche/quic/core/http/quic_server_session_base.h +++ b/quiche/quic/core/http/quic_server_session_base.h
@@ -75,10 +75,6 @@ QuicSSLConfig GetSSLConfig() const override; - bool enable_sending_bandwidth_estimate_when_network_idle() const { - return enable_sending_bandwidth_estimate_when_network_idle_; - } - protected: // QuicSession methods(override them with return type of QuicSpdyStream*): QuicCryptoServerStreamBase* GetMutableCryptoStream() override; @@ -152,8 +148,6 @@ // should go away once we fix http://b//27897982 int32_t BandwidthToCachedParameterBytesPerSecond( const QuicBandwidth& bandwidth) const; - - bool enable_sending_bandwidth_estimate_when_network_idle_ = false; }; } // namespace quic
diff --git a/quiche/quic/core/http/quic_server_session_base_test.cc b/quiche/quic/core/http/quic_server_session_base_test.cc index 058ae5b..7a1c952 100644 --- a/quiche/quic/core/http/quic_server_session_base_test.cc +++ b/quiche/quic/core/http/quic_server_session_base_test.cc
@@ -607,40 +607,35 @@ HAS_RETRANSMITTABLE_DATA, true, ECN_NOT_ECT); - if (GetQuicRestartFlag( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2)) { - EXPECT_CALL(*connection_, OnSendConnectionState(_)).Times(0); - } else { - // Verify that the proto has exactly the values we expect. - CachedNetworkParameters expected_network_params; - expected_network_params.set_bandwidth_estimate_bytes_per_second( - bandwidth_recorder.BandwidthEstimate().ToBytesPerSecond()); - expected_network_params.set_max_bandwidth_estimate_bytes_per_second( - bandwidth_recorder.MaxBandwidthEstimate().ToBytesPerSecond()); - expected_network_params.set_max_bandwidth_timestamp_seconds( - bandwidth_recorder.MaxBandwidthTimestamp()); - expected_network_params.set_min_rtt_ms(session_->connection() - ->sent_packet_manager() - .GetRttStats() - ->min_rtt() - .ToMilliseconds()); - expected_network_params.set_previous_connection_state( - CachedNetworkParameters::CONGESTION_AVOIDANCE); - expected_network_params.set_timestamp( - session_->connection()->clock()->WallNow().ToUNIXSeconds()); - expected_network_params.set_serving_region(serving_region); + // Verify that the proto has exactly the values we expect. + CachedNetworkParameters expected_network_params; + expected_network_params.set_bandwidth_estimate_bytes_per_second( + bandwidth_recorder.BandwidthEstimate().ToBytesPerSecond()); + expected_network_params.set_max_bandwidth_estimate_bytes_per_second( + bandwidth_recorder.MaxBandwidthEstimate().ToBytesPerSecond()); + expected_network_params.set_max_bandwidth_timestamp_seconds( + bandwidth_recorder.MaxBandwidthTimestamp()); + expected_network_params.set_min_rtt_ms(session_->connection() + ->sent_packet_manager() + .GetRttStats() + ->min_rtt() + .ToMilliseconds()); + expected_network_params.set_previous_connection_state( + CachedNetworkParameters::CONGESTION_AVOIDANCE); + expected_network_params.set_timestamp( + session_->connection()->clock()->WallNow().ToUNIXSeconds()); + expected_network_params.set_serving_region(serving_region); - if (quic_crypto_stream) { - EXPECT_CALL(*quic_crypto_stream, - SendServerConfigUpdate(EqualsProto(expected_network_params))) - .Times(1); - } else { - EXPECT_CALL(*tls_server_stream, - GetAddressToken(EqualsProto(expected_network_params))) - .WillOnce(testing::Return("Test address token")); - } - EXPECT_CALL(*connection_, OnSendConnectionState(_)).Times(1); + if (quic_crypto_stream) { + EXPECT_CALL(*quic_crypto_stream, + SendServerConfigUpdate(EqualsProto(expected_network_params))) + .Times(1); + } else { + EXPECT_CALL(*tls_server_stream, + GetAddressToken(EqualsProto(expected_network_params))) + .WillOnce(testing::Return("Test address token")); } + EXPECT_CALL(*connection_, OnSendConnectionState(_)).Times(1); session_->OnCongestionWindowChange(now); }
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index c1cc294..1e1a93e 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -79,8 +79,6 @@ QUIC_FLAG(quic_reloadable_flag_quic_priority_respect_incremental, false) // If true, round-robin stream writes instead of batching in QuicWriteBlockedList. QUIC_FLAG(quic_reloadable_flag_quic_disable_batch_write, false) -// If true, server sends bandwidth eastimate when network is idle for a while. -QUIC_FLAG(quic_restart_flag_quic_enable_sending_bandwidth_estimate_when_network_idle_v2, false) // If true, set burst token to 2 in cwnd bootstrapping experiment. QUIC_FLAG(quic_reloadable_flag_quic_conservative_bursts, false) // If true, use BBRv2 as the default congestion controller. Takes precedence over --quic_default_to_bbr.
diff --git a/quiche/quic/core/quic_idle_network_detector.cc b/quiche/quic/core/quic_idle_network_detector.cc index 5a63ebc..acac42a 100644 --- a/quiche/quic/core/quic_idle_network_detector.cc +++ b/quiche/quic/core/quic_idle_network_detector.cc
@@ -73,14 +73,6 @@ idle_network_timeout_ = idle_network_timeout; bandwidth_update_timeout_ = QuicTime::Delta::Infinite(); - if (GetQuicRestartFlag( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2) && - handshake_timeout_.IsInfinite()) { - QUIC_RESTART_FLAG_COUNT_N( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2, 1, 3); - bandwidth_update_timeout_ = idle_network_timeout_ * 0.5; - } - SetAlarm(); }
diff --git a/quiche/quic/core/quic_idle_network_detector.h b/quiche/quic/core/quic_idle_network_detector.h index bf13d62..7000ce1 100644 --- a/quiche/quic/core/quic_idle_network_detector.h +++ b/quiche/quic/core/quic_idle_network_detector.h
@@ -114,6 +114,7 @@ // Idle network timeout. Infinite means no idle network timeout. QuicTime::Delta idle_network_timeout_; + // TODO(b/296840230) Remove this field. // Bandwidth update timeout. Infinite means no bandwidth update timeout. QuicTime::Delta bandwidth_update_timeout_;
diff --git a/quiche/quic/core/quic_idle_network_detector_test.cc b/quiche/quic/core/quic_idle_network_detector_test.cc index a139eb2..6b9e33e 100644 --- a/quiche/quic/core/quic_idle_network_detector_test.cc +++ b/quiche/quic/core/quic_idle_network_detector_test.cc
@@ -102,30 +102,11 @@ detector_->SetTimeouts( /*handshake_timeout=*/QuicTime::Delta::Infinite(), /*idle_network_timeout=*/QuicTime::Delta::FromSeconds(600)); - if (!GetQuicRestartFlag( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2)) { - EXPECT_EQ(clock_.Now() + QuicTime::Delta::FromSeconds(600), - alarm_->deadline()); - - // No network activity for 600s. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(600)); - EXPECT_CALL(delegate_, OnIdleNetworkDetected()); - alarm_->Fire(); - return; - } - - EXPECT_EQ(clock_.Now() + QuicTime::Delta::FromSeconds(300), - alarm_->deadline()); - - // No network activity for 300s. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(300)); - EXPECT_CALL(delegate_, OnBandwidthUpdateTimeout()); - alarm_->Fire(); - EXPECT_EQ(clock_.Now() + QuicTime::Delta::FromSeconds(300), + EXPECT_EQ(clock_.Now() + QuicTime::Delta::FromSeconds(600), alarm_->deadline()); // No network activity for 600s. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(300)); + clock_.AdvanceTime(QuicTime::Delta::FromSeconds(600)); EXPECT_CALL(delegate_, OnIdleNetworkDetected()); alarm_->Fire(); } @@ -139,16 +120,11 @@ EXPECT_TRUE(alarm_->IsSet()); // Handshake completes in 200ms. - const bool enable_sending_bandwidth_estimate_when_network_idle = - GetQuicRestartFlag( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2); clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(200)); detector_->OnPacketReceived(clock_.Now()); detector_->SetTimeouts( /*handshake_timeout=*/QuicTime::Delta::Infinite(), - enable_sending_bandwidth_estimate_when_network_idle - ? QuicTime::Delta::FromSeconds(1200) - : QuicTime::Delta::FromSeconds(600)); + QuicTime::Delta::FromSeconds(600)); EXPECT_EQ(clock_.Now() + QuicTime::Delta::FromSeconds(600), alarm_->deadline()); @@ -166,27 +142,9 @@ EXPECT_EQ(packet_sent_time + QuicTime::Delta::FromSeconds(600), alarm_->deadline()); - if (!enable_sending_bandwidth_estimate_when_network_idle) { - // No network activity for 600s. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(600) - - QuicTime::Delta::FromMilliseconds(200)); - EXPECT_CALL(delegate_, OnIdleNetworkDetected()); - alarm_->Fire(); - return; - } - - // Bandwidth update times out after no network activity for 600s. + // No network activity for 600s. clock_.AdvanceTime(QuicTime::Delta::FromSeconds(600) - QuicTime::Delta::FromMilliseconds(200)); - EXPECT_CALL(delegate_, OnBandwidthUpdateTimeout()); - alarm_->Fire(); - EXPECT_TRUE(alarm_->IsSet()); - EXPECT_EQ(packet_sent_time + QuicTime::Delta::FromSeconds(1200), - alarm_->deadline()); - - // Network idle time out after no network activity for 1200s. - clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1200) - - QuicTime::Delta::FromMilliseconds(600)); EXPECT_CALL(delegate_, OnIdleNetworkDetected()); alarm_->Fire(); } @@ -194,12 +152,7 @@ TEST_F(QuicIdleNetworkDetectorTest, ShorterIdleTimeoutOnSentPacket) { detector_->enable_shorter_idle_timeout_on_sent_packet(); QuicTime::Delta idle_network_timeout = QuicTime::Delta::Zero(); - if (GetQuicRestartFlag( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2)) { - idle_network_timeout = QuicTime::Delta::FromSeconds(60); - } else { - idle_network_timeout = QuicTime::Delta::FromSeconds(30); - } + idle_network_timeout = QuicTime::Delta::FromSeconds(30); detector_->SetTimeouts( /*handshake_timeout=*/QuicTime::Delta::Infinite(), idle_network_timeout); EXPECT_TRUE(alarm_->IsSet()); @@ -247,35 +200,6 @@ EXPECT_FALSE(alarm_->IsSet()); } -TEST_F(QuicIdleNetworkDetectorTest, - ResetBandwidthTimeoutWhenHandshakeTimeoutIsSet) { - if (!GetQuicRestartFlag( - quic_enable_sending_bandwidth_estimate_when_network_idle_v2)) { - return; - } - detector_->SetTimeouts( - /*handshake_timeout=*/QuicTime::Delta::Infinite(), - /*idle_network_timeout=*/QuicTime::Delta::FromSeconds(20)); - // The deadline is set based on the bandwidth timeout. - EXPECT_EQ(clock_.Now() + QuicTime::Delta::FromSeconds(10), - alarm_->deadline()); - - detector_->SetTimeouts( - /*handshake_timeout=*/QuicTime::Delta::FromSeconds(15), - /*idle_network_timeout=*/QuicTime::Delta::FromSeconds(20)); - // Bandwidth timeout is reset and the deadline is set based on the handshake - // timeout. - EXPECT_EQ(clock_.Now() + QuicTime::Delta::FromSeconds(15), - alarm_->deadline()); - - detector_->SetTimeouts( - /*handshake_timeout=*/QuicTime::Delta::Infinite(), - /*idle_network_timeout=*/QuicTime::Delta::FromSeconds(20)); - // The deadline is set based on the bandwidth timeout. - EXPECT_EQ(clock_.Now() + QuicTime::Delta::FromSeconds(10), - alarm_->deadline()); -} - } // namespace } // namespace test