Closed
Bug 1250250
Opened 9 years ago
Closed 9 years ago
Store updated telemetry SEQ_COUNT before starting the telemetry service
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox45+ wontfix, firefox46+ fixed, firefox47+ fixed, fennec46+)
RESOLVED
FIXED
Firefox 47
People
(Reporter: mcomella, Assigned: mcomella)
References
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
mfinkle
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details |
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.
Assignee | ||
Comment 1•9 years ago
|
||
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 2•9 years ago
|
||
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+
Comment 3•9 years ago
|
||
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
Assignee | ||
Comment 4•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/eaf669f3277786cbf0ee81610a9c4322f55484cb
Bug 1250250 - Store updated seq no. before sending upload Intent. r=mfinkle
Assignee | ||
Comment 5•9 years ago
|
||
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?
Comment 6•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Marking affected, regression from 45.
Should this uplift to beta?
status-firefox45:
--- → affected
status-firefox46:
--- → affected
tracking-firefox45:
--- → +
tracking-firefox46:
--- → +
tracking-firefox47:
--- → +
Flags: needinfo?(michael.l.comella)
Keywords: regression
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+
Comment 9•9 years ago
|
||
bugherder uplift |
Assignee | ||
Comment 10•9 years ago
|
||
(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.
Updated•8 years ago
|
Version: unspecified → 45 Branch
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•