From 9674cdd2d60d7535cc6cb1f327184c22de0cc717 Mon Sep 17 00:00:00 2001 From: brizental Date: Tue, 9 Jun 2020 15:21:55 +0200 Subject: [PATCH 1/3] Limit the number of upload retries in iOS --- glean-core/ios/Glean/Net/HttpPingUploader.swift | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/glean-core/ios/Glean/Net/HttpPingUploader.swift b/glean-core/ios/Glean/Net/HttpPingUploader.swift index a0e67f077e..a88177692c 100644 --- a/glean-core/ios/Glean/Net/HttpPingUploader.swift +++ b/glean-core/ios/Glean/Net/HttpPingUploader.swift @@ -18,6 +18,9 @@ public class HttpPingUploader { static let recoverableErrorStatusCode: UInt16 = 500 // For this error, the ping data will be deleted and no retry happens static let unrecoverableErrorStatusCode: UInt16 = 400 + + // Maximum number of recoverable errors allowed before aborting the ping uploader + static let maxRetries = 3 } private let logger = Logger(tag: Constants.logTag) @@ -99,7 +102,8 @@ public class HttpPingUploader { /// It will report back the task status to Glean, which will take care of deleting pending ping files. /// It will continue upload as long as it can fetch new tasks. func process() { - while true { + var upload_failures = 0; + while upload_failures < Constants.maxRetries { var incomingTask = FfiPingUploadTask() glean_get_upload_task(&incomingTask, config.logPings.toByte()) let task = incomingTask.toPingUploadTask() @@ -107,6 +111,9 @@ public class HttpPingUploader { switch task { case let .upload(request): self.upload(path: request.path, data: request.body, headers: request.headers) { result in + if case .recoverableFailure = result { + upload_failures += 1 + } glean_process_ping_upload_response(&incomingTask, result.toFfi()) } case .wait: From d2735051741d95c0cdfb70f2e4c3ec80862426b3 Mon Sep 17 00:00:00 2001 From: brizental Date: Tue, 9 Jun 2020 15:22:53 +0200 Subject: [PATCH 2/3] Limit the number of upload retries in Python --- glean-core/python/glean/net/ping_upload_worker.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/glean-core/python/glean/net/ping_upload_worker.py b/glean-core/python/glean/net/ping_upload_worker.py index 707183ca1f..b897ce92c1 100644 --- a/glean-core/python/glean/net/ping_upload_worker.py +++ b/glean-core/python/glean/net/ping_upload_worker.py @@ -16,6 +16,7 @@ from .._glean_ffi import ffi as ffi_support # type: ignore from .._dispatcher import Dispatcher from .._process_dispatcher import ProcessDispatcher +from .ping_uploader import RecoverableFailure log = logging.getLogger(__name__) @@ -24,6 +25,8 @@ # How many times to attempt waiting when told to by glean-core's upload API. MAX_WAIT_ATTEMPTS = 3 +# Maximum number of recoverable errors allowed before aborting the ping uploader +MAX_RETRIES = 3 class PingUploadWorker: @classmethod @@ -112,7 +115,9 @@ def _process(data_dir: Path, configuration) -> bool: wait_attempts = 0 - while True: + upload_failures = 0 + + while upload_failures < MAX_RETRIES: incoming_task = ffi_support.new("FfiPingUploadTask *") _ffi.lib.glean_get_upload_task(incoming_task, configuration.log_pings) @@ -136,6 +141,9 @@ def _process(data_dir: Path, configuration) -> bool: url_path, body, _parse_ping_headers(headers, doc_id), configuration ) + if isinstance(upload_result, RecoverableFailure): + upload_failures = upload_failures + 1 + # Process the response. _ffi.lib.glean_process_ping_upload_response( incoming_task, upload_result.to_ffi() From 7798f6995f536407fe02c5489bd8f1a3c4a06c8a Mon Sep 17 00:00:00 2001 From: brizental Date: Tue, 9 Jun 2020 17:32:47 +0200 Subject: [PATCH 3/3] fix lints --- glean-core/ios/Glean/Net/HttpPingUploader.swift | 8 ++++---- glean-core/python/glean/net/ping_upload_worker.py | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/glean-core/ios/Glean/Net/HttpPingUploader.swift b/glean-core/ios/Glean/Net/HttpPingUploader.swift index a88177692c..70e17b018f 100644 --- a/glean-core/ios/Glean/Net/HttpPingUploader.swift +++ b/glean-core/ios/Glean/Net/HttpPingUploader.swift @@ -18,7 +18,7 @@ public class HttpPingUploader { static let recoverableErrorStatusCode: UInt16 = 500 // For this error, the ping data will be deleted and no retry happens static let unrecoverableErrorStatusCode: UInt16 = 400 - + // Maximum number of recoverable errors allowed before aborting the ping uploader static let maxRetries = 3 } @@ -102,8 +102,8 @@ public class HttpPingUploader { /// It will report back the task status to Glean, which will take care of deleting pending ping files. /// It will continue upload as long as it can fetch new tasks. func process() { - var upload_failures = 0; - while upload_failures < Constants.maxRetries { + var uploadFailures = 0 + while uploadFailures < Constants.maxRetries { var incomingTask = FfiPingUploadTask() glean_get_upload_task(&incomingTask, config.logPings.toByte()) let task = incomingTask.toPingUploadTask() @@ -112,7 +112,7 @@ public class HttpPingUploader { case let .upload(request): self.upload(path: request.path, data: request.body, headers: request.headers) { result in if case .recoverableFailure = result { - upload_failures += 1 + uploadFailures += 1 } glean_process_ping_upload_response(&incomingTask, result.toFfi()) } diff --git a/glean-core/python/glean/net/ping_upload_worker.py b/glean-core/python/glean/net/ping_upload_worker.py index b897ce92c1..1a729e4632 100644 --- a/glean-core/python/glean/net/ping_upload_worker.py +++ b/glean-core/python/glean/net/ping_upload_worker.py @@ -28,6 +28,7 @@ # Maximum number of recoverable errors allowed before aborting the ping uploader MAX_RETRIES = 3 + class PingUploadWorker: @classmethod def process(cls):