gfe-relnote: Deprecate unverified flag quic_bbr_less_probe_rtt false.
This is currently only used by Quartc, but we're hoping to migrate to BBRv2, where these connection options currently have no effect and some of the options are implemented in part(ie: Only reduce CWND to 1/2 BDP for PROBE_RTT).
If the COPTs behind this flag are used or planned to be used in production, then I can delay this CL until BBRv2 is 'done', but ideally I'd like to clean them up now if I can.
PiperOrigin-RevId: 290152900
Change-Id: If18f613f08ec1d44cd54d1857f64fda66bb90cf3
diff --git a/quic/core/congestion_control/bbr_sender.cc b/quic/core/congestion_control/bbr_sender.cc
index 358006d..e0be6b2 100644
--- a/quic/core/congestion_control/bbr_sender.cc
+++ b/quic/core/congestion_control/bbr_sender.cc
@@ -133,11 +133,6 @@
enable_ack_aggregation_during_startup_(false),
expire_ack_aggregation_in_startup_(false),
drain_to_target_(false),
- probe_rtt_based_on_bdp_(false),
- probe_rtt_skipped_if_similar_rtt_(false),
- probe_rtt_disabled_if_app_limited_(false),
- app_limited_since_last_probe_rtt_(false),
- min_rtt_since_last_probe_rtt_(QuicTime::Delta::Infinite()),
network_parameters_adjusted_(false),
bytes_lost_with_network_parameters_adjusted_(0),
bytes_lost_multiplier_with_network_parameters_adjusted_(2),
@@ -309,21 +304,6 @@
if (config.HasClientRequestedIndependentOption(kBBR5, perspective)) {
sampler_.SetMaxAckHeightTrackerWindowLength(4 * kBandwidthWindowSize);
}
- if (GetQuicReloadableFlag(quic_bbr_less_probe_rtt) &&
- config.HasClientRequestedIndependentOption(kBBR6, perspective)) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_bbr_less_probe_rtt, 1, 3);
- probe_rtt_based_on_bdp_ = true;
- }
- if (GetQuicReloadableFlag(quic_bbr_less_probe_rtt) &&
- config.HasClientRequestedIndependentOption(kBBR7, perspective)) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_bbr_less_probe_rtt, 2, 3);
- probe_rtt_skipped_if_similar_rtt_ = true;
- }
- if (GetQuicReloadableFlag(quic_bbr_less_probe_rtt) &&
- config.HasClientRequestedIndependentOption(kBBR8, perspective)) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_bbr_less_probe_rtt, 3, 3);
- probe_rtt_disabled_if_app_limited_ = true;
- }
if (GetQuicReloadableFlag(quic_bbr_flexible_app_limited) &&
config.HasClientRequestedIndependentOption(kBBR9, perspective)) {
QUIC_RELOADABLE_FLAG_COUNT(quic_bbr_flexible_app_limited);
@@ -543,9 +523,6 @@
}
QuicByteCount BbrSender::ProbeRttCongestionWindow() const {
- if (probe_rtt_based_on_bdp_) {
- return GetTargetCongestionWindow(kModerateProbeRttMultiplier);
- }
return min_congestion_window_;
}
@@ -642,9 +619,6 @@
bool BbrSender::MaybeUpdateMinRtt(QuicTime now,
QuicTime::Delta sample_min_rtt) {
- min_rtt_since_last_probe_rtt_ =
- std::min(min_rtt_since_last_probe_rtt_, sample_min_rtt);
-
// Do not expire min_rtt if none was ever available.
bool min_rtt_expired =
!min_rtt_.IsZero() && (now > (min_rtt_timestamp_ + kMinRttExpiry));
@@ -654,38 +628,14 @@
<< ", new value: " << sample_min_rtt
<< ", current time: " << now.ToDebuggingValue();
- if (min_rtt_expired && ShouldExtendMinRttExpiry()) {
- min_rtt_expired = false;
- } else {
- min_rtt_ = sample_min_rtt;
- }
+ min_rtt_ = sample_min_rtt;
min_rtt_timestamp_ = now;
- // Reset since_last_probe_rtt fields.
- min_rtt_since_last_probe_rtt_ = QuicTime::Delta::Infinite();
- app_limited_since_last_probe_rtt_ = false;
}
DCHECK(!min_rtt_.IsZero());
return min_rtt_expired;
}
-bool BbrSender::ShouldExtendMinRttExpiry() const {
- if (probe_rtt_disabled_if_app_limited_ && app_limited_since_last_probe_rtt_) {
- // Extend the current min_rtt if we've been app limited recently.
- return true;
- }
- const bool min_rtt_increased_since_last_probe =
- min_rtt_since_last_probe_rtt_ > min_rtt_ * kSimilarMinRttThreshold;
- if (probe_rtt_skipped_if_similar_rtt_ && app_limited_since_last_probe_rtt_ &&
- !min_rtt_increased_since_last_probe) {
- // Extend the current min_rtt if we've been app limited recently and an rtt
- // has been measured in that time that's less than 12.5% more than the
- // current min_rtt.
- return true;
- }
- return false;
-}
-
void BbrSender::UpdateGainCyclePhase(QuicTime now,
QuicByteCount prior_in_flight,
bool has_losses) {
@@ -1065,7 +1015,6 @@
return;
}
- app_limited_since_last_probe_rtt_ = true;
sampler_.OnAppLimited();
QUIC_DVLOG(2) << "Becoming application limited. Last sent packet: "
<< last_sent_packet_ << ", CWND: " << GetCongestionWindow();
diff --git a/quic/core/congestion_control/bbr_sender.h b/quic/core/congestion_control/bbr_sender.h
index fa50de8..4e348f3 100644
--- a/quic/core/congestion_control/bbr_sender.h
+++ b/quic/core/congestion_control/bbr_sender.h
@@ -392,19 +392,6 @@
// or it's time for high gain mode.
bool drain_to_target_;
- // If true, use a CWND of 0.75*BDP during probe_rtt instead of 4 packets.
- bool probe_rtt_based_on_bdp_;
- // If true, skip probe_rtt and update the timestamp of the existing min_rtt to
- // now if min_rtt over the last cycle is within 12.5% of the current min_rtt.
- // Even if the min_rtt is 12.5% too low, the 25% gain cycling and 2x CWND gain
- // should overcome an overly small min_rtt.
- bool probe_rtt_skipped_if_similar_rtt_;
- // If true, disable PROBE_RTT entirely as long as the connection was recently
- // app limited.
- bool probe_rtt_disabled_if_app_limited_;
- bool app_limited_since_last_probe_rtt_;
- QuicTime::Delta min_rtt_since_last_probe_rtt_;
-
// True if network parameters are adjusted, and this will be reset if
// overshooting is detected and pacing rate gets slowed.
bool network_parameters_adjusted_;
diff --git a/quic/core/congestion_control/bbr_sender_test.cc b/quic/core/congestion_control/bbr_sender_test.cc
index beb4bff..78d71a9 100644
--- a/quic/core/congestion_control/bbr_sender_test.cc
+++ b/quic/core/congestion_control/bbr_sender_test.cc
@@ -788,77 +788,6 @@
EXPECT_GE(sender_->ExportDebugState().min_rtt_timestamp, probe_rtt_start);
}
-// Verify that the connection enters and exits PROBE_RTT correctly.
-TEST_F(BbrSenderTest, ProbeRttBDPBasedCWNDTarget) {
- CreateDefaultSetup();
- SetQuicReloadableFlag(quic_bbr_less_probe_rtt, true);
- SetConnectionOption(kBBR6);
- DriveOutOfStartup();
-
- // We have no intention of ever finishing this transfer.
- bbr_sender_.AddBytesToTransfer(100 * 1024 * 1024);
-
- // Wait until the connection enters PROBE_RTT.
- const QuicTime::Delta timeout = QuicTime::Delta::FromSeconds(12);
- bool simulator_result = simulator_.RunUntilOrTimeout(
- [this]() {
- return sender_->ExportDebugState().mode == BbrSender::PROBE_RTT;
- },
- timeout);
- ASSERT_TRUE(simulator_result);
- ASSERT_EQ(BbrSender::PROBE_RTT, sender_->ExportDebugState().mode);
-
- // Exit PROBE_RTT.
- const QuicTime probe_rtt_start = clock_->Now();
- const QuicTime::Delta time_to_exit_probe_rtt =
- kTestRtt + QuicTime::Delta::FromMilliseconds(200);
- simulator_.RunFor(1.5 * time_to_exit_probe_rtt);
- EXPECT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode);
- EXPECT_GE(sender_->ExportDebugState().min_rtt_timestamp, probe_rtt_start);
-}
-
-// Verify that the connection enters does not enter PROBE_RTT.
-TEST_F(BbrSenderTest, ProbeRttSkippedAfterAppLimitedAndStableRtt) {
- CreateDefaultSetup();
- SetQuicReloadableFlag(quic_bbr_less_probe_rtt, true);
- SetConnectionOption(kBBR7);
- DriveOutOfStartup();
-
- // We have no intention of ever finishing this transfer.
- bbr_sender_.AddBytesToTransfer(100 * 1024 * 1024);
-
- // Wait until the connection enters PROBE_RTT.
- const QuicTime::Delta timeout = QuicTime::Delta::FromSeconds(12);
- bool simulator_result = simulator_.RunUntilOrTimeout(
- [this]() {
- return sender_->ExportDebugState().mode == BbrSender::PROBE_RTT;
- },
- timeout);
- ASSERT_FALSE(simulator_result);
- ASSERT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode);
-}
-
-// Verify that the connection enters does not enter PROBE_RTT.
-TEST_F(BbrSenderTest, ProbeRttSkippedAfterAppLimited) {
- CreateDefaultSetup();
- SetQuicReloadableFlag(quic_bbr_less_probe_rtt, true);
- SetConnectionOption(kBBR8);
- DriveOutOfStartup();
-
- // We have no intention of ever finishing this transfer.
- bbr_sender_.AddBytesToTransfer(100 * 1024 * 1024);
-
- // Wait until the connection enters PROBE_RTT.
- const QuicTime::Delta timeout = QuicTime::Delta::FromSeconds(12);
- bool simulator_result = simulator_.RunUntilOrTimeout(
- [this]() {
- return sender_->ExportDebugState().mode == BbrSender::PROBE_RTT;
- },
- timeout);
- ASSERT_FALSE(simulator_result);
- ASSERT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode);
-}
-
// Ensure that a connection that is app-limited and is at sufficiently low
// bandwidth will not exit high gain phase, and similarly ensure that the
// connection will exit low gain early if the number of bytes in flight is low.
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index a1d7d76..9400a5e 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -99,11 +99,6 @@
// per cycle
const QuicTag kBBR4 = TAG('B', 'B', 'R', '4'); // 20 RTT ack aggregation
const QuicTag kBBR5 = TAG('B', 'B', 'R', '5'); // 40 RTT ack aggregation
-const QuicTag kBBR6 = TAG('B', 'B', 'R', '6'); // PROBE_RTT with 0.75 * BDP
-const QuicTag kBBR7 = TAG('B', 'B', 'R', '7'); // Skip PROBE_RTT if rtt has
- // not changed 12.5%
-const QuicTag kBBR8 = TAG('B', 'B', 'R', '8'); // Disable PROBE_RTT when
- // recently app-limited
const QuicTag kBBR9 = TAG('B', 'B', 'R', '9'); // Ignore app-limited calls in
// BBR if enough inflight.
const QuicTag kBBRS = TAG('B', 'B', 'R', 'S'); // Use 1.5x pacing in startup
diff --git a/quic/quartc/quartc_factory.cc b/quic/quartc/quartc_factory.cc
index f5815ea..ff98761 100644
--- a/quic/quartc/quartc_factory.cc
+++ b/quic/quartc/quartc_factory.cc
@@ -75,7 +75,6 @@
// Note: flag settings have no effect for Exoblaze builds since
// SetQuicReloadableFlag() gets stubbed out.
- SetQuicReloadableFlag(quic_bbr_less_probe_rtt, true); // Enable BBR6,7,8.
SetQuicReloadableFlag(quic_unified_iw_options, true); // Enable IWXX opts.
SetQuicReloadableFlag(quic_bbr_flexible_app_limited, true); // Enable BBR9.
}
@@ -106,8 +105,6 @@
copt.push_back(kBBR3); // Stay in low-gain until in-flight < BDP.
copt.push_back(kBBR5); // 40 RTT ack aggregation.
- copt.push_back(kBBR6); // Use a 0.75 * BDP cwnd during PROBE_RTT.
- copt.push_back(kBBR8); // Skip PROBE_RTT if app-limited.
copt.push_back(kBBR9); // Ignore app-limited if enough data is in flight.
copt.push_back(kBBQ1); // 2.773 pacing gain in STARTUP.
copt.push_back(kBBQ2); // 2.0 CWND gain in STARTUP.
