Closed Bug 851380 Opened 11 years ago Closed 3 years ago

Switch to an incrementing version count for changes, rather than timestamps

Categories

(Firefox for Android Graveyard :: Android Sync, defect)

defect
Not set
normal

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.)
If I were designing Sync from scratch, I would use monotonically incrementing version numbers liberally.

+1 +1 +1
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).
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.
SQLite doesn't have stored procedures, fwiw.
I think recent work by Kit superseded this request.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
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)
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.
SGTM! We'll discuss this at the next Sync triage.
Component: Places → Sync
Product: Toolkit → Firefox
Whiteboard: [designing for syncability]
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
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)
Grisha, I think your tracker rewrite in bug 1364644 fixed this on Android, right?
Flags: needinfo?(gkruglov)
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)
Component: Sync → Android Sync
Product: Firefox → Firefox for Android
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: 7 years ago3 years ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.