Repository navigation
fix(sync): stop re-sending applied records and announce only Sync now - #1559
Merged
Merged
Conversation
10 of 15 tasks
11 of 16 tasks
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
Two fixes from the owner's test, found in the server data and the phone's database.
ownSeq191).sent.sentAtwas bumped for all 13,166 watch_history keys at once.applyBatchesrecorded the received line's hash as known. Desktop's canonical form of a record differs from Android's, so the next export saw every applied record as changed and pushed it back. Desktop did the same, and each push sentchangedlive frames.applyBatchesnow takes the device's own export of the affected collections. After a successful apply, it records the hash of this device's own form of each received key, never the received line's hash.outgoingBatchesis unchanged.LinkedSyncWorker.syncSoontakesannounce, passed as work input. Only Sync now sets it, andLinkedSync.syncNow(announce)emits "Received N changes from …" only then.Related issue
Part of #1075 (linked devices). Stacked on #1558.
Change type
Validation
./gradlew :app:assembleGithubDebug: ok./gradlew :app:testGithubDebugUnitTest: 4648 passed, 46 skipped, 0 failed. New inChangeBatchesTest:devices_whose_canonical_forms_differ_exchange_once_and_then_stay_quiet: a phone and a desktop that order the fields of the same record differently exchange once. Two more rounds then push nothing from either side, and both hold the same data. With the old received-hash rule, the phone's next push would carry the desktop's record back.a_local_edit_not_uploaded_yet_still_goes_out_after_a_merge: a local edit that the merge keeps over an incoming record is still pushed.SessionTest,PairingTestand the existingChangeBatchesTestcase now pass the export../gradlew ktlintCheck: okpython3 .github/scripts/nightly_notes.py --check: validNot covered by tests: the snackbar gating to Sync now.
Test device and Android version: none
Risk and compatibility
Each sender's batches now cost two exports of the received collections, one before and one after the merge. For a large watch history this adds export time to a sync that received data. Syncs that receive nothing are unchanged.
Devices already caught in the loop stop once both run this fix. Their next sync records their own forms as known.
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.)