feat(sync-service): SQLite-backed metadata storage #3682
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
✅ Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3682 +/- ##
=======================================
Coverage 87.93% 87.93%
=======================================
Files 23 23
Lines 1898 1898
Branches 499 500 +1
=======================================
Hits 1669 1669
Misses 227 227
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
magnetised
force-pushed
the
magnetised/3514-sqlite
branch
from
7226bf9 to
891453c
Compare
Contributor
Author
|
benchmark this |
This comment has been minimized.
This comment has been minimized.
Contributor
|
Benchmark results, triggered for 89145
|
magnetised
force-pushed
the
magnetised/3514-sqlite
branch
from
891453c to
3653942
Compare
This comment has been minimized.
This comment has been minimized.
magnetised
force-pushed
the
magnetised/3514-sqlite
branch
2 times, most recently
from
7ecf14d to
3fa21d8
Compare
This comment has been minimized.
This comment has been minimized.
Replace enormous ets tables holding shape lookup information with a single SQLite database. This massively reduces memory usage and removes the need to iterate the storage backend for shape information in the case of an unclean shutdown.
magnetised
force-pushed
the
magnetised/3514-sqlite
branch
from
3fa21d8 to
6566a32
Compare
robacourt
reviewed
Jan 8, 2026
| SELECT handle FROM shapes WHERE comparable = ?1 LIMIT 1 | ||
| """, | ||
| shape_lookup: """ | ||
| SELECT shape FROM shapes WHERE handle = ?1 LIMIT 1 |
Contributor
There was a problem hiding this comment.
Should we not make handle unique? Otherwise this returns an arbitrary shape. It would also mean you could get rid of the LIMIT 1. Currently we're only unique on the composite (comparable, handle)
robacourt
reviewed
Jan 8, 2026
| SELECT 1 FROM shapes WHERE handle = ?1 | ||
| """, | ||
| handle_lookup: """ | ||
| SELECT handle FROM shapes WHERE comparable = ?1 LIMIT 1 |
Contributor
There was a problem hiding this comment.
Same for comparable. This should be unique too
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Improves memory usage and actually greatly simplifies the ShapeStatus code as it removes the need for a "restore" path at all, let alone 2 distinct versions based on clean vs unclean shutdown.
The only metadata left in RAM is the LRU information, which is just
{handle, timestamp}.The access is via two NimblePool instances, one for reads which has a "worker" per core and a single write worker which guarantees serialized writes.
Performance locally seems reasonable, though it will be slower since - even if the data is in sqlite's page cache - we're now loading shape data from disk and deserializing using
:erlang.binary_to_term/1.My (hopefully) only controversial move is to add snapshot status information to the shape data so that we can initialize the system and remove any shapes that were left in a half-baked state without querying the storage backend.
I think this is cleaner that the current system, which AFAICT has issues if the system is terminated before a snapshot is started (since the snapshotting requires an active consumer and consumers are now lazily started, meaning that
await_snapshot_start/2will crash or hang).The implications of this are:
I've let claude review and addressed its comments, it's conclusion: