Draft: Sync pause and resume
Why Is This Useful
The SyncManager is an incredibly powerful addition to SignalDB but it is missing the ability to gracefully pause and resume sync.
I maintain a large application made up of many independent modules. Each module uses many collections: some shared, some unique. This application is long-lived. Users leave it running for hours or days at a time. Users have permission to access some or all the modules. All modules are not equally important to the user depending on their role. Users mostly use a few modules and rarely use the rest. Some modules are only used on an annual or quarterly basis.
This change is useful because it allows us to build applications where collections can become active or inactive throughout the lifetime of the session as the user navigates around the application. It would allow us to save both network and server resources by not syncing collections while they're not in use.
Summary of Changes
This is my implementation of sync pause and resume. This PR would introduce the following changes:
- when a collection is in offline mode it will continue recording changes but will not attempt to "push" them
registerRemoteChangetakes thecollectionOptionsjust likepushandpullregisterRemoteChangechanges from executing once in the constructor to executing for each collection every time sync resumesregisterRemoteChangecan return a clean-up function to allow for reconnection after a pause (think WebSockets)- renames
syncAlltostartSyncAll.startSyncAllfulfills the same purpose except it can be called multiple times - introduces
startSyncwhich callsregisterRemoteChangeandsync. This effectively replacessync.syncis now private. Collections start in offline mode and only start syncing afterstartSyncis called - introduces
pauseSyncto clean up remote-change subscriptions and put the collection into offline mode. - introduces
pauseSyncAllto pause sync for all collections
For an example of how this change would affect an application please see this diff: https://github.com/obedm503/trellix-offline/commit/90b030053d09ca3195bff3eac675f6c5a5e08c77
Practical Example
The initial state of the application:
The application goes offline, web sockets are closed:
The user makes changes while offline but sync is paused:
The application goes back online, web sockets are reopened, and collections sync:
This is very much a draft PR. I know you initially said you're not sure about changing how remote-change subscriptions happen, but I hope I made a good case for the change. I updated the tests so they all pass. I have not updated the documentation but will do so once this is a final PR. Also, I tried to maintain your code style, but please let me know if there are any style changes you wish to make. For the future, I recommend adopting something like Prettier to enforce your style in an automated way.
Related to #947
Codecov Report
Attention: Patch coverage is 63.82979% with 17 lines in your changes missing coverage. Please review.
Project coverage is 98.81%. Comparing base (
fa2cedc) to head (2f4b397). Report is 398 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| packages/signaldb/src/SyncManager/index.ts | 63.82% | 11 Missing and 6 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #951 +/- ##
===========================================
- Coverage 100.00% 98.81% -1.19%
===========================================
Files 54 54
Lines 1401 1430 +29
Branches 321 331 +10
===========================================
+ Hits 1401 1413 +12
- Misses 0 11 +11
- Partials 0 6 +6
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks for the PR @obedm503! 🙂 I'm quite busy right now, but I'll do a full review by the beginning of next week.
Some quick thoughts:
I'm unsure about the renaming of sync and syncAll. It seems like the new startSync and startSyncAll functions are calling registerRemoteChange every time they are executed and cleaning up the old one if it's present. I can think of a scenario where you only want to trigger a manual sync without tearing down and reinstantiating the whole connection.
My suggestion would be to keep sync and syncAll as they are, and just toggle a flag indicating whether a collection should be synced in startSync/pauseSync, in addition to the logic for registerRemoteChange. This would allow the user to pause/unpause the sync while keeping manual syncing possible. It would also avoid the breaking change of renaming the functions. What are your thoughts on this?
Saw the changes from #981 #982. I'll rebase this PR and integrate your suggestions.
any updates?