gfe-relnote: In QUIC bbr sender, fix calculation of bytes_lost when gfe2_reloadable_flag_quic_one_bw_sample_per_ack_event is on. Protected by v2 flag gfe2_reloadable_flag_quic_one_bw_sample_per_ack_event2.

Also added a unit test to ensure bytes_lost is used to update recover window.

PiperOrigin-RevId: 289101409
Change-Id: I17ff916303af25a3967fb0559bbc50ec4ace4122

quic/core/congestion_control/bandwidth_sampler.cc

diff --git a/quic/core/congestion_control/bandwidth_sampler.cc b/quic/core/congestion_control/bandwidth_sampler.cc
index 21705a5..d4eeabe 100644
--- a/quic/core/congestion_control/bandwidth_sampler.cc
+++ b/quic/core/congestion_control/bandwidth_sampler.cc
@@ -87,7 +87,7 @@
         quic_bw_sampler_remove_packets_once_per_congestion_event2);
   }
   if (one_bw_sample_per_ack_event_) {
-    QUIC_RELOADABLE_FLAG_COUNT(quic_one_bw_sample_per_ack_event);
+    QUIC_RELOADABLE_FLAG_COUNT(quic_one_bw_sample_per_ack_event2);
   }
 }
 

quic/core/congestion_control/bandwidth_sampler.h

diff --git a/quic/core/congestion_control/bandwidth_sampler.h b/quic/core/congestion_control/bandwidth_sampler.h
index d93af36..df63fa5 100644
--- a/quic/core/congestion_control/bandwidth_sampler.h
+++ b/quic/core/congestion_control/bandwidth_sampler.h
@@ -481,10 +481,10 @@
       quic_bw_sampler_remove_packets_once_per_congestion_event2);
 
   // Latched value of quic_bw_sampler_remove_packets_once_per_congestion_event2
-  // and quic_one_bw_sample_per_ack_event.
+  // and quic_one_bw_sample_per_ack_event2.
   const bool one_bw_sample_per_ack_event_ =
       remove_packets_once_per_congestion_event_ &&
-      GetQuicReloadableFlag(quic_one_bw_sample_per_ack_event);
+      GetQuicReloadableFlag(quic_one_bw_sample_per_ack_event2);
 };
 
 }  // namespace quic

quic/core/congestion_control/bbr_sender.cc

diff --git a/quic/core/congestion_control/bbr_sender.cc b/quic/core/congestion_control/bbr_sender.cc
index f8d22db..f80d4ce 100644
--- a/quic/core/congestion_control/bbr_sender.cc
+++ b/quic/core/congestion_control/bbr_sender.cc
@@ -443,8 +443,8 @@
     if (!sample.sample_rtt.IsInfinite()) {
       min_rtt_expired = MaybeUpdateMinRtt(event_time, sample.sample_rtt);
     }
+    bytes_lost = sampler_.total_bytes_lost() - total_bytes_lost_before;
     if (mode_ == STARTUP) {
-      bytes_lost = sampler_.total_bytes_lost() - total_bytes_lost_before;
       if (stats_) {
         stats_->slowstart_packets_lost += lost_packets.size();
         stats_->slowstart_bytes_lost += bytes_lost;

quic/core/congestion_control/bbr_sender_test.cc

diff --git a/quic/core/congestion_control/bbr_sender_test.cc b/quic/core/congestion_control/bbr_sender_test.cc
index ddb1077..2ff1f8a 100644
--- a/quic/core/congestion_control/bbr_sender_test.cc
+++ b/quic/core/congestion_control/bbr_sender_test.cc
@@ -11,6 +11,7 @@
 
 #include "net/third_party/quiche/src/quic/core/congestion_control/rtt_stats.h"
 #include "net/third_party/quiche/src/quic/core/quic_packets.h"
+#include "net/third_party/quiche/src/quic/core/quic_types.h"
 #include "net/third_party/quiche/src/quic/core/quic_utils.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
@@ -377,6 +378,54 @@
   EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited);
 }
 
+TEST_F(BbrSenderTest, RemoveBytesLostInRecovery) {
+  SetQuicReloadableFlag(quic_bbr_one_mss_conservation, false);
+  // Disable Ack Decimation on the receiver, because it can increase srtt.
+  QuicConnectionPeer::SetAckMode(receiver_.connection(), AckMode::TCP_ACKING);
+  CreateDefaultSetup();
+
+  DriveOutOfStartup();
+
+  // Drop a packet to enter recovery.
+  receiver_.DropNextIncomingPacket();
+  ASSERT_TRUE(
+      simulator_.RunUntilOrTimeout([this]() { return sender_->InRecovery(); },
+                                   QuicTime::Delta::FromSeconds(30)));
+
+  QuicUnackedPacketMap* unacked_packets =
+      QuicSentPacketManagerPeer::GetUnackedPacketMap(
+          QuicConnectionPeer::GetSentPacketManager(bbr_sender_.connection()));
+  QuicPacketNumber largest_sent =
+      bbr_sender_.connection()->sent_packet_manager().GetLargestSentPacket();
+  // least_inflight is the smallest inflight packet.
+  QuicPacketNumber least_inflight =
+      bbr_sender_.connection()->sent_packet_manager().GetLeastUnacked();
+  while (!unacked_packets->GetTransmissionInfo(least_inflight).in_flight) {
+    ASSERT_LE(least_inflight, largest_sent);
+    least_inflight++;
+  }
+  QuicPacketLength least_inflight_packet_size =
+      unacked_packets->GetTransmissionInfo(least_inflight).bytes_sent;
+  QuicByteCount prior_recovery_window =
+      sender_->ExportDebugState().recovery_window;
+  QuicByteCount prior_inflight = unacked_packets->bytes_in_flight();
+  QUIC_LOG(INFO) << "Recovery window:" << prior_recovery_window
+                 << ", least_inflight_packet_size:"
+                 << least_inflight_packet_size
+                 << ", bytes_in_flight:" << prior_inflight;
+  ASSERT_GT(prior_recovery_window, least_inflight_packet_size);
+
+  // Lose the least inflight packet and expect the recovery window to drop.
+  unacked_packets->RemoveFromInFlight(least_inflight);
+  LostPacketVector lost_packets;
+  lost_packets.emplace_back(least_inflight, least_inflight_packet_size);
+  sender_->OnCongestionEvent(false, prior_inflight, clock_->Now(), {},
+                             lost_packets);
+  EXPECT_EQ(sender_->ExportDebugState().recovery_window,
+            prior_inflight - least_inflight_packet_size);
+  EXPECT_LT(sender_->ExportDebugState().recovery_window, prior_recovery_window);
+}
+
 // Test a simple long data transfer with 2 rtts of aggregation.
 TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationBytes) {
   CreateDefaultSetup();