Automated g4 rollback of changelist 269791493.
*** Reason for rollback ***
Causes b/142408319 when combine with gfe2_reloadable_flag_quic_treat_queued_packets_as_sent
*** Original change description ***
gfe-relnote: In QUIC, making sure there are always enough credits for non-loss retransmission. Protected by gfe2_reloadable_flag_quic_grant_enough_credits.
***
PiperOrigin-RevId: 274006897
Change-Id: I2616771107eb81111a0b5afa0c5af465cf984ffa
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index bbcbf95..68ba446 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -3668,7 +3668,8 @@
}
TEST_P(QuicConnectionTest, QueueAfterTwoRTOs) {
- if (connection_.PtoEnabled()) {
+ if (connection_.PtoEnabled() ||
+ !connection_.session_decides_what_to_write()) {
return;
}
connection_.SetMaxTailLossProbes(0);
@@ -3681,19 +3682,10 @@
// Block the writer and ensure they're queued.
BlockOnNextWrite();
clock_.AdvanceTime(DefaultRetransmissionTime());
- // Only one packet should be retransmitted.
- if (connection_.session_decides_what_to_write()) {
- if (GetQuicReloadableFlag(quic_treat_queued_packets_as_sent)) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
- } else {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
- }
+ if (GetQuicReloadableFlag(quic_treat_queued_packets_as_sent)) {
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
} else {
- if (GetQuicReloadableFlag(quic_treat_queued_packets_as_sent)) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
- } else {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
- }
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
}
connection_.GetRetransmissionAlarm()->Fire();
EXPECT_TRUE(connection_.HasQueuedData());
@@ -3702,21 +3694,11 @@
writer_->SetWritable();
clock_.AdvanceTime(QuicTime::Delta::FromMicroseconds(
2 * DefaultRetransmissionTime().ToMicroseconds()));
- // Retransmit already retransmitted packets event though the packet number
- // greater than the largest observed.
- if (connection_.session_decides_what_to_write()) {
- if (GetQuicReloadableFlag(quic_treat_queued_packets_as_sent)) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
- } else {
- // 2 RTOs + 1 TLP.
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(3);
- }
+ if (GetQuicReloadableFlag(quic_treat_queued_packets_as_sent)) {
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
} else {
- if (GetQuicReloadableFlag(quic_treat_queued_packets_as_sent)) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
- } else {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
- }
+ // 2 RTOs + 1 TLP, which is buggy.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(3);
}
connection_.GetRetransmissionAlarm()->Fire();
connection_.OnCanWrite();
@@ -9338,8 +9320,7 @@
TEST_P(QuicConnectionTest, RtoPacketAsTwo) {
if (!QuicConnectionPeer::GetSentPacketManager(&connection_)
->fix_rto_retransmission() ||
- connection_.PtoEnabled() ||
- GetQuicReloadableFlag(quic_grant_enough_credits)) {
+ connection_.PtoEnabled()) {
return;
}
connection_.SetMaxTailLossProbes(1);
@@ -9412,62 +9393,6 @@
EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet());
}
-TEST_P(QuicConnectionTest, RetransmitPacketAsTwo) {
- if (!connection_.session_decides_what_to_write() ||
- !GetQuicReloadableFlag(quic_grant_enough_credits)) {
- return;
- }
- connection_.SetMaxTailLossProbes(1);
- connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
- std::string stream_data(3000, 's');
- // Send packets 1 - 66 and exhaust cwnd.
- for (size_t i = 0; i < 22; ++i) {
- // 3 packets for each stream, the first 2 are guaranteed to be full packets.
- SendStreamDataToPeer(i + 2, stream_data, 0, FIN, nullptr);
- }
- CongestionBlockWrites();
-
- if (connection_.PtoEnabled()) {
- // Fires PTO, packets 1 and 2 are retransmitted as 4 packets.
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(67), _, _));
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(68), _, _));
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(69), _, _));
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(70), _, _));
- } else {
- // Fires TLP. Verify 2 packets gets sent.
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(67), _, _));
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(68), _, _));
- }
- connection_.GetRetransmissionAlarm()->Fire();
-
- if (connection_.PtoEnabled()) {
- // Fires PTO, packets 3 and 4 are retransmitted as 3 packets.
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(71), _, _));
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(72), _, _));
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(73), _, _));
- } else {
- // Fires RTO. Verify packets 2 and 3 are retransmitted as 3 packets.
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(69), _, _));
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(70), _, _));
- EXPECT_CALL(*send_algorithm_,
- OnPacketSent(_, _, QuicPacketNumber(71), _, _));
- }
- connection_.GetRetransmissionAlarm()->Fire();
- EXPECT_EQ(
- 0u, connection_.sent_packet_manager().pending_timer_transmission_count());
-}
-
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index a32a253..a747f0b 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -67,31 +67,6 @@
(unacked_packets_.perspective() == Perspective::IS_SERVER ? "Server: " \
: "Client: ")
-QuicSentPacketManager::ScopedCreditGrantor::ScopedCreditGrantor(
- QuicSentPacketManager* manager)
- : manager_(manager), credits_granted_(false) {
- if (!GetQuicReloadableFlag(quic_grant_enough_credits)) {
- return;
- }
- QUIC_RELOADABLE_FLAG_COUNT(quic_grant_enough_credits);
- if (manager_->pending_timer_transmission_count() > 1) {
- // There are enough credits to retransmit one packet.
- return;
- }
- // Grant 2 credits because a single packet can be transmitted as 2 (if packet
- // number length changes).
- manager_->set_pending_timer_transmission_count(2);
- credits_granted_ = true;
-}
-
-QuicSentPacketManager::ScopedCreditGrantor::~ScopedCreditGrantor() {
- if (!credits_granted_) {
- // Do not clear credits if there is no credit granted.
- return;
- }
- manager_->set_pending_timer_transmission_count(0);
-}
-
QuicSentPacketManager::QuicSentPacketManager(
Perspective perspective,
const QuicClock* clock,
@@ -555,7 +530,6 @@
// applications may want to use higher priority stream data for bandwidth
// probing, and some applications want to consider RTO is an indication of
// loss, etc.
- ScopedCreditGrantor grantor(this);
unacked_packets_.RetransmitFrames(*transmission_info, transmission_type);
return;
}
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index cc6f4b9..9e903fd 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -108,19 +108,6 @@
PTO_MODE,
};
- // Grantor makes sure there are enough credits to retransmit a packet. When
- // grantor is out of scope, remaining credits will be cleared.
- class QUIC_EXPORT_PRIVATE ScopedCreditGrantor {
- public:
- explicit ScopedCreditGrantor(QuicSentPacketManager* manager);
- ~ScopedCreditGrantor();
-
- private:
- QuicSentPacketManager* manager_;
- // Indicates whether any credit has been granted.
- bool credits_granted_;
- };
-
QuicSentPacketManager(Perspective perspective,
const QuicClock* clock,
QuicRandom* random,
@@ -386,10 +373,6 @@
return pending_timer_transmission_count_;
}
- void set_pending_timer_transmission_count(size_t count) {
- pending_timer_transmission_count_ = count;
- }
-
QuicTime::Delta peer_max_ack_delay() const { return peer_max_ack_delay_; }
void set_peer_max_ack_delay(QuicTime::Delta peer_max_ack_delay) {
