Closed
Bug 851380
Opened 12 years ago
Closed 4 years ago
Switch to an incrementing version count for changes, rather than timestamps
Categories
(Firefox for Android Graveyard :: Android Sync, defect)
Firefox for Android Graveyard
Android Sync
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: rnewman, Unassigned)
Details
We already know that system clocks are evil: we regularly see user data submissions in FHR and Product Announcements that reveal clocks wrong by anywhere from seconds to years, both ahead and behind.
Syncing user data using local timestamps as boundaries (as Sync does on Android, and PICL likely will on desktop) only works so long as clocks are monotonically increasing: all changes that happened since you last synced have a larger timestamp than the sync event.
But users' clocks are not monotonically increasing: they drift, which is fine, but then they get reset manually or via NTP. They jump around based on broken cell towers. They get reset to 2001 when you change the battery.
If you last synced at time 234, and a user's clock is now 123, you'll miss all of their recent changes. And then you'll re-sync everything they did before, as if it just happened, or your client code will detect -- but be unable to do anything about -- the presence of records changed in the future.
So: let's only use timestamps for last-ditch conflict reconciliation, and switch to using monotonically incrementing version counts in our local databases, much as we now do on servers.
Thoughts, folks?
(At some point I'll file a separate bug for Fennec, and for our other local databases. This is just for Places.)
Comment 1•12 years ago
|
||
If I were designing Sync from scratch, I would use monotonically incrementing version numbers liberally.
+1 +1 +1
Comment 2•12 years ago
|
||
so you suggest adding sort of a meta table to all of the databases and after a fetch increase the counter?
and any insert or update of tracked entries should be associated to that counter?
Sounds like feasible, could increase the cost of queries to keep it updated, unless we read the counter on startup, that adds similar issues to those we had with session id.
But this makes more sense in a world where components are designed for Sync rather than Sync designed to wrap components (that we sadly have today).
Reporter | ||
Comment 3•12 years ago
|
||
I'm basically suggesting exactly what sqlite does for rowid: a sqlite_sequences entry that gets incremented. Note that it doesn't matter if version numbers are non sequential, only that they be monotonically increasing. Sequences support in sqlite would make this trivial, but we can do it easily enough with a stored procedure.
Comment 4•12 years ago
|
||
SQLite doesn't have stored procedures, fwiw.
Comment 5•8 years ago
|
||
I think recent work by Kit superseded this request.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Comment 6•8 years ago
|
||
I don't think so; Sync still uses last-modified times for conflict resolution. AIUI, Richard is requesting a column that's incremented once per change, and never decremented. The change counter doesn't guarantee either. If a bookmark is changed on two devices, we can use this "version" number to decide which one is newer, instead of the modified time. Is that accurate?
Flags: needinfo?(rnewman)
Comment 7•8 years ago
|
||
maybe this should be moved to Sync (and eventually depend on an implementation bug in Places?). As it is doesn't look like being part of any project or blocking anything.
Comment 8•8 years ago
|
||
SGTM! We'll discuss this at the next Sync triage.
Component: Places → Sync
Product: Toolkit → Firefox
Whiteboard: [designing for syncability]
Updated•8 years ago
|
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Reporter | ||
Comment 9•8 years ago
|
||
I think the recent desktop tracker work obsoleted any desktop part of this bug.
Android still uses timestamps to detect local changes, so it would very much benefit from this.
Flags: needinfo?(rnewman)
Comment 10•7 years ago
|
||
Grisha, I think your tracker rewrite in bug 1364644 fixed this on Android, right?
Flags: needinfo?(gkruglov)
Comment 11•7 years ago
|
||
That work was targeted specifically at bookmarks, introducing two version counters (localVersion & syncedVersion), as well as storage and sync machinery to support these.
So, yes, partially - that bug fixes the Android bookmarks parts of this bug, and it lays down groundwork for other data types to follow suit.
Flags: needinfo?(gkruglov)
Updated•7 years ago
|
Component: Sync → Android Sync
Product: Firefox → Firefox for Android
Comment 12•4 years ago
|
||
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 4 years ago
Resolution: --- → INCOMPLETE
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
•