Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1661240 - Have same throttle backoff time for all bindings #1240

Merged
merged 2 commits into from
Oct 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions glean-core/csharp/Glean/Net/BaseUploader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -88,8 +89,6 @@ internal void TriggerUpload(Configuration config)
// 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)
Expand Down Expand Up @@ -126,7 +125,7 @@ internal void TriggerUpload(Configuration config)
}
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.
Expand Down
2 changes: 2 additions & 0 deletions glean-core/ios/Glean/Net/HttpPingUploader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -117,6 +118,7 @@ public class HttpPingUploader {
glean_process_ping_upload_response(&incomingTask, result.toFfi())
}
case .wait:
sleep(Constants.throttleBackoffMs)
brizental marked this conversation as resolved.
Show resolved Hide resolved
continue
case .done:
return
Expand Down
3 changes: 3 additions & 0 deletions glean-core/python/glean/glean.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ def _initialize_with_tempdir_for_testing(
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
badboy marked this conversation as resolved.
Show resolved Hide resolved

actual_data_dir = Path(tempfile.TemporaryDirectory().name)
cls.initialize(
application_id,
Expand Down
16 changes: 13 additions & 3 deletions glean-core/python/glean/net/ping_upload_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@


class PingUploadWorker:
_throttle_backoff_time = 60_000

@classmethod
def process(cls):
"""
Expand All @@ -43,7 +45,13 @@ def _process(cls):
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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion glean-core/python/tests/test_glean.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
from glean.net import PingUploadWorker
from glean.testing import _RecordingUploader


GLEAN_APP_ID = "glean-python-test"


Expand Down