signaldb icon indicating copy to clipboard operation
signaldb copied to clipboard

Draft: Sync pause and resume

Open obedm503 opened this issue 1 year ago • 3 comments

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
  • registerRemoteChange takes the collectionOptions just like push and pull
  • registerRemoteChange changes from executing once in the constructor to executing for each collection every time sync resumes
  • registerRemoteChange can return a clean-up function to allow for reconnection after a pause (think WebSockets)
  • renames syncAll to startSyncAll. startSyncAll fulfills the same purpose except it can be called multiple times
  • introduces startSync which calls registerRemoteChange and sync. This effectively replaces sync. sync is now private. Collections start in offline mode and only start syncing after startSync is called
  • introduces pauseSync to clean up remote-change subscriptions and put the collection into offline mode.
  • introduces pauseSyncAll to 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: initial-state

The application goes offline, web sockets are closed: offline

The user makes changes while offline but sync is paused: make-changes

The application goes back online, web sockets are reopened, and collections sync: reconnect


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

obedm503 avatar Sep 19 '24 01:09 obedm503

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.

codecov[bot] avatar Sep 19 '24 01:09 codecov[bot]

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?

maxnowack avatar Sep 19 '24 07:09 maxnowack

Saw the changes from #981 #982. I'll rebase this PR and integrate your suggestions.

obedm503 avatar Sep 28 '24 01:09 obedm503

any updates?

lorof avatar Jan 11 '25 09:01 lorof