Closed Bug 1250250 Opened 7 years ago Closed 7 years ago
Store updated telemetry SEQ
_COUNT before starting the telemetry service
58 bytes, text/x-review-board-request
There's a very small chance that we get killed after starting the service but before updating the sequence count on disk (see bug 1249308 comment 4). It's better skip seq values than to duplicate them since if we hit a network error, we already skip seq values. Note that the update on disk should be blocking so we're sure we updated the value before we start the upload service.
Review commit: https://reviewboard.mozilla.org/r/35865/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/35865/
Attachment #8722117 - Flags: review?(mark.finkle)
Comment on attachment 8722117 [details] MozReview Request: Bug 1250250 - Store updated seq no. before sending upload Intent. r=mfinkle https://reviewboard.mozilla.org/r/35865/#review32537
Attachment #8722117 - Flags: review?(mark.finkle) → review+
https://reviewboard.mozilla.org/r/35865/#review32539 ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3981 (Diff revision 1) > + // We store synchronously before sending the Intent to ensure this sequence number will not be re-used. I wouldn't mind a blank line before the comment
https://hg.mozilla.org/integration/fx-team/rev/eaf669f3277786cbf0ee81610a9c4322f55484cb Bug 1250250 - Store updated seq no. before sending upload Intent. r=mfinkle
Comment on attachment 8722117 [details] MozReview Request: Bug 1250250 - Store updated seq no. before sending upload Intent. r=mfinkle Since the doc IDs are the same in the duplicate pings (bug 1249308 comment 5), it's unlikely that this is the cause for the duplicate pings – let's just uplift to aurora. Approval Request Comment [Feature/regressing bug #]: bug 1205835, unlikely bug 1249308 [User impact if declined]: Users in really unlikely cases may upload duplicate sequence numbers for telemetry since we start the upload attempt before we store the data. [Describe test coverage new/current, TreeHerder]: Tested locally. [Risks and why]: Extremely low – we move an existing line before the code to start the service. [String/UUID change made/needed]: None
Attachment #8722117 - Flags: approval-mozilla-aurora?
Marking affected, regression from 45. Should this uplift to beta?
Comment on attachment 8722117 [details] MozReview Request: Bug 1250250 - Store updated seq no. before sending upload Intent. r=mfinkle Fix for minor recent regression in telemetry, ok to uplift to aurora.
Attachment #8722117 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #7) > Marking affected, regression from 45. > Should this uplift to beta? Since we don't know for sure that this is causing issues and it's close to the merge date, I'm fine letting this ride the trains.
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.