Add maximum number of retries to PingUploadWorker #953
Conversation
| } | ||
| PingUploadTask.Wait -> return Result.retry() | ||
| PingUploadTask.Done -> return Result.success() | ||
| } | ||
| } while (true) | ||
| } while (uploadFailures < MAX_RETRIES) | ||
| return Result.failure() |
There was a problem hiding this comment.
Unsure if this should really be a failure or a retry.
There was a problem hiding this comment.
We use a OneTimeWorkRequest which defaults to exponential backoff on retry.
I think failure is the right thing here. If we fail too often, we don't want to reschedule it at all. When a ping gets submitted again (e.g. next time the app is backgrounded) any pending pings are picked up again anyway.
There was a problem hiding this comment.
cc @Dexterp37 just for a quick look
There was a problem hiding this comment.
Works for me, assuming failure doesn't mean deleting the pings
There was a problem hiding this comment.
Nope, it's only marking the worker as failed. The pings are handled inside glean-core and will be retried by the next worker.
glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/PingUploadWorker.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Code wise good to go, let's double check with @Dexterp37 quickly.
We should also land this in the iOS part, as that's deployed in Firefox for iOS.
Once that's done we should release.
|
Should we file a follow up bug for Swift and Python? |
|
Fixes Bug 1644290
This is a temporary fix so we can stop this issue.
I opened a bug to move this retrying logic to the Rust side: https://bugzilla.mozilla.org/show_bug.cgi?id=1644364