Put ets cache in front of sqlite #3716
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes extend the shape caching system by replacing the in-memory ETS-backed last-used table with an expanded metadata table storing handle, hash, snapshot state, and read time tuples. Database operations are annotated with action labels for improved telemetry tracking, and the query layer is updated to expose richer shape metadata. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3716 +/- ##
=======================================
Coverage 87.36% 87.36%
=======================================
Files 23 23
Lines 2011 2011
Branches 532 532
=======================================
Hits 1757 1757
Misses 252 252
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/sqlite-ets-cache
branch
from
1cdec3c to
2734d7b
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
benchmark this |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@packages/sync-service/lib/electric/shape_cache/shape_status/shape_db/connection.ex`:
- Around line 106-125: The fallback clause handle_checkout({:checkout, _label},
_from, conn, pool_state) is unreachable because handle_enqueue({:checkout,
label}, pool_state) always enqueues {:checkout, label, now()}, so remove the
dead clause (the two-arity pattern) from connection.ex (or instead replace it
with a clear comment if you intentionally want defensive behavior); update tests
accordingly to ensure no behavior changes.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
packages/sync-service/lib/electric/shape_cache/shape_status.expackages/sync-service/lib/electric/shape_cache/shape_status/shape_db.expackages/sync-service/lib/electric/shape_cache/shape_status/shape_db/connection.expackages/sync-service/lib/electric/shape_cache/shape_status/shape_db/query.expackages/sync-service/test/electric/shape_cache/shape_status/shape_db_test.exs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: electric-sql/electric PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-14T14:45:05.838Z
Learning: Define shapes in server/proxy – no client-defined tables/WHERE clauses
📚 Learning: 2026-01-14T14:45:05.838Z
Learnt from: CR
Repo: electric-sql/electric PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-14T14:45:05.838Z
Learning: Avoid old Electric patterns (bidirectional SQLite sync, `electrify()` API) – use Electric HTTP streaming with TanStack DB collections instead
Applied to files:
packages/sync-service/lib/electric/shape_cache/shape_status/shape_db/connection.ex
🧬 Code graph analysis (2)
packages/sync-service/lib/electric/shape_cache/shape_status/shape_db/connection.ex (3)
packages/sync-service/lib/electric/connection/manager.ex (1)
pool_name(180-182)packages/sync-service/lib/electric/shape_cache/shape_status/shape_db.ex (1)
explain(182-186)packages/sync-service/lib/electric/shape_cache/shape_status/shape_db/query.ex (2)
explain(263-265)explain(267-269)
packages/sync-service/lib/electric/shape_cache/shape_status/shape_db.ex (1)
packages/sync-service/lib/electric/shape_cache/shape_status/shape_db/connection.ex (2)
checkout_write!(168-180)checkout!(159-166)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Check and build examples/linearlite-read-only
- GitHub Check: Check and build examples/tanstack-db-web-starter
- GitHub Check: Check and build examples/linearlite
- GitHub Check: Check and build examples/nextjs
- GitHub Check: Check and build examples/react
- GitHub Check: Check and build examples/encryption
- GitHub Check: Check and build examples/proxy-auth
- GitHub Check: Run Lux integration tests
🔇 Additional comments (18)
packages/sync-service/lib/electric/shape_cache/shape_status/shape_db/query.ex (2)
6-8: LGTM!The new
list_shape_metaquery correctly selects the required metadata fields (handle, hash, snapshot_state) for the ETS cache population.
249-253: LGTM!The stream function correctly transforms the database's integer
snapshot_stateto a boolean representing "snapshot started" status, consistent with thesnapshot_started?function logic at Line 199.packages/sync-service/lib/electric/shape_cache/shape_status/shape_db.ex (3)
12-15: LGTM!The updated imports correctly reflect the new function signatures that accept action labels for telemetry tracking.
35-47: LGTM!Action labels are consistently applied across all checkout operations, enabling meaningful telemetry tracking per operation type.
100-106: LGTM!The renamed
reduce_shape_meta/3correctly uses the newlist_shape_meta_streamand the name accurately reflects the richer metadata being processed.packages/sync-service/test/electric/shape_cache/shape_status/shape_db_test.exs (1)
176-199: LGTM!The test comprehensively validates the renamed
reduce_shape_meta/3function, correctly verifying the {handle, hash, snapshot_started} tuple structure with proper setup of both started and non-started snapshots.packages/sync-service/lib/electric/shape_cache/shape_status/shape_db/connection.ex (3)
132-140: LGTM!The reduced SQLite page cache (512KB) is appropriate since hot-path lookups now go through the ETS cache. The comment clearly explains the negative value convention.
114-118: LGTM!The telemetry emission provides valuable observability for pool checkout latency, enabling monitoring and debugging of potential connection pool bottlenecks per operation type.
159-180: LGTM!The updated function signatures correctly propagate the label through to NimblePool, enabling the telemetry instrumentation in
handle_checkout.packages/sync-service/lib/electric/shape_cache/shape_status.ex (9)
32-34: LGTM!The tuple structure
{handle, hash, snapshot_started, last_read_time}and position constant are clearly documented, making the code self-explanatory.
69-75: LGTM!The ETS insertion correctly uses
insert_newto prevent race conditions and properly sequences the persistent store write before the cache write.
169-175: LGTM!The hash validation via ETS lookup is an effective optimization, avoiding SQLite queries for shape validation on the hot path.
182-186: LGTM!The update correctly sequences the persistent write before the cache update and properly uses
update_elementwith the correct tuple position.
190-193: LGTM!Reading
snapshot_started?from ETS cache provides fast hot-path access while maintaining a safe default offalsefor missing entries.
203-205: Verify:snapshot_complete?not cached in ETS.While
snapshot_started?now reads from the ETS cache (Line 190-193),snapshot_complete?still queries SQLite directly. This asymmetry may be intentional ifsnapshot_complete?is called less frequently, but it creates inconsistent performance characteristics.If
snapshot_complete?is also on a hot path, consider extending the meta tuple to{handle, hash, snapshot_started, snapshot_complete, last_read_time}and caching both states.
231-267: LGTM!The LRU algorithm using
gb_treesis an efficient approach (O(n) scan with O(log k) tree operations) and correctly handles the new 4-tuple structure.
297-310: LGTM!The population logic correctly transforms the 3-tuple from
ShapeDb.reduce_shape_metainto the 4-tuple required by ETS, initializing all shapes with the samestart_timefor a fair LRU baseline after restart.
283-295: LGTM!The ETS table configuration with
read_concurrency: trueandwrite_concurrency: :autois well-suited for a cache that's primarily read-heavy with occasional writes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
packages/sync-service/lib/electric/shape_cache/shape_status/shape_db/connection.ex
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
|
Benchmark results, triggered for 2734d
|
magnetised
force-pushed
the
magnetised/sqlite-ets-cache
branch
from
2734d7b to
bb41c9a
Compare
Reduce load on sqlite by moving the core metadata checked by most read requests into ETS.
SQLite is now only responsible for the shape -> handle lookups for requests without a handle within the read path.
The metadata is only integers and atoms we retain the massive memory savings gained by moving the shapes themselves out of ets.