fix(sync): stop a phantom conflict deleting a record everywhere — 2.6.53 - #163
Merged
Conversation
The first review of the previous release found that its two changes, each right on its own, combined into silent loss. Conflict rows replicate: a device that has never held a record can receive one. Retiring a stale row ran only when a tombstone or a live update was applied, never on the insert path a fresh or third device takes, and the new content rule kept a row alive while the local record matched either of its versions. So after another device revived a record by keeping the alternative, the newcomer still held a row whose pre-selected winner was the deletion. One tap and the record was gone from every device, with no conflict raised anywhere. Retiring now runs on every path a record arrives by, and a row survives only while the record still equals its winner: a record that matches the alternative was switched to it by a resolution somewhere, so the winner is a version it has left behind. An unsettled alternative comes back once, against the current version, rather than as a deletion. The preferences base was written inside the apply transaction, before the commit that stores the settings had succeeded. A refused or thrown commit left the base naming a version that was never applied, and the next build then published the local settings with no conflict, silently discarding the other device's change. It is written after the commit returns true. Taking the remote whenever the local version equals the base ignored direction, so a missing head bundle made a device holding the newer version revert, and every unedited device followed. An older remote now falls through to last-writer-wins, which keeps the newer copy and republishes it. Verified on a Pixel 7a as three peers against one real Drive: B edits a task, C deletes it, B stores the conflict, a fresh peer A receives that row while holding no task at all, B keeps the alternative and republishes — and A's next sync retires the phantom and ends up with the task alive. Both peers stay clean afterwards. 343 unit tests, 92 instrumentation tests, 0 failures; lint 0 errors.
Test coverage (unit + instrumentation)
Files
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The first review of 2.6.52 found that its two changes, each correct on its own, combined into silent data loss.
Conflict rows replicate, so a device that has never held a record can receive one. Retiring a stale row ran only when a tombstone or a live update was applied — never on the insert path a fresh or third device takes — while the new content rule kept a row alive as long as the local record matched either of its versions. After another device revived a record by keeping the alternative, the newcomer was still holding a row whose pre-selected winner was the deletion. One tap and the record was gone from every device, with no conflict raised anywhere.
What changed
Retiring now runs on every path a record can arrive by, and a row survives only while the record still equals its winner. A record that matches the alternative was switched to it by a resolution somewhere, so the winner is a version it has left behind. An unsettled alternative comes back once, against the current version, instead of as a deletion.
The preferences base was written inside the apply transaction, before the commit that stores the settings had succeeded. A refused or thrown commit left the base naming a version that was never applied, and the next build then published the local settings with no conflict, silently discarding the other device's change. It is now written after that commit returns true.
Taking the remote whenever the local version equals the base ignored direction, so a missing head bundle made a device holding the newer version revert — and every unedited device followed. An older remote now falls through to last-writer-wins, which keeps the newer copy and republishes it, repairing Drive.
Verification
343 unit tests, 92 instrumentation tests, 0 failures; lint 0 errors; R8 clean.
The blocker is pinned by a device test with three real stores (
aRecordRevivedElsewhereLeavesNoPhantomDeletionOnAPeerThatNeverHeldIt) and reproduced by hand on a Pixel 7a as three peers against one real Drive: B edits a task, C deletes it, B syncs and stores the conflict, a fresh peer A signs in and receives that row while holding no task at all, B keeps the alternative and republishes — and A's next sync retires the phantom and ends up with the task alive. Both peers stay clean afterwards, no exception in any run.Also verified on the device this round, unchanged by these fixes: tasks (A deletes, B edits, one honest conflict, converges in one resolution, the task returns to A with B's edit) and tags (A renames, B renames differently, one conflict, both peers converge on the kept name).
Known limitation
A record edited or deleted before its first sync after the 21 → 22 migration still raises one honest conflict through the no-base fallback. It settles in one resolution and cannot recur.