gfe-relnote: In QUIC, do not send STOP_WAITING if no_stop_waiting_frame_ is true. Protected by gfe2_reloadable_flag_quic_simplify_stop_waiting.
This change is from cl/214914333.
PiperOrigin-RevId: 249558708
Change-Id: I48d5045185980ecc0264ad6a9dff1e47f69a6d20
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 73959f0..2260766 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -1205,7 +1205,13 @@
// If the incoming ack's packets set expresses received packets: peer is still
// acking packets which we never care about.
// Send an ack to raise the high water mark.
- PostProcessAfterAckFrame(GetLeastUnacked() > start,
+ bool send_stop_waiting = GetLeastUnacked() > start;
+ if (GetQuicReloadableFlag(quic_simplify_stop_waiting) &&
+ no_stop_waiting_frames_) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_simplify_stop_waiting);
+ send_stop_waiting = false;
+ }
+ PostProcessAfterAckFrame(send_stop_waiting,
ack_result == PACKETS_NEWLY_ACKED);
processing_ack_frame_ = false;
@@ -3910,8 +3916,6 @@
SetRetransmissionAlarm();
MaybeSetPathDegradingAlarm(acked_new_packet);
- // TODO(ianswett): Only increment stop_waiting_count_ if StopWaiting frames
- // are sent.
if (send_stop_waiting) {
++stop_waiting_count_;
} else {
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 3d421f5..3126e0b 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1287,6 +1287,7 @@
QuicPacketCount num_packets_received_since_last_ack_sent_;
// Indicates how many consecutive times an ack has arrived which indicates
// the peer needs to stop waiting for some packets.
+ // TODO(fayang): remove this when deprecating quic_simplify_stop_waiting.
int stop_waiting_count_;
// TODO(fayang): Remove ack_mode_, ack_decimation_delay_,
// unlimited_ack_decimation_, fast_ack_after_quiescence_ when deprecating
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 3e03e1f..ebfaeaa 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -883,11 +883,11 @@
{AckResponse::kDefer, AckResponse::kImmediate}) {
for (bool no_stop_waiting : {true, false}) {
// After version 43, never use STOP_WAITING.
- if (all_supported_versions[i].transport_version <= QUIC_VERSION_43 ||
- no_stop_waiting) {
- params.push_back(TestParams(all_supported_versions[i], ack_response,
- no_stop_waiting));
- }
+ params.push_back(TestParams(
+ all_supported_versions[i], ack_response,
+ all_supported_versions[i].transport_version <= QUIC_VERSION_43
+ ? no_stop_waiting
+ : true));
}
}
}
@@ -2622,15 +2622,25 @@
ProcessAckPacket(&frame2);
EXPECT_CALL(*send_algorithm_,
OnPacketSent(_, _, _, _, HAS_RETRANSMITTABLE_DATA));
- connection_.SendStreamDataWithString(3, "foo", 9, NO_FIN);
+ connection_.SendStreamDataWithString(3, "foofoofoo", 9, NO_FIN);
// Ack bundled.
if (GetParam().no_stop_waiting) {
- EXPECT_EQ(2u, writer_->frame_count());
+ if (GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+ // Do not ACK acks.
+ EXPECT_EQ(1u, writer_->frame_count());
+ } else {
+ EXPECT_EQ(2u, writer_->frame_count());
+ }
} else {
EXPECT_EQ(3u, writer_->frame_count());
}
EXPECT_EQ(1u, writer_->stream_frames().size());
- EXPECT_FALSE(writer_->ack_frames().empty());
+ if (GetParam().no_stop_waiting &&
+ GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+ EXPECT_TRUE(writer_->ack_frames().empty());
+ } else {
+ EXPECT_FALSE(writer_->ack_frames().empty());
+ }
// But an ack with no missing packets will not send an ack.
AckPacket(original, &frame2);
@@ -6496,14 +6506,25 @@
// Check that ack is bundled with outgoing data and the delayed ack
// alarm is reset.
if (GetParam().no_stop_waiting) {
- EXPECT_EQ(2u, writer_->frame_count());
- EXPECT_TRUE(writer_->stop_waiting_frames().empty());
+ if (GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+ // Do not ACK acks.
+ EXPECT_EQ(1u, writer_->frame_count());
+ } else {
+ EXPECT_EQ(2u, writer_->frame_count());
+ EXPECT_TRUE(writer_->stop_waiting_frames().empty());
+ }
} else {
EXPECT_EQ(3u, writer_->frame_count());
EXPECT_FALSE(writer_->stop_waiting_frames().empty());
}
- EXPECT_FALSE(writer_->ack_frames().empty());
- EXPECT_EQ(QuicPacketNumber(3u), LargestAcked(writer_->ack_frames().front()));
+ if (GetParam().no_stop_waiting &&
+ GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+ EXPECT_TRUE(writer_->ack_frames().empty());
+ } else {
+ EXPECT_FALSE(writer_->ack_frames().empty());
+ EXPECT_EQ(QuicPacketNumber(3u),
+ LargestAcked(writer_->ack_frames().front()));
+ }
EXPECT_EQ(1u, writer_->stream_frames().size());
EXPECT_FALSE(connection_.GetAckAlarm()->IsSet());
}
@@ -8094,7 +8115,12 @@
connection_.GetPingAlarm()->Fire();
size_t padding_frame_count = writer_->padding_frames().size();
if (GetParam().no_stop_waiting) {
- EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count());
+ if (GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+ // Do not ACK acks.
+ EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count());
+ } else {
+ EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count());
+ }
} else {
EXPECT_EQ(padding_frame_count + 3u, writer_->frame_count());
}
