In quic loss detection tuner, do not give feedback if a connection 1) has no packets lost, 2) would still have no packets lost if the reordering_shift increased by one. protected by existing flag --gfe2_reloadable_flag_quic_enable_loss_detection_experiment_at_gfe.
PiperOrigin-RevId: 312830024
Change-Id: I76bc158d505792c31bf99fe040c7958418517173
diff --git a/quic/core/congestion_control/general_loss_algorithm.cc b/quic/core/congestion_control/general_loss_algorithm.cc
index f00045e..eb0ed77 100644
--- a/quic/core/congestion_control/general_loss_algorithm.cc
+++ b/quic/core/congestion_control/general_loss_algorithm.cc
@@ -56,7 +56,7 @@
QuicTime::Delta max_rtt =
std::max(rtt_stats.previous_srtt(), rtt_stats.latest_rtt());
max_rtt = std::max(kAlarmGranularity, max_rtt);
- QuicTime::Delta loss_delay = max_rtt + (max_rtt >> reordering_shift_);
+ const QuicTime::Delta loss_delay = max_rtt + (max_rtt >> reordering_shift_);
QuicPacketNumber packet_number = unacked_packets.GetLeastUnacked();
auto it = unacked_packets.begin();
if (least_in_flight_.IsInitialized() && least_in_flight_ >= packet_number) {
@@ -106,6 +106,10 @@
// Time threshold loss detection.
QuicTime when_lost = it->sent_time + loss_delay;
if (time < when_lost) {
+ if (time >=
+ it->sent_time + max_rtt + (max_rtt >> (reordering_shift_ + 1))) {
+ ++detection_stats.sent_packets_num_borderline_time_reorderings;
+ }
loss_detection_timeout_ = when_lost;
if (!least_in_flight_.IsInitialized()) {
// At this point, packet_number is in flight and not detected as lost.
diff --git a/quic/core/congestion_control/general_loss_algorithm_test.cc b/quic/core/congestion_control/general_loss_algorithm_test.cc
index 1a58ade..be33c25 100644
--- a/quic/core/congestion_control/general_loss_algorithm_test.cc
+++ b/quic/core/congestion_control/general_loss_algorithm_test.cc
@@ -62,14 +62,17 @@
const AckedPacketVector& packets_acked,
const std::vector<uint64_t>& losses_expected) {
return VerifyLosses(largest_newly_acked, packets_acked, losses_expected,
+ quiche::QuicheOptional<QuicPacketCount>(),
quiche::QuicheOptional<QuicPacketCount>());
}
- void VerifyLosses(uint64_t largest_newly_acked,
- const AckedPacketVector& packets_acked,
- const std::vector<uint64_t>& losses_expected,
- quiche::QuicheOptional<QuicPacketCount>
- max_sequence_reordering_expected) {
+ void VerifyLosses(
+ uint64_t largest_newly_acked,
+ const AckedPacketVector& packets_acked,
+ const std::vector<uint64_t>& losses_expected,
+ quiche::QuicheOptional<QuicPacketCount> max_sequence_reordering_expected,
+ quiche::QuicheOptional<QuicPacketCount>
+ num_borderline_time_reorderings_expected) {
unacked_packets_.MaybeUpdateLargestAckedOfPacketNumberSpace(
APPLICATION_DATA, QuicPacketNumber(largest_newly_acked));
LostPacketVector lost_packets;
@@ -80,6 +83,10 @@
EXPECT_EQ(stats.sent_packets_max_sequence_reordering,
max_sequence_reordering_expected.value());
}
+ if (num_borderline_time_reorderings_expected.has_value()) {
+ EXPECT_EQ(stats.sent_packets_num_borderline_time_reorderings,
+ num_borderline_time_reorderings_expected.value());
+ }
ASSERT_EQ(losses_expected.size(), lost_packets.size());
for (size_t i = 0; i < losses_expected.size(); ++i) {
EXPECT_EQ(lost_packets[i].packet_number,
@@ -104,19 +111,19 @@
unacked_packets_.RemoveFromInFlight(QuicPacketNumber(2));
packets_acked.push_back(AckedPacket(
QuicPacketNumber(2), kMaxOutgoingPacketSize, QuicTime::Zero()));
- VerifyLosses(2, packets_acked, std::vector<uint64_t>{}, 1);
+ VerifyLosses(2, packets_acked, std::vector<uint64_t>{}, 1, 0);
packets_acked.clear();
// No loss on two acks.
unacked_packets_.RemoveFromInFlight(QuicPacketNumber(3));
packets_acked.push_back(AckedPacket(
QuicPacketNumber(3), kMaxOutgoingPacketSize, QuicTime::Zero()));
- VerifyLosses(3, packets_acked, std::vector<uint64_t>{}, 2);
+ VerifyLosses(3, packets_acked, std::vector<uint64_t>{}, 2, 0);
packets_acked.clear();
// Loss on three acks.
unacked_packets_.RemoveFromInFlight(QuicPacketNumber(4));
packets_acked.push_back(AckedPacket(
QuicPacketNumber(4), kMaxOutgoingPacketSize, QuicTime::Zero()));
- VerifyLosses(4, packets_acked, {1}, 3);
+ VerifyLosses(4, packets_acked, {1}, 3, 0);
EXPECT_EQ(QuicTime::Zero(), loss_algorithm_.GetLossTimeout());
}
@@ -176,7 +183,12 @@
EXPECT_EQ(clock_.Now() + 1.25 * rtt_stats_.smoothed_rtt(),
loss_algorithm_.GetLossTimeout());
- clock_.AdvanceTime(1.25 * rtt_stats_.latest_rtt());
+ clock_.AdvanceTime(1.13 * rtt_stats_.latest_rtt());
+ // If reordering_shift increases by one we should have detected a loss.
+ VerifyLosses(2, packets_acked, {}, /*max_sequence_reordering_expected=*/1,
+ /*num_borderline_time_reorderings_expected=*/1);
+
+ clock_.AdvanceTime(0.13 * rtt_stats_.latest_rtt());
VerifyLosses(2, packets_acked, {1});
EXPECT_EQ(QuicTime::Zero(), loss_algorithm_.GetLossTimeout());
}
diff --git a/quic/core/congestion_control/loss_detection_interface.h b/quic/core/congestion_control/loss_detection_interface.h
index 8d91976..cd2bd72 100644
--- a/quic/core/congestion_control/loss_detection_interface.h
+++ b/quic/core/congestion_control/loss_detection_interface.h
@@ -29,6 +29,7 @@
struct QUIC_NO_EXPORT DetectionStats {
// Maximum sequence reordering observed in newly acked packets.
QuicPacketCount sent_packets_max_sequence_reordering = 0;
+ QuicPacketCount sent_packets_num_borderline_time_reorderings = 0;
};
// Called when a new ack arrives or the loss alarm fires.
diff --git a/quic/core/congestion_control/uber_loss_algorithm.cc b/quic/core/congestion_control/uber_loss_algorithm.cc
index 0f294d6..6fd23d6 100644
--- a/quic/core/congestion_control/uber_loss_algorithm.cc
+++ b/quic/core/congestion_control/uber_loss_algorithm.cc
@@ -54,6 +54,8 @@
overall_stats.sent_packets_max_sequence_reordering =
std::max(overall_stats.sent_packets_max_sequence_reordering,
stats.sent_packets_max_sequence_reordering);
+ overall_stats.sent_packets_num_borderline_time_reorderings +=
+ stats.sent_packets_num_borderline_time_reorderings;
}
return overall_stats;
diff --git a/quic/core/quic_connection_stats.h b/quic/core/quic_connection_stats.h
index 0aee970..397936b 100644
--- a/quic/core/quic_connection_stats.h
+++ b/quic/core/quic_connection_stats.h
@@ -104,6 +104,9 @@
// Maximum sequence reordering observed from acked packets.
QuicPacketCount sent_packets_max_sequence_reordering = 0;
+ // Number of times that a packet is not detected as lost per reordering_shift,
+ // but would have been if the reordering_shift increases by one.
+ QuicPacketCount sent_packets_num_borderline_time_reorderings = 0;
// The following stats are used only in TcpCubicSender.
// The number of loss events from TCP's perspective. Each loss event includes
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index 7d32176..3941a3e 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -972,6 +972,9 @@
detection_stats.sent_packets_max_sequence_reordering;
}
+ stats_->sent_packets_num_borderline_time_reorderings +=
+ detection_stats.sent_packets_num_borderline_time_reorderings;
+
for (const LostPacket& packet : packets_lost_) {
QuicTransmissionInfo* info =
unacked_packets_.GetMutableTransmissionInfo(packet.packet_number);
