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. Options: 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), etc.) 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 addon) Anyway, that's the writeup on this that I said I would do. Thoughts? - Thom Chiovoloni
_______________________________________________ Sync-dev mailing list Sync-dev@mozilla.org https://mail.mozilla.org/listinfo/sync-dev
