-
Notifications
You must be signed in to change notification settings - Fork 21
[MongoDB Storage] Compact parameter lookups #315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: d2bc507 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 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 |
66f815b
to
fc32d7c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the MongoDB storage implementation to include parameter lookup compaction alongside existing bucket data compaction. The compact action now also removes superseded parameter storage entries, addressing storage growth and query performance issues in use cases where parameter lookups are modified frequently.
Key changes:
- Added parameter data compaction with snapshot consistency guarantees
- Refactored storage APIs to use snapshot-based parameter queries instead of checkpoint-based queries
- Implemented comprehensive test coverage for parameter compaction scenarios
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
packages/service-core/src/storage/SyncRulesBucketStorage.ts |
Adds compactParameterData option and moves getParameterSets to ReplicationCheckpoint interface |
packages/service-core/src/sync/BucketChecksumState.ts |
Updates parameter lookup calls to use checkpoint's getParameterSets method |
packages/service-core/src/entry/commands/compact-action.ts |
Modifies compact command to enable parameter compaction when no specific buckets are targeted |
packages/service-core/test/src/sync/BucketChecksumState.test.ts |
Updates test mocks to use new checkpoint-based parameter lookup API |
packages/service-core-tests/src/tests/register-parameter-compacting-tests.ts |
Adds comprehensive tests for parameter compaction functionality |
modules/module-postgres-storage/src/storage/PostgresSyncRulesStorage.ts |
Implements new checkpoint-based parameter lookup interface for Postgres |
modules/module-mongodb-storage/src/storage/implementation/MongoSyncBucketStorage.ts |
Implements snapshot-based parameter queries and integrates parameter compaction |
modules/module-mongodb-storage/src/storage/implementation/MongoParameterCompactor.ts |
New compactor implementation for parameter lookup data |
Comments suppressed due to low confidence (1)
packages/service-core-tests/src/tests/register-parameter-compacting-tests.ts:78
- The test does not verify that parameter compaction actually occurred by checking storage metrics before and after compaction. Consider adding assertions to validate that the parameter storage size decreased after compaction.
await bucketStorage.compact({ compactParameterData: true });
modules/module-mongodb-storage/src/storage/implementation/MongoParameterCompactor.ts
Outdated
Show resolved
Hide resolved
modules/module-mongodb-storage/src/storage/implementation/MongoParameterCompactor.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very cool use-case for MongoDB snapshot queries. The changes here look good to me, I could not spot any potential issues :D
The compact action now also compacts bucket parameter storage.
I many use cases, parameter lookups are not modified often, and take up fairly little storage. However, there are some cases where it is modified often, and this causes two issues:
This compacts the parameter storage when using MongoDB storage.
See the new
docs/parameters-lookups.md
for details on the implementation.One notable change is that we now do "snapshot" queries for parameter data, matching the snapshot state of the checkpoint query. This is required for consistency of parameter lookups during a compact operation. This required some refactoring to the storage APIs.
TODO: