Repository navigation
fix(sync): keep a superseded snapshot until every device has read past it - #1562
Merged
Merged
Conversation
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.
Summary
Garbage collection could delete a device's own snapshot manifest while other devices hadn't read that manifest's log entry yet. Readers then waited forever at the gap it left. Desktop hit this (the owner's phone is stuck at the Mac's seq 572 because the Mac deleted manifest 573), and Android has the same rule.
Facts.superseded()now also requiresacked(old.did) >= old.seq: every current device's latest ack hasvv[X] >= nfor the manifest's own seqn. This follows the spec fix in flow-sync Shorts Track selection #43 (spec/protocol.md, Garbage collection).Related issue
Part of #1075 (linked devices).
Change type
Validation
./gradlew :app:assembleGithubDebug: ok./gradlew :app:testGithubDebugUnitTest: 4650 passed, 46 skipped, 0 failed. New:GarbageCollectionTest.a_snapshot_stays_until_every_device_read_past_it_even_when_another_covers_it, the Mac case:./gradlew ktlintCheck: okpython3 .github/scripts/nightly_notes.py --check: validTest device and Android version: none
Risk and compatibility
Superseded manifests stay a little longer: until the slowest current device has read past them.
This does not repair a gap that already exists. A device stuck at a deleted manifest needs the Desktop-side handling or a fresh link.
Branched from
origin/mainbefore fix(sync): stop re-sending applied records and announce only Sync now #1559 merged. It touches none of fix(sync): stop re-sending applied records and announce only Sync now #1559's files.The change does not introduce secrets, private data, or unexpected telemetry.
New user-facing text uses Android string resources. (None.)
Dependency and lockfile changes are intentional and limited to this PR. (None.)
Room schema changes include the required version bump and migration, or this PR does not change the Room schema. (No schema change.)
Breaking changes and upgrade steps are clearly documented. (None.)