55 bytes, text/x-github-pull-request
|Details | Review|
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 . 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.  https://dxr.mozilla.org/mozilla-central/source/services/sync/modules/bookmark_validator.js#148
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?
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
(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.
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.
Summary: Add detailed bookmark validation errors → Add detailed bookmark validation errors and stats collection
Assignee: nobody → sleroux
Status: NEW → ASSIGNED
Iteration: 1.15 → 1.16
Moving out of this sprint to focus on 7.0 stability issues.
Iteration: 1.16 → ---
Mostly contains changes to some of the validation code you wrote Edouard! For the most part this follows the sync ping spec  - we just omit some properties like version/checked/failureReason.  http://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/data/sync-ping.html#syncs-devices
Attachment #8868607 - Flags: review?(eoger)
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.