In MaxAckHeightTracker, always starts a new ack aggregation epoch if a full round has passed since the start of the current epoch. Enabled by the BBRA connection option.
Protected by FLAGS_quic_reloadable_flag_quic_bbr_start_new_aggregation_epoch_after_a_full_round.
PiperOrigin-RevId: 398548798
diff --git a/quic/core/congestion_control/bandwidth_sampler.cc b/quic/core/congestion_control/bandwidth_sampler.cc
index 657cad1..81f9e91 100644
--- a/quic/core/congestion_control/bandwidth_sampler.cc
+++ b/quic/core/congestion_control/bandwidth_sampler.cc
@@ -23,13 +23,31 @@
return os;
}
-QuicByteCount MaxAckHeightTracker::Update(QuicBandwidth bandwidth_estimate,
- QuicRoundTripCount round_trip_count,
- QuicTime ack_time,
- QuicByteCount bytes_acked) {
- if (aggregation_epoch_start_time_ == QuicTime::Zero()) {
+QuicByteCount MaxAckHeightTracker::Update(
+ QuicBandwidth bandwidth_estimate, QuicRoundTripCount round_trip_count,
+ QuicPacketNumber last_sent_packet_number,
+ QuicPacketNumber last_acked_packet_number, QuicTime ack_time,
+ QuicByteCount bytes_acked) {
+ bool force_new_epoch = false;
+
+ // If any packet sent after the start of the epoch has been acked, start a new
+ // epoch.
+ if (start_new_aggregation_epoch_after_full_round_ &&
+ last_sent_packet_number_before_epoch_.IsInitialized() &&
+ last_acked_packet_number.IsInitialized() &&
+ last_acked_packet_number > last_sent_packet_number_before_epoch_) {
+ QUIC_RELOADABLE_FLAG_COUNT(
+ quic_bbr_start_new_aggregation_epoch_after_a_full_round);
+ QUIC_DVLOG(3) << "Force starting a new aggregation epoch. "
+ "last_sent_packet_number_before_epoch_:"
+ << last_sent_packet_number_before_epoch_
+ << ", last_acked_packet_number:" << last_acked_packet_number;
+ force_new_epoch = true;
+ }
+ if (aggregation_epoch_start_time_ == QuicTime::Zero() || force_new_epoch) {
aggregation_epoch_bytes_ = bytes_acked;
aggregation_epoch_start_time_ = ack_time;
+ last_sent_packet_number_before_epoch_ = last_sent_packet_number;
++num_ack_aggregation_epochs_;
return 0;
}
@@ -57,6 +75,7 @@
// Reset to start measuring a new aggregation epoch.
aggregation_epoch_bytes_ = bytes_acked;
aggregation_epoch_start_time_ = ack_time;
+ last_sent_packet_number_before_epoch_ = last_sent_packet_number;
++num_ack_aggregation_epochs_;
return 0;
}
@@ -67,7 +86,7 @@
QuicByteCount extra_bytes_acked =
aggregation_epoch_bytes_ - expected_bytes_acked;
QUIC_DVLOG(3) << "Updating MaxAckHeight. ack_time:" << ack_time
- << ", round trip count:" << round_trip_count
+ << ", last sent packet:" << last_sent_packet_number
<< ", bandwidth_estimate:" << bandwidth_estimate
<< ", bytes_acked:" << bytes_acked
<< ", expected_bytes_acked:" << expected_bytes_acked
@@ -105,6 +124,7 @@
last_acked_packet_sent_time_(other.last_acked_packet_sent_time_),
last_acked_packet_ack_time_(other.last_acked_packet_ack_time_),
last_sent_packet_(other.last_sent_packet_),
+ last_acked_packet_(other.last_acked_packet_),
is_app_limited_(other.is_app_limited_),
end_of_app_limited_phase_(other.end_of_app_limited_phase_),
connection_state_map_(other.connection_state_map_),
@@ -301,8 +321,8 @@
total_bytes_acked_after_last_ack_event_ = total_bytes_acked_;
QuicByteCount extra_acked = max_ack_height_tracker_.Update(
- bandwidth_estimate, round_trip_count, last_acked_packet_ack_time_,
- newly_acked_bytes);
+ bandwidth_estimate, round_trip_count, last_sent_packet_,
+ last_acked_packet_, last_acked_packet_ack_time_, newly_acked_bytes);
// If |extra_acked| is zero, i.e. this ack event marks the start of a new ack
// aggregation epoch, save LessRecentPoint, which is the last ack point of the
// previous epoch, as a A0 candidate.
@@ -316,6 +336,7 @@
BandwidthSample BandwidthSampler::OnPacketAcknowledged(
QuicTime ack_time,
QuicPacketNumber packet_number) {
+ last_acked_packet_ = packet_number;
ConnectionStateOnSentPacket* sent_packet_pointer =
connection_state_map_.GetEntry(packet_number);
if (sent_packet_pointer == nullptr) {
diff --git a/quic/core/congestion_control/bandwidth_sampler.h b/quic/core/congestion_control/bandwidth_sampler.h
index 2d5924b..4c72939 100644
--- a/quic/core/congestion_control/bandwidth_sampler.h
+++ b/quic/core/congestion_control/bandwidth_sampler.h
@@ -9,6 +9,7 @@
#include "quic/core/congestion_control/windowed_filter.h"
#include "quic/core/packet_number_indexed_queue.h"
#include "quic/core/quic_bandwidth.h"
+#include "quic/core/quic_packet_number.h"
#include "quic/core/quic_packets.h"
#include "quic/core/quic_time.h"
#include "quic/core/quic_types.h"
@@ -104,8 +105,9 @@
QuicByteCount Update(QuicBandwidth bandwidth_estimate,
QuicRoundTripCount round_trip_count,
- QuicTime ack_time,
- QuicByteCount bytes_acked);
+ QuicPacketNumber last_sent_packet_number,
+ QuicPacketNumber last_acked_packet_number,
+ QuicTime ack_time, QuicByteCount bytes_acked);
void SetFilterWindowLength(QuicRoundTripCount length) {
max_ack_height_filter_.SetWindowLength(length);
@@ -119,6 +121,10 @@
ack_aggregation_bandwidth_threshold_ = threshold;
}
+ void SetStartNewAggregationEpochAfterFullRound(bool value) {
+ start_new_aggregation_epoch_after_full_round_ = value;
+ }
+
double ack_aggregation_bandwidth_threshold() const {
return ack_aggregation_bandwidth_threshold_;
}
@@ -139,11 +145,14 @@
// The time this aggregation started and the number of bytes acked during it.
QuicTime aggregation_epoch_start_time_ = QuicTime::Zero();
QuicByteCount aggregation_epoch_bytes_ = 0;
+ // The last sent packet number before the current aggregation epoch started.
+ QuicPacketNumber last_sent_packet_number_before_epoch_;
// The number of ack aggregation epochs ever started, including the ongoing
// one. Stats only.
uint64_t num_ack_aggregation_epochs_ = 0;
double ack_aggregation_bandwidth_threshold_ =
GetQuicFlag(FLAGS_quic_ack_aggregation_bandwidth_threshold);
+ bool start_new_aggregation_epoch_after_full_round_ = false;
};
// An interface common to any class that can provide bandwidth samples from the
@@ -355,6 +364,10 @@
max_ack_height_tracker_.Reset(new_height, new_time);
}
+ void SetStartNewAggregationEpochAfterFullRound(bool value) {
+ max_ack_height_tracker_.SetStartNewAggregationEpochAfterFullRound(value);
+ }
+
// AckPoint represents a point on the ack line.
struct QUIC_NO_EXPORT AckPoint {
QuicTime ack_time = QuicTime::Zero();
@@ -528,6 +541,9 @@
// The most recently sent packet.
QuicPacketNumber last_sent_packet_;
+ // The most recently acked packet.
+ QuicPacketNumber last_acked_packet_;
+
// Indicates whether the bandwidth sampler is currently in an app-limited
// phase.
bool is_app_limited_;
diff --git a/quic/core/congestion_control/bandwidth_sampler_test.cc b/quic/core/congestion_control/bandwidth_sampler_test.cc
index 67dad1c..a371079 100644
--- a/quic/core/congestion_control/bandwidth_sampler_test.cc
+++ b/quic/core/congestion_control/bandwidth_sampler_test.cc
@@ -685,22 +685,23 @@
QuicBandwidth::FromBytesAndTimeDelta(kRegularPacketSize,
time_between_packets);
- // Send and ack packet 1.
+ // Send packets 1 to 4 and ack packet 1.
SendPacket(1);
clock_.AdvanceTime(time_between_packets);
+ SendPacket(2);
+ SendPacket(3);
+ SendPacket(4);
BandwidthSampler::CongestionEventSample sample = OnCongestionEvent({1}, {});
EXPECT_EQ(first_packet_sending_rate, sample.sample_max_bandwidth);
EXPECT_EQ(first_packet_sending_rate, max_bandwidth_);
- // Send and ack packet 2, 3 and 4.
+ // Ack packet 2, 3 and 4, all of which uses S(1) to calculate ack rate since
+ // there were no acks at the time they were sent.
round_trip_count_++;
est_bandwidth_upper_bound_ = first_packet_sending_rate * 0.3;
- SendPacket(2);
- SendPacket(3);
- SendPacket(4);
clock_.AdvanceTime(time_between_packets);
sample = OnCongestionEvent({2, 3, 4}, {});
- EXPECT_EQ(first_packet_sending_rate * 3, sample.sample_max_bandwidth);
+ EXPECT_EQ(first_packet_sending_rate * 2, sample.sample_max_bandwidth);
EXPECT_EQ(max_bandwidth_, sample.sample_max_bandwidth);
EXPECT_LT(2 * kRegularPacketSize, sample.extra_acked);
@@ -710,6 +711,11 @@
protected:
MaxAckHeightTrackerTest() : tracker_(/*initial_filter_window=*/10) {
tracker_.SetAckAggregationBandwidthThreshold(1.8);
+
+ if (GetQuicReloadableFlag(
+ quic_bbr_start_new_aggregation_epoch_after_a_full_round)) {
+ tracker_.SetStartNewAggregationEpochAfterFullRound(true);
+ }
}
// Run a full aggregation episode, which is one or more aggregated acks,
@@ -749,8 +755,9 @@
QuicByteCount last_extra_acked = 0;
for (QuicByteCount bytes = 0; bytes < aggregation_bytes;
bytes += bytes_per_ack) {
- QuicByteCount extra_acked =
- tracker_.Update(bandwidth_, RoundTripCount(), now_, bytes_per_ack);
+ QuicByteCount extra_acked = tracker_.Update(
+ bandwidth_, RoundTripCount(), last_sent_packet_number_,
+ last_acked_packet_number_, now_, bytes_per_ack);
QUIC_VLOG(1) << "T" << now_ << ": Update after " << bytes_per_ack
<< " bytes acked, " << extra_acked << " extra bytes acked";
// |extra_acked| should be 0 if either
@@ -784,6 +791,8 @@
QuicBandwidth bandwidth_ = QuicBandwidth::FromBytesPerSecond(10 * 1000);
QuicTime now_ = QuicTime::Zero() + QuicTime::Delta::FromMilliseconds(1);
QuicTime::Delta rtt_ = QuicTime::Delta::FromMilliseconds(60);
+ QuicPacketNumber last_sent_packet_number_;
+ QuicPacketNumber last_acked_packet_number_;
};
TEST_F(MaxAckHeightTrackerTest, VeryAggregatedLargeAck) {
@@ -864,5 +873,25 @@
EXPECT_LT(2u, tracker_.num_ack_aggregation_epochs());
}
+TEST_F(MaxAckHeightTrackerTest, StartNewEpochAfterAFullRound) {
+ last_sent_packet_number_ = QuicPacketNumber(10);
+ AggregationEpisode(bandwidth_ * 2, QuicTime::Delta::FromMilliseconds(50), 100,
+ true);
+
+ last_acked_packet_number_ = QuicPacketNumber(11);
+ // Update with a tiny bandwidth causes a very low expected bytes acked, which
+ // in turn causes the current epoch to continue if the |tracker_| doesn't
+ // check the packet numbers.
+ tracker_.Update(bandwidth_ * 0.1, RoundTripCount(), last_sent_packet_number_,
+ last_acked_packet_number_, now_, 100);
+
+ if (GetQuicReloadableFlag(
+ quic_bbr_start_new_aggregation_epoch_after_a_full_round)) {
+ EXPECT_EQ(2u, tracker_.num_ack_aggregation_epochs());
+ } else {
+ EXPECT_EQ(1u, tracker_.num_ack_aggregation_epochs());
+ }
+}
+
} // namespace test
} // namespace quic
diff --git a/quic/core/congestion_control/bbr2_misc.h b/quic/core/congestion_control/bbr2_misc.h
index 8234999..d745b11 100644
--- a/quic/core/congestion_control/bbr2_misc.h
+++ b/quic/core/congestion_control/bbr2_misc.h
@@ -411,6 +411,10 @@
return bandwidth_sampler_.num_ack_aggregation_epochs();
}
+ void SetStartNewAggregationEpochAfterFullRound(bool value) {
+ bandwidth_sampler_.SetStartNewAggregationEpochAfterFullRound(value);
+ }
+
bool MaybeExpireMinRtt(const Bbr2CongestionEvent& congestion_event);
QuicBandwidth BandwidthEstimate() const {
diff --git a/quic/core/congestion_control/bbr2_sender.cc b/quic/core/congestion_control/bbr2_sender.cc
index 4ff7a6a..cf75e31 100644
--- a/quic/core/congestion_control/bbr2_sender.cc
+++ b/quic/core/congestion_control/bbr2_sender.cc
@@ -165,6 +165,11 @@
quic_bbr2_check_cwnd_limited_before_aggregation_epoch);
params_.probe_bw_check_cwnd_limited_before_aggregation_epoch = true;
}
+ if (GetQuicReloadableFlag(
+ quic_bbr_start_new_aggregation_epoch_after_a_full_round) &&
+ ContainsQuicTag(connection_options, kBBRA)) {
+ model_.SetStartNewAggregationEpochAfterFullRound(true);
+ }
}
Limits<QuicByteCount> Bbr2Sender::GetCwndLimitsByMode() const {
diff --git a/quic/core/congestion_control/bbr2_simulator_test.cc b/quic/core/congestion_control/bbr2_simulator_test.cc
index 9157e1f..052e186 100644
--- a/quic/core/congestion_control/bbr2_simulator_test.cc
+++ b/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -207,6 +207,10 @@
// dedicated tests for this option.
SetConnectionOption(kB201, sender);
}
+ if (GetQuicReloadableFlag(
+ quic_bbr_start_new_aggregation_epoch_after_a_full_round)) {
+ SetConnectionOption(kBBRA, sender);
+ }
QuicConnectionPeer::SetSendAlgorithm(endpoint->connection(), sender);
endpoint->RecordTrace();
return sender;
diff --git a/quic/core/congestion_control/bbr_sender.cc b/quic/core/congestion_control/bbr_sender.cc
index 940fe26..4d6ae70 100644
--- a/quic/core/congestion_control/bbr_sender.cc
+++ b/quic/core/congestion_control/bbr_sender.cc
@@ -278,6 +278,11 @@
if (ContainsQuicTag(connection_options, kBSAO)) {
sampler_.EnableOverestimateAvoidance();
}
+ if (GetQuicReloadableFlag(
+ quic_bbr_start_new_aggregation_epoch_after_a_full_round) &&
+ ContainsQuicTag(connection_options, kBBRA)) {
+ sampler_.SetStartNewAggregationEpochAfterFullRound(true);
+ }
}
void BbrSender::AdjustNetworkParameters(const NetworkParams& params) {
diff --git a/quic/core/congestion_control/bbr_sender_test.cc b/quic/core/congestion_control/bbr_sender_test.cc
index 4a5cb0f..ca2adbd 100644
--- a/quic/core/congestion_control/bbr_sender_test.cc
+++ b/quic/core/congestion_control/bbr_sender_test.cc
@@ -10,6 +10,7 @@
#include <utility>
#include "quic/core/congestion_control/rtt_stats.h"
+#include "quic/core/crypto/crypto_protocol.h"
#include "quic/core/quic_bandwidth.h"
#include "quic/core/quic_packets.h"
#include "quic/core/quic_types.h"
@@ -110,6 +111,10 @@
{&receiver_, &competing_receiver_}) {
rtt_stats_ = bbr_sender_.connection()->sent_packet_manager().GetRttStats();
sender_ = SetupBbrSender(&bbr_sender_);
+ if (GetQuicReloadableFlag(
+ quic_bbr_start_new_aggregation_epoch_after_a_full_round)) {
+ SetConnectionOption(kBBRA);
+ }
clock_ = simulator_.GetClock();
}
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index 13ec7fd..6de2419 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -99,6 +99,9 @@
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 kBBR9 = TAG('B', 'B', 'R', '9'); // DEPRECATED
+const QuicTag kBBRA = TAG('B', 'B', 'R', 'A'); // Starts a new ack aggregation
+ // epoch if a full round has
+ // passed
const QuicTag kBBRS = TAG('B', 'B', 'R', 'S'); // DEPRECATED
const QuicTag kBBQ1 = TAG('B', 'B', 'Q', '1'); // BBR with lower 2.77 STARTUP
// pacing and CWND gain.
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index dfee754..f8fab11 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -37,6 +37,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_add_missing_update_ack_timeout, true)
// If true, allow client to enable BBRv2 on server via connection option \'B2ON\'.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_allow_client_enabled_bbr_v2, false)
+// If true, always starts a new ack aggregation epoch if a full round has passed since the start of the current epoch.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr_start_new_aggregation_epoch_after_a_full_round, true)
// If true, avoid calling reloadable flags in QuicVersionManager constructor by lazily initializing internal state.
QUIC_FLAG(FLAGS_quic_restart_flag_quic_lazy_quic_version_manager, true)
// If true, clear undecryptable packets on handshake complete.
