gfe-relnote: Deprecate gfe2_reloadable_flag_quic_fix_rto_retransmission3.
PiperOrigin-RevId: 275103541
Change-Id: I316da0f4640701d44078a2413619069b220d834b
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 0b4c05f..079e14d 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2712,7 +2712,7 @@
WriteIfNotBlocked();
}
- if (sent_packet_manager_.fix_rto_retransmission()) {
+ if (session_decides_what_to_write()) {
if (packet_generator_.packet_number() == previous_created_packet_number &&
(retransmission_mode == QuicSentPacketManager::TLP_MODE ||
retransmission_mode == QuicSentPacketManager::RTO_MODE ||
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 3d923a2..20f63f7 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -3495,8 +3495,7 @@
// Ensure that the data is still in flight, but the retransmission alarm is no
// longer set.
EXPECT_GT(manager_->GetBytesInFlight(), 0u);
- if (QuicConnectionPeer::GetSentPacketManager(&connection_)
- ->fix_rto_retransmission()) {
+ if (connection_.session_decides_what_to_write()) {
EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet());
} else {
EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
@@ -3779,8 +3778,7 @@
writer_->SetWritable();
connection_.OnCanWrite();
- if (QuicConnectionPeer::GetSentPacketManager(&connection_)
- ->fix_rto_retransmission()) {
+ if (connection_.session_decides_what_to_write()) {
EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet());
} else {
EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
@@ -4243,31 +4241,14 @@
// Simulate the retransmission alarm firing.
clock_.AdvanceTime(DefaultRetransmissionTime());
// RTO fires, but there is no packet to be RTOed.
- if (GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
- } else {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
- }
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
connection_.GetRetransmissionAlarm()->Fire();
- if (GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
- EXPECT_EQ(1u, writer_->rst_stream_frames().size());
- }
+ EXPECT_EQ(1u, writer_->rst_stream_frames().size());
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(40);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(20);
- if (GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
- EXPECT_CALL(visitor_, WillingAndAbleToWrite())
- .WillRepeatedly(Return(false));
- } else {
- EXPECT_CALL(visitor_, WillingAndAbleToWrite()).WillRepeatedly(Return(true));
- }
- if (GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
- EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(1);
- } else {
- // Since there is a buffered RST_STREAM, no retransmittable frame is bundled
- // with ACKs.
- EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(0);
- }
+ EXPECT_CALL(visitor_, WillingAndAbleToWrite()).WillRepeatedly(Return(false));
+ EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(1);
// Receives packets 1 - 40.
for (size_t i = 1; i <= 40; ++i) {
ProcessDataPacket(i);
@@ -9185,8 +9166,7 @@
// Regresstion test for b/138962304.
TEST_P(QuicConnectionTest, RtoAndWriteBlocked) {
- if (!QuicConnectionPeer::GetSentPacketManager(&connection_)
- ->fix_rto_retransmission()) {
+ if (!connection_.session_decides_what_to_write()) {
return;
}
EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
@@ -9215,8 +9195,7 @@
// Regresstion test for b/138962304.
TEST_P(QuicConnectionTest, TlpAndWriteBlocked) {
- if (!QuicConnectionPeer::GetSentPacketManager(&connection_)
- ->fix_rto_retransmission()) {
+ if (!connection_.session_decides_what_to_write()) {
return;
}
EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
@@ -9249,8 +9228,7 @@
// Regresstion test for b/139375344.
TEST_P(QuicConnectionTest, RtoForcesSendingPing) {
- if (!QuicConnectionPeer::GetSentPacketManager(&connection_)
- ->fix_rto_retransmission() ||
+ if (!connection_.session_decides_what_to_write() ||
connection_.PtoEnabled()) {
return;
}
@@ -9289,12 +9267,10 @@
}
TEST_P(QuicConnectionTest, ProbeTimeout) {
- if (!connection_.session_decides_what_to_write() ||
- !GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
+ if (!connection_.session_decides_what_to_write()) {
return;
}
SetQuicReloadableFlag(quic_enable_pto, true);
- SetQuicReloadableFlag(quic_fix_rto_retransmission3, true);
QuicConfig config;
QuicTagVector connection_options;
connection_options.push_back(k2PTO);
@@ -9322,8 +9298,7 @@
}
TEST_P(QuicConnectionTest, CloseConnectionAfter6ClientPTOs) {
- if (!connection_.session_decides_what_to_write() ||
- !GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
+ if (!connection_.session_decides_what_to_write()) {
return;
}
SetQuicReloadableFlag(quic_enable_pto, true);
@@ -9364,8 +9339,7 @@
}
TEST_P(QuicConnectionTest, CloseConnectionAfter7ClientPTOs) {
- if (!connection_.session_decides_what_to_write() ||
- !GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
+ if (!connection_.session_decides_what_to_write()) {
return;
}
SetQuicReloadableFlag(quic_enable_pto, true);
@@ -9405,8 +9379,7 @@
}
TEST_P(QuicConnectionTest, CloseConnectionAfter8ClientPTOs) {
- if (!connection_.session_decides_what_to_write() ||
- !GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
+ if (!connection_.session_decides_what_to_write()) {
return;
}
SetQuicReloadableFlag(quic_enable_pto, true);
@@ -9558,8 +9531,7 @@
// Regression test for b/137401387 and b/138962304.
TEST_P(QuicConnectionTest, RtoPacketAsTwo) {
- if (!QuicConnectionPeer::GetSentPacketManager(&connection_)
- ->fix_rto_retransmission() ||
+ if (!connection_.session_decides_what_to_write() ||
connection_.PtoEnabled()) {
return;
}
@@ -9603,8 +9575,7 @@
}
TEST_P(QuicConnectionTest, PtoSkipsPacketNumber) {
- if (!connection_.session_decides_what_to_write() ||
- !GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
+ if (!connection_.session_decides_what_to_write()) {
return;
}
SetQuicReloadableFlag(quic_enable_pto, true);
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index a747f0b..d6bf700 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -108,7 +108,6 @@
pto_enabled_(false),
max_probe_packets_per_pto_(2),
consecutive_pto_count_(0),
- fix_rto_retransmission_(false),
handshake_mode_disabled_(false),
detect_spurious_losses_(GetQuicReloadableFlag(quic_detect_spurious_loss)),
neuter_handshake_packets_once_(
@@ -170,7 +169,8 @@
}
}
- if (GetQuicReloadableFlag(quic_enable_pto) && fix_rto_retransmission_) {
+ if (GetQuicReloadableFlag(quic_enable_pto) &&
+ session_decides_what_to_write()) {
if (config.HasClientSentConnectionOption(k2PTO, perspective)) {
pto_enabled_ = true;
QUIC_RELOADABLE_FLAG_COUNT_N(quic_enable_pto, 2, 4);
@@ -881,7 +881,8 @@
if (session_decides_what_to_write()) {
has_retransmissions = it->state != OUTSTANDING;
}
- if (!fix_rto_retransmission_ && it->in_flight && !has_retransmissions &&
+ if (!session_decides_what_to_write() && it->in_flight &&
+ !has_retransmissions &&
!unacked_packets_.HasRetransmittableFrames(*it)) {
// Log only for non-retransmittable data.
// Retransmittable data is marked as lost during loss detection, and will
@@ -903,7 +904,7 @@
for (QuicPacketNumber retransmission : retransmissions) {
MarkForRetransmission(retransmission, RTO_RETRANSMISSION);
}
- if (fix_rto_retransmission_ && retransmissions.empty()) {
+ if (retransmissions.empty()) {
QUIC_BUG_IF(pending_timer_transmission_count_ != 0);
// No packets to be RTO retransmitted, raise up a credit to allow
// connection to send.
@@ -950,7 +951,6 @@
void QuicSentPacketManager::EnableIetfPtoAndLossDetection() {
DCHECK(session_decides_what_to_write());
- fix_rto_retransmission_ = true;
pto_enabled_ = true;
handshake_mode_disabled_ = true;
uber_loss_algorithm_.SetLossDetectionType(kIetfLossDetection);
@@ -1057,7 +1057,7 @@
// Do not set the timer if there is any credit left.
return QuicTime::Zero();
}
- if (!fix_rto_retransmission_ &&
+ if (!session_decides_what_to_write() &&
!unacked_packets_.HasUnackedRetransmittableFrames()) {
return QuicTime::Zero();
}
@@ -1426,11 +1426,6 @@
void QuicSentPacketManager::SetSessionDecideWhatToWrite(
bool session_decides_what_to_write) {
- if (GetQuicReloadableFlag(quic_fix_rto_retransmission3) &&
- session_decides_what_to_write) {
- fix_rto_retransmission_ = true;
- QUIC_RELOADABLE_FLAG_COUNT(quic_fix_rto_retransmission3);
- }
unacked_packets_.SetSessionDecideWhatToWrite(session_decides_what_to_write);
}
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index 9e903fd..e0adb01 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -409,8 +409,6 @@
return unacked_packets_.supports_multiple_packet_number_spaces();
}
- bool fix_rto_retransmission() const { return fix_rto_retransmission_; }
-
bool pto_enabled() const { return pto_enabled_; }
bool handshake_mode_disabled() const { return handshake_mode_disabled_; }
@@ -652,10 +650,6 @@
// Number of times the PTO timer has fired in a row without receiving an ack.
size_t consecutive_pto_count_;
- // Latched value of quic_fix_rto_retransmission3 and
- // session_decides_what_to_write.
- bool fix_rto_retransmission_;
-
// True if HANDSHAKE mode has been disabled.
bool handshake_mode_disabled_;
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc
index 4ee9fd3..389f5cb 100644
--- a/quic/core/quic_sent_packet_manager_test.cc
+++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -358,7 +358,6 @@
}
void EnablePto(QuicTag tag) {
- SetQuicReloadableFlag(quic_fix_rto_retransmission3, true);
manager_.SetSessionDecideWhatToWrite(true);
SetQuicReloadableFlag(quic_enable_pto, true);
QuicConfig config;
@@ -3070,12 +3069,8 @@
EXPECT_CALL(notifier_, RetransmitFrames(_, _)).Times(0);
manager_.OnRetransmissionTimeout();
EXPECT_EQ(2u, stats_.rto_count);
- if (GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
- // Verify a credit is raised up.
- EXPECT_EQ(1u, manager_.pending_timer_transmission_count());
- } else {
- EXPECT_EQ(0u, manager_.pending_timer_transmission_count());
- }
+ // Verify a credit is raised up.
+ EXPECT_EQ(1u, manager_.pending_timer_transmission_count());
}
TEST_P(QuicSentPacketManagerTest, ComputingProbeTimeout) {
