Optimize incremental sync: Phase 1 #197
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
🦋 Changeset detectedLatest commit: bc5326c The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Remove queryBucketDescriptions(). Refactor logic to keep track of checksums per connection. Refactor BucketParameterQuerier implementation. Refactor watchWriteCheckpoint. Refactor bucket position tracking. For static updates, do diff of individual buckets if available. Start by always sending an invalidation event. Remove the need for fullDocument: 'updateLookup' for watching checkpoints. Move watchWriteCheckpoint to SyncRulesBucketStorage. We now always watch checkpoints per sync rules instance. Fix tests to use the new apis. Type fixes. Fix tests. Test storage on github actions.
rkistner
force-pushed
the
optimize-bucket-lookups
branch
from
5e745e3 to
2b024ff
Compare
rkistner
force-pushed
the
optimize-bucket-lookups
branch
from
6c02b7b to
5df24a0
Compare
Add more units tests.
rkistner
changed the title
rkistner
commented
Feb 17, 2025
We just let the SyncRulesBucketStorage instances get garbage collected - no need for active disposing of listeners. The lifecycle of SyncRulesBucketStorage is not well-defined, so active disposing could lead to more confusion down the line. BucketStorageFactory and BucketStorageBatch are still AsyncDisposable.
…imize-bucket-lookups
stevensJourney
previously approved these changes
Feb 18, 2025
simolus3
previously approved these changes
Feb 18, 2025
rkistner
dismissed stale reviews from simolus3 and stevensJourney
The base branch was changed.
An error occurred while trying to automatically change base from
feat/bucket-priorities
to
main
February 18, 2025 12:41
stevensJourney
approved these changes
Feb 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This refactors large parts of the sync flow to allow better optimizations.
Refactoring:
watchWriteCheckpointis moved fromBucketStorageFactorytoSyncRulesBucketStorage, making it operate on a single sync-rules instance/version only. This simplifies the implementation and will make it much easier to do optimizations here, since we don't have to deal with sync rule changes.sync.ts, the bucket parameter state management is moved to a separate class, keeping the main loop simpler.The biggest actual optimization in this phase is removing
fullDocument: 'uploadLookup'from the changestream for watching checkpoints. Since this changeStream could be updated very frequently, this can add significant overhead on the bucket storage database. We now instead merge the incremental updates ourselves.Builds on some API refactoring in #192.
I added a new test-client command to test concurrent connections:
This just opens connections and keep them open to simulate sync load. No direct reporting of performance is done yet, but you can view the timestamp differences between the first
checkpoint_diffand the lastcheckpoint_completeoperations to get an idea of performance.