When there is no retransmittable packet on the wire and there are open streams, the connection may send retransmittable PINGs to detect path degrading with aggressive timeout.
This change ensures that if there is no stream data received, the timeout will: 1. send first "X" pings with very aggressive timeout, i.e., initial_retransmittable_on_wire_timeout.
2. then send pings with less aggressive timeout using exponentially back-off until it exceeds the default ping timeout.
This prevents excessive PINGs being sent when upper layer forgets to cancel open streams. Protected by FLAGS_quic_max_aggressive_retransmittable_on_wire_ping_count, which also sets the limit of aggressive pings, "X".
gfe-relnote: n/a, client change only.
PiperOrigin-RevId: 275943283
Change-Id: I90f9346d76b2dbde7c57d226d6a7e4ad293a8175
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 7916791..b9bfbaf 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -256,7 +256,8 @@
pending_retransmission_alarm_(false),
defer_send_in_response_to_packets_(false),
ping_timeout_(QuicTime::Delta::FromSeconds(kPingTimeoutSecs)),
- retransmittable_on_wire_timeout_(QuicTime::Delta::Infinite()),
+ initial_retransmittable_on_wire_timeout_(QuicTime::Delta::Infinite()),
+ consecutive_retransmittable_on_wire_ping_count_(0),
arena_(),
ack_alarm_(alarm_factory_->CreateAlarm(arena_.New<AckAlarmDelegate>(this),
&arena_)),
@@ -909,6 +910,7 @@
visitor_->OnStreamFrame(frame);
stats_.stream_bytes_received += frame.data_length;
should_last_packet_instigate_acks_ = true;
+ consecutive_retransmittable_on_wire_ping_count_ = 0;
return connected_;
}
@@ -3162,24 +3164,48 @@
// because it is expecting a response from the server.
return;
}
- if (retransmittable_on_wire_timeout_.IsInfinite() ||
+ if (initial_retransmittable_on_wire_timeout_.IsInfinite() ||
sent_packet_manager_.HasInFlightPackets()) {
// Extend the ping alarm.
ping_alarm_->Update(clock_->ApproximateNow() + ping_timeout_,
QuicTime::Delta::FromSeconds(1));
return;
}
- DCHECK_LT(retransmittable_on_wire_timeout_, ping_timeout_);
+ DCHECK_LT(initial_retransmittable_on_wire_timeout_, ping_timeout_);
+ QuicTime::Delta retransmittable_on_wire_timeout =
+ initial_retransmittable_on_wire_timeout_;
+ int max_aggressive_retransmittable_on_wire_ping_count =
+ GetQuicFlag(FLAGS_quic_max_aggressive_retransmittable_on_wire_ping_count);
+ DCHECK_LE(0, max_aggressive_retransmittable_on_wire_ping_count);
+ if (consecutive_retransmittable_on_wire_ping_count_ >
+ max_aggressive_retransmittable_on_wire_ping_count) {
+ // Exponentially back off the timeout if the number of consecutive
+ // retransmittable on wire pings has exceeds the allowance.
+ int shift = consecutive_retransmittable_on_wire_ping_count_ -
+ max_aggressive_retransmittable_on_wire_ping_count;
+ retransmittable_on_wire_timeout =
+ initial_retransmittable_on_wire_timeout_ * (1 << shift);
+ }
// If it's already set to an earlier time, then don't update it.
if (ping_alarm_->IsSet() &&
ping_alarm_->deadline() <
- clock_->ApproximateNow() + retransmittable_on_wire_timeout_) {
+ clock_->ApproximateNow() + retransmittable_on_wire_timeout) {
return;
}
- // Use a shorter timeout if there are open streams, but nothing on the wire.
- ping_alarm_->Update(
- clock_->ApproximateNow() + retransmittable_on_wire_timeout_,
- QuicTime::Delta::FromMilliseconds(1));
+
+ if (retransmittable_on_wire_timeout < ping_timeout_) {
+ // Use a shorter timeout if there are open streams, but nothing on the wire.
+ ping_alarm_->Update(
+ clock_->ApproximateNow() + retransmittable_on_wire_timeout,
+ QuicTime::Delta::FromMilliseconds(1));
+ if (max_aggressive_retransmittable_on_wire_ping_count != 0) {
+ consecutive_retransmittable_on_wire_ping_count_++;
+ }
+ return;
+ }
+
+ ping_alarm_->Update(clock_->ApproximateNow() + ping_timeout_,
+ QuicTime::Delta::FromMilliseconds(1));
}
void QuicConnection::SetRetransmissionAlarm() {
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 3d07946..e9eb5e1 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -555,15 +555,12 @@
}
const QuicTime::Delta ping_timeout() { return ping_timeout_; }
// Used in Chromium, but not internally.
- // Sets a timeout for the ping alarm when there is no retransmittable data
- // in flight, allowing for a more aggressive ping alarm in that case.
- void set_retransmittable_on_wire_timeout(
+ // Sets an initial timeout for the ping alarm when there is no retransmittable
+ // data in flight, allowing for a more aggressive ping alarm in that case.
+ void set_initial_retransmittable_on_wire_timeout(
QuicTime::Delta retransmittable_on_wire_timeout) {
DCHECK(!ping_alarm_->IsSet());
- retransmittable_on_wire_timeout_ = retransmittable_on_wire_timeout;
- }
- const QuicTime::Delta retransmittable_on_wire_timeout() {
- return retransmittable_on_wire_timeout_;
+ initial_retransmittable_on_wire_timeout_ = retransmittable_on_wire_timeout;
}
// Used in Chromium, but not internally.
void set_creator_debug_delegate(QuicPacketCreator::DebugDelegate* visitor) {
@@ -1314,8 +1311,12 @@
// The timeout for PING.
QuicTime::Delta ping_timeout_;
- // Timeout for how long the wire can have no retransmittable packets.
- QuicTime::Delta retransmittable_on_wire_timeout_;
+ // Initial timeout for how long the wire can have no retransmittable packets.
+ QuicTime::Delta initial_retransmittable_on_wire_timeout_;
+
+ // Indicates how many retransmittable-on-wire pings have been emitted without
+ // receiving any new data in between.
+ int consecutive_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 e9d6615..e2ef838 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -4090,7 +4090,7 @@
// Sets retransmittable on wire.
const QuicTime::Delta retransmittable_on_wire_timeout =
QuicTime::Delta::FromMilliseconds(50);
- connection_.set_retransmittable_on_wire_timeout(
+ connection_.set_initial_retransmittable_on_wire_timeout(
retransmittable_on_wire_timeout);
EXPECT_TRUE(connection_.connected());
@@ -7742,7 +7742,7 @@
TEST_P(QuicConnectionTest, RetransmittableOnWireSetsPingAlarm) {
const QuicTime::Delta retransmittable_on_wire_timeout =
QuicTime::Delta::FromMilliseconds(50);
- connection_.set_retransmittable_on_wire_timeout(
+ connection_.set_initial_retransmittable_on_wire_timeout(
retransmittable_on_wire_timeout);
EXPECT_TRUE(connection_.connected());
@@ -8309,7 +8309,7 @@
TEST_P(QuicConnectionTest, PingAfterLastRetransmittablePacketAcked) {
const QuicTime::Delta retransmittable_on_wire_timeout =
QuicTime::Delta::FromMilliseconds(50);
- connection_.set_retransmittable_on_wire_timeout(
+ connection_.set_initial_retransmittable_on_wire_timeout(
retransmittable_on_wire_timeout);
EXPECT_TRUE(connection_.connected());
@@ -8401,7 +8401,7 @@
TEST_P(QuicConnectionTest, NoPingIfRetransmittablePacketSent) {
const QuicTime::Delta retransmittable_on_wire_timeout =
QuicTime::Delta::FromMilliseconds(50);
- connection_.set_retransmittable_on_wire_timeout(
+ connection_.set_initial_retransmittable_on_wire_timeout(
retransmittable_on_wire_timeout);
EXPECT_TRUE(connection_.connected());
@@ -8476,6 +8476,225 @@
ASSERT_EQ(1u, writer_->ping_frames().size());
}
+// When there is no stream data received but are open streams, send the
+// first few consecutive pings with aggressive retransmittable-on-wire
+// timeout. Exponentially back off the retransmittable-on-wire ping timeout
+// afterwards until it exceeds the default ping timeout.
+TEST_P(QuicConnectionTest, BackOffRetransmittableOnWireTimeout) {
+ int max_aggressive_retransmittable_on_wire_ping_count = 5;
+ SetQuicFlag(FLAGS_quic_max_aggressive_retransmittable_on_wire_ping_count,
+ max_aggressive_retransmittable_on_wire_ping_count);
+ const QuicTime::Delta initial_retransmittable_on_wire_timeout =
+ QuicTime::Delta::FromMilliseconds(200);
+ 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(QuicTime::Delta::FromMilliseconds(5));
+ 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 <= max_aggressive_retransmittable_on_wire_ping_count; i++) {
+ // Receive an ACK of the previous packet. This should set the ping alarm
+ // with the initial retransmittable-on-wire timeout.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+ 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();
+ EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() {
+ SendPing();
+ }));
+ clock_.AdvanceTime(initial_retransmittable_on_wire_timeout);
+ connection_.GetPingAlarm()->Fire();
+ }
+
+ QuicTime::Delta retransmittable_on_wire_timeout =
+ initial_retransmittable_on_wire_timeout;
+
+ // Verify subsequent pings are sent with timeout that is exponentially backed
+ // off.
+ while (retransmittable_on_wire_timeout * 2 < connection_.ping_timeout()) {
+ // Receive an ACK for the previous PING. This should set the
+ // ping alarm with backed off retransmittable-on-wire timeout.
+ retransmittable_on_wire_timeout = retransmittable_on_wire_timeout * 2;
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+ 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(retransmittable_on_wire_timeout,
+ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+
+ // Simulate the alarm firing and check that a PING is sent.
+ writer_->Reset();
+ EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() {
+ SendPing();
+ }));
+ clock_.AdvanceTime(retransmittable_on_wire_timeout);
+ connection_.GetPingAlarm()->Fire();
+ }
+
+ // The ping alarm is set with default ping timeout.
+ EXPECT_TRUE(connection_.GetPingAlarm()->IsSet());
+ EXPECT_EQ(connection_.ping_timeout(),
+ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+
+ // Receive an ACK for the previous PING. The ping alarm is set with an
+ // earlier deadline.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+ 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() - QuicTime::Delta::FromMilliseconds(5),
+ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+}
+
+// This test verify that the count of consecutive aggressive pings is reset
+// when new data is received. And it also verifies the connection resets
+// the exponential back-off of the retransmittable-on-wire ping timeout
+// after receiving new stream data.
+TEST_P(QuicConnectionTest, ResetBackOffRetransmitableOnWireTimeout) {
+ int max_aggressive_retransmittable_on_wire_ping_count = 3;
+ SetQuicFlag(FLAGS_quic_max_aggressive_retransmittable_on_wire_ping_count, 3);
+ const QuicTime::Delta initial_retransmittable_on_wire_timeout =
+ QuicTime::Delta::FromMilliseconds(200);
+ connection_.set_initial_retransmittable_on_wire_timeout(
+ initial_retransmittable_on_wire_timeout);
+
+ EXPECT_TRUE(connection_.connected());
+ EXPECT_CALL(visitor_, ShouldKeepConnectionAlive())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)).Times(AnyNumber());
+ EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _))
+ .Times(AnyNumber());
+
+ const char data[] = "data";
+ // Advance 5ms, send a retransmittable data packet to the peer.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+ 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());
+
+ // Receive an ACK of the first packet. This should set the ping alarm with
+ // initial retransmittable-on-wire timeout since there is no retransmittable
+ // packet on the wire.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+ QuicAckFrame frame =
+ InitAckFrame({{QuicPacketNumber(1), QuicPacketNumber(2)}});
+ 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();
+ EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); }));
+ clock_.AdvanceTime(initial_retransmittable_on_wire_timeout);
+ connection_.GetPingAlarm()->Fire();
+
+ // Receive an ACK for the previous PING. Ping alarm will be set with
+ // aggressive timeout.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+ QuicPacketNumber ack_num = creator_->packet_number();
+ 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());
+
+ // Process a data packet.
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1);
+ ProcessDataPacket(peer_creator_.packet_number() + 1);
+ QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_,
+ peer_creator_.packet_number() + 1);
+ EXPECT_EQ(initial_retransmittable_on_wire_timeout,
+ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+
+ // Verify the count of consecutive aggressive pings is reset.
+ for (int i = 0; i < max_aggressive_retransmittable_on_wire_ping_count; i++) {
+ // Receive an ACK of the previous packet. This should set the ping alarm
+ // with the initial retransmittable-on-wire 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(initial_retransmittable_on_wire_timeout,
+ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+ // Simulate the alarm firing and check that a PING is sent.
+ writer_->Reset();
+ EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() {
+ SendPing();
+ }));
+ clock_.AdvanceTime(initial_retransmittable_on_wire_timeout);
+ connection_.GetPingAlarm()->Fire();
+ // Advance 5ms to receive next packet.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+ }
+
+ // Receive another ACK for the previous PING. This should set the
+ // ping alarm with backed off retransmittable-on-wire timeout.
+ ack_num = creator_->packet_number();
+ frame = InitAckFrame(
+ {{QuicPacketNumber(ack_num), QuicPacketNumber(ack_num + 1)}});
+ ProcessAckPacket(&frame);
+ EXPECT_TRUE(connection_.GetPingAlarm()->IsSet());
+ EXPECT_EQ(initial_retransmittable_on_wire_timeout * 2,
+ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+
+ writer_->Reset();
+ EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); }));
+ clock_.AdvanceTime(2 * initial_retransmittable_on_wire_timeout);
+ connection_.GetPingAlarm()->Fire();
+
+ // Process another data packet and a new ACK packet. The ping alarm is set
+ // with aggressive ping timeout again.
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1);
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+ ProcessDataPacket(peer_creator_.packet_number() + 1);
+ QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_,
+ peer_creator_.packet_number() + 1);
+ ack_num = creator_->packet_number();
+ 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());
+}
+
TEST_P(QuicConnectionTest, OnForwardProgressConfirmed) {
EXPECT_CALL(visitor_, OnForwardProgressConfirmed()).Times(Exactly(0));
EXPECT_TRUE(connection_.connected());
diff --git a/quic/test_tools/quic_packet_creator_peer.cc b/quic/test_tools/quic_packet_creator_peer.cc
index 20a3834..8ffd59f 100644
--- a/quic/test_tools/quic_packet_creator_peer.cc
+++ b/quic/test_tools/quic_packet_creator_peer.cc
@@ -70,6 +70,11 @@
creator->packet_.packet_number = QuicPacketNumber(s);
}
+void QuicPacketCreatorPeer::SetPacketNumber(QuicPacketCreator* creator,
+ QuicPacketNumber num) {
+ creator->packet_.packet_number = num;
+}
+
// static
void QuicPacketCreatorPeer::ClearPacketNumber(QuicPacketCreator* creator) {
creator->packet_.packet_number.Clear();
diff --git a/quic/test_tools/quic_packet_creator_peer.h b/quic/test_tools/quic_packet_creator_peer.h
index a633ef4..3948aa4 100644
--- a/quic/test_tools/quic_packet_creator_peer.h
+++ b/quic/test_tools/quic_packet_creator_peer.h
@@ -32,6 +32,7 @@
static QuicVariableLengthIntegerLength GetLengthLength(
QuicPacketCreator* creator);
static void SetPacketNumber(QuicPacketCreator* creator, uint64_t s);
+ static void SetPacketNumber(QuicPacketCreator* creator, QuicPacketNumber num);
static void ClearPacketNumber(QuicPacketCreator* creator);
static void FillPacketHeader(QuicPacketCreator* creator,
QuicPacketHeader* header);
