Reduce number of packets sent by ROWP
This CL changes our default ROWP behavior to start the exponential backoff after 10 packets instead of 200, and also adds a connection-lifetime cap on ROWP to disable ROWP after 1000 ROWPs have been sent.
PiperOrigin-RevId: 338078574
Change-Id: If025717bf5b509c6bbbfb3dec34a928dfdeec7d5
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index fa02801..1e59d4f 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -285,6 +285,7 @@
ping_timeout_(QuicTime::Delta::FromSeconds(kPingTimeoutSecs)),
initial_retransmittable_on_wire_timeout_(QuicTime::Delta::Infinite()),
consecutive_retransmittable_on_wire_ping_count_(0),
+ retransmittable_on_wire_ping_count_(0),
arena_(),
ack_alarm_(alarm_factory_->CreateAlarm(arena_.New<AckAlarmDelegate>(this),
&arena_)),
@@ -4054,7 +4055,9 @@
return;
}
if (initial_retransmittable_on_wire_timeout_.IsInfinite() ||
- sent_packet_manager_.HasInFlightPackets()) {
+ sent_packet_manager_.HasInFlightPackets() ||
+ retransmittable_on_wire_ping_count_ >
+ GetQuicFlag(FLAGS_quic_max_retransmittable_on_wire_ping_count)) {
// Extend the ping alarm.
ping_alarm_->Update(clock_->ApproximateNow() + ping_timeout_,
QuicTime::Delta::FromSeconds(1));
@@ -4090,6 +4093,7 @@
if (max_aggressive_retransmittable_on_wire_ping_count != 0) {
consecutive_retransmittable_on_wire_ping_count_++;
}
+ retransmittable_on_wire_ping_count_++;
return;
}
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 4be1b96..59fa80d 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -526,6 +526,10 @@
QuicConnectionStats& mutable_stats() { return stats_; }
+ int retransmittable_on_wire_ping_count() const {
+ return retransmittable_on_wire_ping_count_;
+ }
+
// Returns statistics tracked for this connection.
const QuicConnectionStats& GetStats();
@@ -1651,6 +1655,9 @@
// receiving any new data in between.
int consecutive_retransmittable_on_wire_ping_count_;
+ // Indicates how many retransmittable-on-wire pings have been emitted.
+ int retransmittable_on_wire_ping_count_;
+
// Arena to store class implementations within the QuicConnection.
QuicConnectionArena arena_;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 433ba74..7d736b3 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -8251,6 +8251,75 @@
connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
}
+// Make sure that we never send more retransmissible on the wire pings than
+// the limit in FLAGS_quic_max_retransmittable_on_wire_ping_count.
+TEST_P(QuicConnectionTest, RetransmittableOnWirePingLimit) {
+ static constexpr int kMaxRetransmittableOnWirePingCount = 3;
+ SetQuicFlag(FLAGS_quic_max_retransmittable_on_wire_ping_count,
+ kMaxRetransmittableOnWirePingCount);
+ static constexpr QuicTime::Delta initial_retransmittable_on_wire_timeout =
+ QuicTime::Delta::FromMilliseconds(200);
+ static constexpr QuicTime::Delta short_delay =
+ QuicTime::Delta::FromMilliseconds(5);
+ ASSERT_LT(short_delay * 10, initial_retransmittable_on_wire_timeout);
+ connection_.set_initial_retransmittable_on_wire_timeout(
+ initial_retransmittable_on_wire_timeout);
+
+ EXPECT_TRUE(connection_.connected());
+ EXPECT_CALL(visitor_, ShouldKeepConnectionAlive())
+ .WillRepeatedly(Return(true));
+
+ const char data[] = "data";
+ // Advance 5ms, send a retransmittable data packet to the peer.
+ clock_.AdvanceTime(short_delay);
+ EXPECT_FALSE(connection_.GetPingAlarm()->IsSet());
+ connection_.SendStreamDataWithString(1, data, 0, NO_FIN);
+ EXPECT_TRUE(connection_.sent_packet_manager().HasInFlightPackets());
+ // The ping alarm is set for the ping timeout, not the shorter
+ // retransmittable_on_wire_timeout.
+ EXPECT_TRUE(connection_.GetPingAlarm()->IsSet());
+ EXPECT_EQ(connection_.ping_timeout(),
+ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+
+ EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)).Times(AnyNumber());
+ EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _))
+ .Times(AnyNumber());
+
+ // Verify that the first few consecutive retransmittable on wire pings are
+ // sent with aggressive timeout.
+ for (int i = 0; i <= kMaxRetransmittableOnWirePingCount; i++) {
+ // Receive an ACK of the previous packet. This should set the ping alarm
+ // with the initial retransmittable-on-wire timeout.
+ clock_.AdvanceTime(short_delay);
+ QuicPacketNumber ack_num = creator_->packet_number();
+ QuicAckFrame frame = InitAckFrame(
+ {{QuicPacketNumber(ack_num), QuicPacketNumber(ack_num + 1)}});
+ ProcessAckPacket(&frame);
+ EXPECT_TRUE(connection_.GetPingAlarm()->IsSet());
+ EXPECT_EQ(initial_retransmittable_on_wire_timeout,
+ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+ // Simulate the alarm firing and check that a PING is sent.
+ writer_->Reset();
+ if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) {
+ EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() {
+ SendPing();
+ }));
+ }
+ clock_.AdvanceTime(initial_retransmittable_on_wire_timeout);
+ connection_.GetPingAlarm()->Fire();
+ }
+
+ // Receive an ACK of the previous packet. This should set the ping alarm
+ // but this time with the default ping timeout.
+ QuicPacketNumber ack_num = creator_->packet_number();
+ QuicAckFrame frame = InitAckFrame(
+ {{QuicPacketNumber(ack_num), QuicPacketNumber(ack_num + 1)}});
+ ProcessAckPacket(&frame);
+ EXPECT_TRUE(connection_.GetPingAlarm()->IsSet());
+ EXPECT_EQ(connection_.ping_timeout(),
+ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+}
+
TEST_P(QuicConnectionTest, ValidStatelessResetToken) {
const QuicUint128 kTestToken = 1010101;
const QuicUint128 kWrongTestToken = 1010100;
