Fix: Resolve infinite sync loops causing gigantic unnecessary restorations#1640
Fix: Resolve infinite sync loops causing gigantic unnecessary restorations#1640Lolle2000la wants to merge 4 commits into
Conversation
The synchronization process was previously suffering from runaway loops where devices would continuously restore hundreds of unchanged manga and chapters. This commit implements the fixes needed to prevent these loops: - In MangaRestorer, ensure that chapter versions are properly updated locally when the chapter's read/bookmark contents are identical to the remote, preventing the loop where a device fails to update its local version and continuously resyncs the chapter. - Update SyncManager and SyncService to use stable identifiers (URLs and source IDs) instead of volatile metadata (titles, authors) for composite keys to avoid mistakenly recreating matching items. - In MangaRestorer, properly copy internal og* fields rather than frontend override fields during restoration, and properly restore chapterFlags, viewerFlags, and updateStrategy.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Lolle2000la, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical synchronization bug where devices would enter an infinite 'ping-pong' loop, triggering massive, redundant restoration processes. By refining how the system identifies mangas and chapters and ensuring that version and sync-state metadata are correctly persisted during restoration, the changes stabilize the synchronization flow and eliminate the runaway operations that previously caused significant performance degradation. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. The sync loop spun in endless night, / With titles shifting out of sight. / Now keys are fixed and versions stay, / To keep the ping-pong ghosts at bay. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the manga restoration process to include additional metadata fields such as ogTitle and chapterFlags, and refines chapter comparison logic to handle version mismatches during synchronization. Additionally, it simplifies the composite keys used for manga and chapter identification by reducing them to source and URL-based identifiers. A review comment noted an unused import for BackupCategory in SyncManager.kt.
Without this, very small unnecessary restorations are still left. It is also expected behavior
This means that when for example `savedSearchesFeeds` is disabled on one device but not the other, then the device where setting is disabled will restore the saved searches from the device where it isn't. In fact, just syncing once will poising the entire remote store, with it becoming impossible to disable anymore
|
is this related to your same DB restoration speed PR #1160 or different? |
|
@cuong-tran The upstream PR to TachiyomiSY I made just got merged, but please consider not cherry picking it and instead merging this, since there were some komikku specific changes necessary. |
2 participants
Currently, the synchronization functionality suffers from a severe "ping-pong" loop where devices continuously trigger full restorations of hundreds or thousands of mangas unnecessarily. This causes massive, long-running sync operations even when no actual changes have been made by the user.
The above screenshot shows why I created this PR, with a synchronization of literally nothing taking 21 minutes and 40 seconds.
This runaway restoration loop is caused by a few specific oversights in how the sync system compares and updates local vs. remote states:
The problems
MangaRestorer.kt): When a remote chapter is downloaded that has an identical read/bookmark state to the local database but a higherversion, the restorer optimized the process by completely skipping the chapter. Because it was skipped, the local chapter'sversionwas never updated to match the remote. On the very next sync,SyncManager.areChaptersDifferentdetected the mismatched versions and falsely flagged the entire manga as modified, triggering another massive restoration.MangaRestorer.kt): TheManga.copyFrommethod was missing several critical fields, most notablyogTitleand the sync state flags (chapterFlags,viewerFlags,updateStrategy). When a remote manga was restored, its title was never saved to the local database.SyncManager.kt&SyncService.kt): The sync logic used display metadata liketitle,author, and chapternamein its lookup maps. Because the local database'sogTitlewas never updating (due to the bug above),SyncManagerwould look for the old title in memory, fail to find the remote's new title, assume it was a brand-new manga, and trigger a full restoration. The database would correctly deduplicate it upon insertion, but the loop would repeat indefinitely.Bug-Flow
sequenceDiagram participant A as Device A participant S as Sync Server participant B as Device B Note over A, B: Both devices initially up-to-date A->>A: Sets 1 chapter to read A->>S: Synchronizes changes B->>S: Starts synchronization S-->>B: Downloads backup Note over B: Bug triggered: Mismatched keys and skipped versions B->>B: Restores hundreds/thousands of unaffected mangas B->>S: Uploads "merged" state with mismatched versions Note over A: User makes NO changes A->>S: Synchronizes again S-->>A: Downloads backup from B Note over A: Bug triggered: Mismatched keys and skipped versions A->>A: Restores hundreds/thousands of unaffected mangas A->>S: Uploads "merged" state again Note over A, B: The Ping-Pong Loop continues indefinitely...The Solution
This PR implements the minimal structural fixes required to permanently terminate these sync loops:
MangaRestorer.restoreChaptersto explicitly adopt the remote chapter'sversion(via a lightweight.copy(id = dbChapter.id)) even if the content state is identical, permanently breaking the version mismatch loop.mangaCompositeKeyinSyncServiceand thelocalMangaMapinSyncManagerto strictly usePair(source, url). Chapter keys were reverted tourl. This guarantees mangas and chapters are matched purely by their immutable backend IDs, preventing volatile display names from tricking the app into restoring them.Manga.copyFrominMangaRestorerto correctly map toogTitle,ogAuthor, etc. instead of the frontend override fields, and included the missingchapterFlags,viewerFlags, andupdateStrategymappings so the database properly adopts the remote sync state.This is a cherry-pick of my PR to TachiyomiSY. I resolved merge conflicts that come from Komikku specific features and changes.