Closed
Bug 1324558
Opened 8 years ago
Closed 7 years ago
Add detailed bookmark validation errors and stats collection
Categories
(Firefox for iOS :: Sync, defect, P1)
Tracking
()
RESOLVED
FIXED
Iteration:
1.22
Tracking | Status | |
---|---|---|
fxios | 9.0 | --- |
People
(Reporter: sleroux, Assigned: sleroux)
References
Details
(Whiteboard: [MobileCore][ios-sync])
Attachments
(1 file)
On iOS, we only capture problems with bookmark merging at a higher level than desktop: Missing Values, Missing Structure, Overlapping Structure, Parent ID Disagreement. The Sync Ping on desktop goes into more detail on these issues [1]. If we want to be able to understand the issues on iOS relative to other platforms in the sync ecosystem we should strive to report similar data for easier comparison.
[1] https://dxr.mozilla.org/mozilla-central/source/services/sync/modules/bookmark_validator.js#148
Assignee | ||
Comment 1•8 years ago
|
||
I vaguely remember talking about how we shouldn't expect to have the same data points between platforms but I can see us having value in compare apples w/ apples when looking at the sync data. Do you think this would be a requirement to get any valuable information from the Sync Ping?
Flags: needinfo?(rnewman)
Flags: needinfo?(markh)
Assignee | ||
Comment 2•8 years ago
|
||
For reference, iOS currently pokes the DB to get an idea of what could be happening:
https://github.com/mozilla-mobile/firefox-ios/blob/master/Storage/SQL/SQLiteBookmarksSyncing.swift#L773
Comment 3•8 years ago
|
||
(In reply to Stephan Leroux [:sleroux] from comment #1)
> I vaguely remember talking about how we shouldn't expect to have the same
> data points between platforms but I can see us having value in compare
> apples w/ apples when looking at the sync data. Do you think this would be a
> requirement to get any valuable information from the Sync Ping?
In theory it would be perfect if the validation results were identical, but in practice I think that will be difficult. There might be some things we can consolidate though which probably is valuable.
The desktop validator is at https://dxr.mozilla.org/mozilla-central/source/services/sync/modules/bookmark_validator.js and the comments near the top of that file show what we record. From a quick look, we probably have finer-grained info that iOS might have, but they were decided based purely on "what inconsistencies are possible" rather than "what inconsistencies lead to a particular bad outcome" (which IIUC is where iOS is coming from)
For example, I expect that what you record as "MissingStructure", in desktop might be a combination of "orphans", "multipleParents", "deletedParents", "duplicateChildren", "childrenOnNonFolder", "parentNotFolder", "clientMissing", "serverMissing", "serverDeleted" etc - however, I'm not convinced iOS should try and replicate all of them. Indeed, once we do a little more analysis, I expect we will find that many of these cluster (eg, an "orphan" is probably associated with, say, "serverDeleted" or "serverMissing" etc).
Given all that, I think that iOS should do the smallest necessary to expose its concept of "invalid", and we can allow our dashboards or other ad-hoc telemetry to try and group these into buckets based on that device submitted the ping. As a followup next year we can consider working with Android to come up with a set of validation issues that all platforms report consistently.
Flags: needinfo?(markh)
Updated•8 years ago
|
Flags: needinfo?(rnewman)
Comment 4•8 years ago
|
||
Note that it would be relatively straightforward to extend the set of validations on iOS — e.g., a query for children with multiple parents is very easy to express — and we will likely do so as part of repair, because we'd need to determine which records to fetch.
Assignee | ||
Updated•8 years ago
|
tracking-fxios:
--- → 7.0+
Whiteboard: [MobileCore]
Assignee | ||
Updated•8 years ago
|
Iteration: --- → 1.13
Priority: -- → P1
Updated•8 years ago
|
Iteration: 1.13 → 1.14
Updated•8 years ago
|
Iteration: 1.14 → 1.15
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Updated•8 years ago
|
Summary: Add detailed bookmark validation errors → Add detailed bookmark validation errors and stats collection
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → sleroux
Status: NEW → ASSIGNED
Iteration: 1.15 → 1.16
Updated•8 years ago
|
Whiteboard: [MobileCore] → [MobileCore][ios-sync]
Assignee | ||
Comment 5•8 years ago
|
||
Moving out of this sprint to focus on 7.0 stability issues.
Iteration: 1.16 → ---
Updated•7 years ago
|
Assignee | ||
Comment 6•7 years ago
|
||
Mostly contains changes to some of the validation code you wrote Edouard!
For the most part this follows the sync ping spec [1] - we just omit some properties like version/checked/failureReason.
[1] http://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/data/sync-ping.html#syncs-devices
Attachment #8868607 -
Flags: review?(eoger)
Assignee | ||
Updated•7 years ago
|
Iteration: --- → 1.22
Updated•7 years ago
|
Attachment #8868607 -
Flags: review?(eoger) → review+
Assignee | ||
Comment 7•7 years ago
|
||
master f75916c078c47a0c363bf27830cc49c9a6c36f89
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•