From d742382eb0c576efd7ac2fac0e239177b31cc52b Mon Sep 17 00:00:00 2001
From: Per Kjellander <perkj@webrtc.org>
Date: Fri, 18 Nov 2022 12:47:29 +0100
Subject: [PATCH] Limit numer of pending probes.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created probes are currently timed out after 5s. But to be safe, also limit the number of pending probes to 5.

Bug: webrtc:14392, b/259541308
Change-Id: Ibf630704ffe14cb165ab849b881cf75857376f82
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/284080
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38697}
---
 modules/pacing/bitrate_prober.cc          |  6 ++--
 modules/pacing/bitrate_prober_unittest.cc | 37 +++++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/modules/pacing/bitrate_prober.cc b/modules/pacing/bitrate_prober.cc
index e01c3ae5a9..ce45ea4e7a 100644
--- a/modules/pacing/bitrate_prober.cc
+++ b/modules/pacing/bitrate_prober.cc
@@ -24,6 +24,7 @@ namespace webrtc {
 
 namespace {
 constexpr TimeDelta kProbeClusterTimeout = TimeDelta::Seconds(5);
+constexpr size_t kMaxPendingProbeClusters = 5;
 
 }  // namespace
 
@@ -84,8 +85,9 @@ void BitrateProber::CreateProbeCluster(
 
   total_probe_count_++;
   while (!clusters_.empty() &&
-         cluster_config.at_time - clusters_.front().requested_at >
-             kProbeClusterTimeout) {
+         (cluster_config.at_time - clusters_.front().requested_at >
+              kProbeClusterTimeout ||
+          clusters_.size() > kMaxPendingProbeClusters)) {
     clusters_.pop();
     total_failed_probe_count_++;
   }
diff --git a/modules/pacing/bitrate_prober_unittest.cc b/modules/pacing/bitrate_prober_unittest.cc
index 00f84e69f1..3be7d2d99e 100644
--- a/modules/pacing/bitrate_prober_unittest.cc
+++ b/modules/pacing/bitrate_prober_unittest.cc
@@ -13,6 +13,8 @@
 #include <algorithm>
 
 #include "api/units/data_rate.h"
+#include "api/units/time_delta.h"
+#include "api/units/timestamp.h"
 #include "test/explicit_key_value_config.h"
 #include "test/gtest.h"
 
@@ -143,6 +145,41 @@ TEST(BitrateProberTest, DiscardsDelayedProbes) {
   EXPECT_FALSE(prober.CurrentCluster(now).has_value());
 }
 
+TEST(BitrateProberTest, LimitsNumberOfPendingProbeClusters) {
+  const FieldTrialBasedConfig config;
+  BitrateProber prober(config);
+  const DataSize kProbeSize = DataSize::Bytes(1000);
+  Timestamp now = Timestamp::Zero();
+  prober.CreateProbeCluster({.at_time = now,
+                             .target_data_rate = DataRate::KilobitsPerSec(900),
+                             .target_duration = TimeDelta::Millis(15),
+                             .target_probe_count = 5,
+                             .id = 0});
+  prober.OnIncomingPacket(kProbeSize);
+  ASSERT_TRUE(prober.is_probing());
+  ASSERT_EQ(prober.CurrentCluster(now)->probe_cluster_id, 0);
+
+  for (int i = 1; i < 11; ++i) {
+    prober.CreateProbeCluster(
+        {.at_time = now,
+         .target_data_rate = DataRate::KilobitsPerSec(900),
+         .target_duration = TimeDelta::Millis(15),
+         .target_probe_count = 5,
+         .id = i});
+    prober.OnIncomingPacket(kProbeSize);
+  }
+  // Expect some clusters has been dropped.
+  EXPECT_TRUE(prober.is_probing());
+  EXPECT_GE(prober.CurrentCluster(now)->probe_cluster_id, 5);
+
+  Timestamp max_expected_probe_time = now + TimeDelta::Seconds(1);
+  while (prober.is_probing() && now < max_expected_probe_time) {
+    now = std::max(now, prober.NextProbeTime(now));
+    prober.ProbeSent(now, kProbeSize);
+  }
+  EXPECT_FALSE(prober.is_probing());
+}
+
 TEST(BitrateProberTest, DoesntInitializeProbingForSmallPackets) {
   const FieldTrialBasedConfig config;
   BitrateProber prober(config);