Syncing Bookmark Tombsones
tchiovoloni at mozilla.com
Thu Jan 25 22:45:21 UTC 2018
Before break (before Austin, really), I put a patch up on bug 1343103
<https://bugzil.la/1343103> that I'm only so-so about. It got f+, but I'm
going to explain my concerns before finishing it.
Currently we store tombstones locally until the next time we sync. Once we
sync and upload the tombstones we have locally, we delete them from places.
When we receive a remote tombstone to the server, if we don't have that
item we do nothing, and if we do, we delete it (without creating a
tombstone for it). The server will store tombstones roughly indefinitely,
but not durably.
1. Treat tombstones more-or-less like we treat bookmarks now, that is,
keep them around locally, store remote ones locally when they come in, and
resolve conflicts based on the modified timestamp on the item.
This approach fails in several cases. For example, if there is a node
reassignment, if the first client to sync didn't see all tombstones on the
server before the node reassignment, we will resurrect the deleted item.
2. Store tombstones from the server locally in places (as in #1), but
always reconcile in favor of deleted items over new items. Once an item is
deleted it's gone. (There are some nuances, e.g. If an incoming item has a
local tombstone, we reupload that tombstone to the server (to delete it),
This is the approach I took with my patch.
This has issues around bookmark restoring, which aren't handled by my
patch (probably it's biggest TODO). We probably need to store tombstone
guids with the bookmark restore data. We also probably need to fix bugs
around missed bookmark restores that happen before sync is initialized. I
don't know for sure if this is enough or not.
It also seems dangerous to never allow items to be undeleted -- once a
guid is discarded, it's effectively gone forever (except for cases where
all clients with the guid wipeClient).
3. The "most correct" approach is probably to store the (best guess for)
the server timestamp when the bookmark was deleted in the places database,
which would then be used for reconciliation (Possibly using a scheme
similar to dateAdded for ratcheting time backwards).
We'd also need to add the 'real' modification date to each bookmark
record, e.g. the server timestamp upon editing the record. This would allow
us to continue working as we do now (sometimes undeleting bookmarks).
This is much more complexity, and is still imperfect (it fails if
there's a long enough time between when the bookmarks are modified and we
sync, since we won't be able to figure out the server timestamps).
4. I guess we could also do nothing. It's an edge case, there doesn't
seem to be a perfect solution (after writing this up I still think #2 is
probably the best). I also suspect that most people who have seen
resurrected items, it was really bug 1177516 <https://bugzil.la/1177516>,
which this won't fix at all.
I don't think #4 is the right choice, but I'm including it since Mark
mentioned it in our one-on-one. I think we should still do this, since the
code complexity isn't *that* bad (well, for #3 it's a good amount worse),
and it does solve a known issue -- we also knew it wouldn't be trivial when
we decided to do it initially, and I don't think much has changed around
this since then.
(Admittedly, I'm biased though, I'd rather not have another OKR I'm
assigned to where we end up doing nothing; as with the sync-repair system
Anyway, that's the writeup on this that I said I would do. Thoughts?
- Thom Chiovoloni
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Sync-dev