Move the increments of retransmittable-on-wire counters to when the alarm fires in retransmittable-on-wire mode.
Protected by FLAGS_quic_reloadable_flag_quic_use_ping_manager2.
PiperOrigin-RevId: 447876160
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index 3913635..1e458c6 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -128,7 +128,7 @@
void OnAlarm() override {
QUICHE_DCHECK(connection_->connected());
- QUICHE_DCHECK(!GetQuicReloadableFlag(quic_use_ping_manager));
+ QUICHE_DCHECK(!GetQuicReloadableFlag(quic_use_ping_manager2));
connection_->OnPingTimeout();
}
};
@@ -4702,7 +4702,7 @@
return;
}
if (use_ping_manager_) {
- QUIC_RELOADABLE_FLAG_COUNT(quic_use_ping_manager);
+ QUIC_RELOADABLE_FLAG_COUNT(quic_use_ping_manager2);
ping_manager_.SetAlarm(clock_->ApproximateNow(),
visitor_->ShouldKeepConnectionAlive(),
sent_packet_manager_.HasInFlightPackets());
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h
index b214361..09cef3d 100644
--- a/quiche/quic/core/quic_connection.h
+++ b/quiche/quic/core/quic_connection.h
@@ -1995,7 +1995,7 @@
bool defer_send_in_response_to_packets_;
// TODO(fayang): remove PING related fields below when deprecating
- // quic_use_ping_manager.
+ // quic_use_ping_manager2.
// The timeout for keep-alive PING.
QuicTime::Delta keep_alive_ping_timeout_;
@@ -2019,7 +2019,7 @@
// An alarm that is scheduled when the SentPacketManager requires a delay
// before sending packets and fires when the packet may be sent.
QuicArenaScopedPtr<QuicAlarm> send_alarm_;
- // TODO(fayang): remove ping_alarm_ when deprecating quic_use_ping_manager.
+ // TODO(fayang): remove ping_alarm_ when deprecating quic_use_ping_manager2.
// An alarm that fires when a ping should be sent.
QuicArenaScopedPtr<QuicAlarm> ping_alarm_;
// An alarm that fires when an MTU probe should be sent.
@@ -2256,7 +2256,7 @@
// If true, send connection close packet on INVALID_VERSION.
bool send_connection_close_for_invalid_version_ = false;
- const bool use_ping_manager_ = GetQuicReloadableFlag(quic_use_ping_manager);
+ const bool use_ping_manager_ = GetQuicReloadableFlag(quic_use_ping_manager2);
QuicPingManager ping_manager_;
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index 0da0673..692e3ef 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -7185,7 +7185,7 @@
}
TEST_P(QuicConnectionTest, RetransmittableOnWireSendFirstPacket) {
- if (!GetQuicReloadableFlag(quic_use_ping_manager) ||
+ if (!GetQuicReloadableFlag(quic_use_ping_manager2) ||
!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
return;
}
@@ -7231,7 +7231,7 @@
}
TEST_P(QuicConnectionTest, RetransmittableOnWireSendRandomBytes) {
- if (!GetQuicReloadableFlag(quic_use_ping_manager) ||
+ if (!GetQuicReloadableFlag(quic_use_ping_manager2) ||
!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
return;
}
@@ -7280,7 +7280,7 @@
TEST_P(QuicConnectionTest,
RetransmittableOnWireSendRandomBytesWithWriterBlocked) {
- if (!GetQuicReloadableFlag(quic_use_ping_manager) ||
+ if (!GetQuicReloadableFlag(quic_use_ping_manager2) ||
!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
return;
}
@@ -8250,6 +8250,8 @@
peer_creator_.packet_number() + 1);
EXPECT_EQ(initial_retransmittable_on_wire_timeout,
connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow());
+ clock_.AdvanceTime(initial_retransmittable_on_wire_timeout);
+ connection_.GetPingAlarm()->Fire();
// Verify the count of consecutive aggressive pings is reset.
for (int i = 0; i < max_aggressive_retransmittable_on_wire_ping_count; i++) {
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index 75c3687..73b41a3 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -86,7 +86,7 @@
// If true, use BBRv2 as the default congestion controller. Takes precedence over --quic_default_to_bbr.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_default_to_bbr_v2, false)
// If true, use PING manager to manage the PING alarm.
-QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_ping_manager, true)
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_ping_manager2, true)
// If true, use new connection ID in connection migration.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_connection_migration_use_new_cid_v2, true)
// If true, uses conservative cwnd gain and pacing gain when cwnd gets bootstrapped.
diff --git a/quiche/quic/core/quic_ping_manager.cc b/quiche/quic/core/quic_ping_manager.cc
index e3947e5..e2ef9c2 100644
--- a/quiche/quic/core/quic_ping_manager.cc
+++ b/quiche/quic/core/quic_ping_manager.cc
@@ -8,6 +8,11 @@
namespace {
+// Maximum shift used to calculate retransmittable on wire timeout. For 200ms
+// initial retransmittable on wire delay, this would get a maximum of 200ms * (1
+// << 10) = 204.8s
+const int kMaxRetransmittableOnWireDelayShift = 10;
+
class AlarmDelegate : public QuicAlarm::DelegateWithContext {
public:
explicit AlarmDelegate(QuicPingManager* manager,
@@ -47,11 +52,6 @@
return;
}
alarm_->Update(earliest_deadline, kAlarmGranularity);
- if (GetQuicFlag(
- FLAGS_quic_max_aggressive_retransmittable_on_wire_ping_count) != 0) {
- ++consecutive_retransmittable_on_wire_count_;
- }
- ++retransmittable_on_wire_count_;
}
void QuicPingManager::OnAlarm() {
@@ -65,6 +65,12 @@
// to SetAlarm later.
if (earliest_deadline == retransmittable_on_wire_deadline_) {
retransmittable_on_wire_deadline_ = QuicTime::Zero();
+ if (GetQuicFlag(
+ FLAGS_quic_max_aggressive_retransmittable_on_wire_ping_count) !=
+ 0) {
+ ++consecutive_retransmittable_on_wire_count_;
+ }
+ ++retransmittable_on_wire_count_;
delegate_->OnRetransmittableOnWireTimeout();
return;
}
@@ -125,8 +131,9 @@
max_aggressive_retransmittable_on_wire_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_count_ -
- max_aggressive_retransmittable_on_wire_count;
+ int shift = std::min(consecutive_retransmittable_on_wire_count_ -
+ max_aggressive_retransmittable_on_wire_count,
+ kMaxRetransmittableOnWireDelayShift);
retransmittable_on_wire_timeout =
initial_retransmittable_on_wire_timeout_ * (1 << shift);
}
diff --git a/quiche/quic/core/quic_ping_manager_test.cc b/quiche/quic/core/quic_ping_manager_test.cc
index e7b4b78..509022b 100644
--- a/quiche/quic/core/quic_ping_manager_test.cc
+++ b/quiche/quic/core/quic_ping_manager_test.cc
@@ -16,6 +16,11 @@
static QuicAlarm* GetAlarm(QuicPingManager* manager) {
return manager->alarm_.get();
}
+
+ static void SetPerspective(QuicPingManager* manager,
+ Perspective perspective) {
+ manager->perspective_ = perspective;
+ }
};
namespace {
@@ -292,6 +297,9 @@
!kHasInflightPackets);
EXPECT_EQ(initial_retransmittable_on_wire_timeout,
alarm_->deadline() - clock_.ApproximateNow());
+ EXPECT_CALL(delegate_, OnRetransmittableOnWireTimeout());
+ clock_.AdvanceTime(initial_retransmittable_on_wire_timeout);
+ alarm_->Fire();
for (int i = 0; i < kMaxAggressiveRetransmittableOnWireCount; i++) {
manager_.SetAlarm(clock_.ApproximateNow(), kShouldKeepAlive,
@@ -375,6 +383,46 @@
EXPECT_FALSE(alarm_->IsSet());
}
+TEST_F(QuicPingManagerTest, MaxRetransmittableOnWireDelayShift) {
+ QuicPingManagerPeer::SetPerspective(&manager_, Perspective::IS_SERVER);
+ const int kMaxAggressiveRetransmittableOnWireCount = 3;
+ SetQuicFlag(FLAGS_quic_max_aggressive_retransmittable_on_wire_ping_count,
+ kMaxAggressiveRetransmittableOnWireCount);
+ const QuicTime::Delta initial_retransmittable_on_wire_timeout =
+ QuicTime::Delta::FromMilliseconds(200);
+ manager_.set_initial_retransmittable_on_wire_timeout(
+ initial_retransmittable_on_wire_timeout);
+
+ for (int i = 0; i <= kMaxAggressiveRetransmittableOnWireCount; i++) {
+ manager_.SetAlarm(clock_.ApproximateNow(), kShouldKeepAlive,
+ !kHasInflightPackets);
+ EXPECT_TRUE(alarm_->IsSet());
+ EXPECT_EQ(initial_retransmittable_on_wire_timeout,
+ alarm_->deadline() - clock_.ApproximateNow());
+ clock_.AdvanceTime(initial_retransmittable_on_wire_timeout);
+ EXPECT_CALL(delegate_, OnRetransmittableOnWireTimeout());
+ alarm_->Fire();
+ manager_.SetAlarm(clock_.ApproximateNow(), kShouldKeepAlive,
+ kHasInflightPackets);
+ }
+ for (int i = 1; i <= 20; ++i) {
+ manager_.SetAlarm(clock_.ApproximateNow(), kShouldKeepAlive,
+ !kHasInflightPackets);
+ EXPECT_TRUE(alarm_->IsSet());
+ if (i <= 10) {
+ EXPECT_EQ(initial_retransmittable_on_wire_timeout * (1 << i),
+ alarm_->deadline() - clock_.ApproximateNow());
+ } else {
+ // Verify shift is capped.
+ EXPECT_EQ(initial_retransmittable_on_wire_timeout * (1 << 10),
+ alarm_->deadline() - clock_.ApproximateNow());
+ }
+ clock_.AdvanceTime(alarm_->deadline() - clock_.ApproximateNow());
+ EXPECT_CALL(delegate_, OnRetransmittableOnWireTimeout());
+ alarm_->Fire();
+ }
+}
+
} // namespace
} // namespace test
