commit 0335e4eaf81e71648c7cabd591afd918acdbc2cd
author ianswett <ianswett@google.com> Thu Jan 16 17:23:38 2020 -0800
committer Copybara-Service <copybara-worker@google.com> Thu Jan 16 17:24:08 2020 -0800
tree d4563f9d665c700671e556f75e80508cc2e56f52
parent 9af2484f9efefa78d33664a083f7c1629088daa5

gfe-relnote: Ensure the ACK delay is at least the alarm granularity(1ms) when ACK decimation is enabled.  Protected by gfe2_reloadable_flag_quic_ack_delay_alarm_granularity.

PiperOrigin-RevId: 290179167
Change-Id: I71fdac347c6e8637a481280b3aa9bb40a96c22df

quic/core/quic_received_packet_manager.cc

diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc
index 86b060e..9e33ea4 100644
--- a/quic/core/quic_received_packet_manager.cc
+++ b/quic/core/quic_received_packet_manager.cc
@@ -245,6 +245,10 @@
     // before sending an ack.
     QuicTime::Delta ack_delay = std::min(
         local_max_ack_delay_, rtt_stats->min_rtt() * ack_decimation_delay_);
+    if (GetQuicReloadableFlag(quic_ack_delay_alarm_granularity)) {
+      QUIC_RELOADABLE_FLAG_COUNT(quic_ack_delay_alarm_granularity);
+      ack_delay = std::max(ack_delay, kAlarmGranularity);
+    }
     if (fast_ack_after_quiescence_ && now - time_of_previous_received_packet_ >
                                           rtt_stats->SmoothedOrInitialRtt()) {
       // Ack the first packet out of queiscence faster, because QUIC does

quic/core/quic_received_packet_manager_test.cc

diff --git a/quic/core/quic_received_packet_manager_test.cc b/quic/core/quic_received_packet_manager_test.cc
index fd27663..7355847 100644
--- a/quic/core/quic_received_packet_manager_test.cc
+++ b/quic/core/quic_received_packet_manager_test.cc
@@ -11,7 +11,9 @@
 #include "net/third_party/quiche/src/quic/core/congestion_control/rtt_stats.h"
 #include "net/third_party/quiche/src/quic/core/crypto/crypto_protocol.h"
 #include "net/third_party/quiche/src/quic/core/quic_connection_stats.h"
+#include "net/third_party/quiche/src/quic/core/quic_constants.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
 #include "net/third_party/quiche/src/quic/test_tools/mock_clock.h"
 
@@ -415,6 +417,43 @@
   CheckAckTimeout(clock_.ApproximateNow());
 }
 
+TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationMin1ms) {
+  if (!GetQuicReloadableFlag(quic_ack_delay_alarm_granularity)) {
+    return;
+  }
+  EXPECT_FALSE(HasPendingAck());
+  QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION);
+  // Seed the min_rtt with a kAlarmGranularity signal.
+  rtt_stats_.UpdateRtt(kAlarmGranularity, QuicTime::Delta::Zero(),
+                       clock_.ApproximateNow());
+  // The ack time should be based on kAlarmGranularity, since the RTT is 1ms.
+  QuicTime ack_time = clock_.ApproximateNow() + kAlarmGranularity;
+
+  // Process all the packets in order so there aren't missing packets.
+  uint64_t kFirstDecimatedPacket = 101;
+  for (uint64_t i = 1; i < kFirstDecimatedPacket; ++i) {
+    RecordPacketReceipt(i, clock_.ApproximateNow());
+    MaybeUpdateAckTimeout(kInstigateAck, i);
+    if (i % 2 == 0) {
+      // Ack every 2 packets by default.
+      CheckAckTimeout(clock_.ApproximateNow());
+    } else {
+      CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
+    }
+  }
+
+  RecordPacketReceipt(kFirstDecimatedPacket, clock_.ApproximateNow());
+  MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket);
+  CheckAckTimeout(ack_time);
+
+  // The 10th received packet causes an ack to be sent.
+  for (uint64_t i = 1; i < 10; ++i) {
+    RecordPacketReceipt(kFirstDecimatedPacket + i, clock_.ApproximateNow());
+    MaybeUpdateAckTimeout(kInstigateAck, kFirstDecimatedPacket + i);
+  }
+  CheckAckTimeout(clock_.ApproximateNow());
+}
+
 TEST_P(QuicReceivedPacketManagerTest,
        SendDelayedAckAckDecimationAfterQuiescence) {
   EXPECT_FALSE(HasPendingAck());