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




Firefox for Android
Android Sync
5 years ago
4 months ago


(Reporter: rnewman, Unassigned)



Firefox Tracking Flags

(Not tracked)




5 years ago
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

5 years ago
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).

Comment 3

5 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.
SQLite doesn't have stored procedures, fwiw.

Comment 5

11 months ago
I think recent work by Kit superseded this request.
Last Resolved: 11 months 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)

Comment 7

11 months 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.
SGTM! We'll discuss this at the next Sync triage.
Component: Places → Sync
Product: Toolkit → Firefox
Whiteboard: [designing for syncability]
Resolution: WORKSFORME → ---

Comment 9

11 months 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)
Grisha, I think your tracker rewrite in bug 1364644 fixed this on Android, right?
Flags: needinfo?(gkruglov)

Comment 11

4 months 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)
Component: Sync → Android Sync
Product: Firefox → Firefox for Android
You need to log in before you can comment on or make changes to this bug.