From 2261845761251d91b6968f29846dc3aabbc0cc45 Mon Sep 17 00:00:00 2001
From: Beatriz Rizental <brizental@mozilla.com>
Date: Tue, 13 Oct 2020 10:52:57 +0200
Subject: [PATCH] Bug 1661240 - Have same throttle backoff time for all
 bindings (#1240)

---
 CHANGELOG.md                                     |  1 +
 glean-core/csharp/Glean/Net/BaseUploader.cs      |  5 ++---
 glean-core/ios/Glean/Net/HttpPingUploader.swift  |  2 ++
 glean-core/python/glean/glean.py                 |  3 +++
 .../python/glean/net/ping_upload_worker.py       | 16 +++++++++++++---
 glean-core/python/tests/test_glean.py            |  1 -
 6 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index eeb6ab487..e45d34ab1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 [Full changelog](https://github.com/mozilla/glean/compare/v33.0.4...main)
 
 * General
+  * Standardize throttle backoff time throughout all bindings. ([#1240](https://github.com/mozilla/glean/pull/1240))
   * Update `glean_parser` to 1.29.0
     * Generated code now includes a comment next to each metric containing the name of the metric in its original `snake_case` form.
 * Android
diff --git a/glean-core/csharp/Glean/Net/BaseUploader.cs b/glean-core/csharp/Glean/Net/BaseUploader.cs
index 4f9684146..0111daaa8 100644
--- a/glean-core/csharp/Glean/Net/BaseUploader.cs
+++ b/glean-core/csharp/Glean/Net/BaseUploader.cs
@@ -19,6 +19,7 @@ namespace Mozilla.Glean.Net
     /// </summary>
     internal class BaseUploader
     {
+        internal const int THROTTLED_BACKOFF_MS = 60_000;
         private readonly IPingUploader uploader;
 
         /// <summary>
@@ -88,8 +89,6 @@ namespace Mozilla.Glean.Net
             // TODO: must not work like this, it should work off the main thread.
             // FOR TESTING Implement the upload worker here and call this from Glean.cs
 
-            int waitAttempts = 0;
-
             // Limits are enforced by glean-core to avoid an inifinite loop here.
             // Whenever a limit is reached, this binding will receive `UploadTaskTag.Done` and step out.
             while (true)
@@ -126,7 +125,7 @@ namespace Mozilla.Glean.Net
                         }
                         break;
                     case UploadTaskTag.Wait:
-                        Thread.Sleep(100);
+                        Thread.Sleep(THROTTLED_BACKOFF_MS);
                         break;
                     case UploadTaskTag.Done:
                         // Nothing to do here, break out of the loop.
diff --git a/glean-core/ios/Glean/Net/HttpPingUploader.swift b/glean-core/ios/Glean/Net/HttpPingUploader.swift
index 9d8a8e758..06431ddd8 100644
--- a/glean-core/ios/Glean/Net/HttpPingUploader.swift
+++ b/glean-core/ios/Glean/Net/HttpPingUploader.swift
@@ -13,6 +13,7 @@ public class HttpPingUploader {
         // Since ping file names are UUIDs, this matches UUIDs for filtering purposes
         static let logTag = "glean/HttpPingUploader"
         static let connectionTimeout = 10000
+        static let throttleBackoffMs: UInt32 = 60_000
 
         // For this error, the ping will be retried later
         static let recoverableErrorStatusCode: UInt16 = 500
@@ -117,6 +118,7 @@ public class HttpPingUploader {
                     glean_process_ping_upload_response(&incomingTask, result.toFfi())
                 }
             case .wait:
+                sleep(Constants.throttleBackoffMs)
                 continue
             case .done:
                 return
diff --git a/glean-core/python/glean/glean.py b/glean-core/python/glean/glean.py
index 1f901b507..e3e31ac81 100644
--- a/glean-core/python/glean/glean.py
+++ b/glean-core/python/glean/glean.py
@@ -230,6 +230,9 @@ class Glean:
         The temporary directory will be destroyed when Glean is initialized
         again or at process shutdown.
         """
+        # Lower throttle backoff time for testing
+        PingUploadWorker._throttle_backoff_time = 1
+
         actual_data_dir = Path(tempfile.TemporaryDirectory().name)
         cls.initialize(
             application_id,
diff --git a/glean-core/python/glean/net/ping_upload_worker.py b/glean-core/python/glean/net/ping_upload_worker.py
index 04c69f8e3..4c9834395 100644
--- a/glean-core/python/glean/net/ping_upload_worker.py
+++ b/glean-core/python/glean/net/ping_upload_worker.py
@@ -23,6 +23,8 @@ log = logging.getLogger(__name__)
 
 
 class PingUploadWorker:
+    _throttle_backoff_time = 60_000
+
     @classmethod
     def process(cls):
         """
@@ -43,7 +45,13 @@ class PingUploadWorker:
         from .. import Glean
 
         return ProcessDispatcher.dispatch(
-            _process, (Glean._data_dir, Glean._application_id, Glean._configuration)
+            _process,
+            (
+                Glean._data_dir,
+                Glean._application_id,
+                Glean._configuration,
+                cls._throttle_backoff_time,
+            ),
         )
 
     @classmethod
@@ -94,7 +102,9 @@ def _parse_ping_headers(
     return headers
 
 
-def _process(data_dir: Path, application_id: str, configuration) -> bool:
+def _process(
+    data_dir: Path, application_id: str, configuration, throttle_backoff_time: int
+) -> bool:
 
     # Import here to avoid cyclical import
     from ..glean import Glean
@@ -144,7 +154,7 @@ def _process(data_dir: Path, application_id: str, configuration) -> bool:
                 incoming_task, upload_result.to_ffi()
             )
         elif tag == UploadTaskTag.WAIT:
-            time.sleep(1)
+            time.sleep(throttle_backoff_time)
         elif tag == UploadTaskTag.DONE:
             return True
 
diff --git a/glean-core/python/tests/test_glean.py b/glean-core/python/tests/test_glean.py
index 1df3b7343..92206b7ed 100644
--- a/glean-core/python/tests/test_glean.py
+++ b/glean-core/python/tests/test_glean.py
@@ -37,7 +37,6 @@ from glean.metrics import (
 from glean.net import PingUploadWorker
 from glean.testing import _RecordingUploader
 
-
 GLEAN_APP_ID = "glean-python-test"