fix(db): reclaim unused sync without aborting preloads - #1810
Merged
Conversation
`startGCTimer` had one caller: `removeSubscriber`, on the edge where the last subscriber leaves. A collection whose subscriber count went from zero straight back to zero never crossed that edge and so never armed the timer. Sync starts without a subscriber in three places -- `startSync: true`, `preload()` and `startSyncImmediate()`. A live query started that way keeps a subscription on every collection it reads from, so it both survives and reprocesses every source change forever, however short its `gcTime` is. Framework adapters build their live query collection while rendering and subscribe when that render commits, which makes every render React discards before committing -- a suspended subtree, a render that throws, a time-sliced render restarted by an interleaved update -- leak one compiled query graph rooted at a long-lived source collection. `startSync` is the single point every route into sync passes through, so it now arms the timer whenever it runs unsubscribed. The delay is floored at 50ms so a subscriber arriving with the commit cancels it first; the floor does not apply to the last-subscriber-leaves timer, which still fires on `gcTime`. `gcTime: 0` still disables GC. `CleanupQueue.cancel` also retires the shared root timer once it empties the queue, instead of leaving it armed with nothing to run.
The comments introduced with the unsubscribed-sync fix explained the defect three times over, at a length the surrounding methods do not use. What is left is the part that is invisible from the code: why the floor exists and cannot be `gcTime`, why a non-empty cleanup queue keeps a timer that may wake early, and that `addSubscriber` counts itself in before starting sync. `startGCTimerIfUnsubscribed` now reads like its siblings -- what it does, then when it runs. Its history belongs in the log, not above the method.
`meta-framework` said an unpreloaded collection starts syncing when the component mounts. It starts on the component's first render -- `useLiveQuery` constructs with `startSync: true` from the render body -- which is why a render that never commits can still start sync. Mount is only where teardown is keyed. `live-queries` documented that `gcTime: 0` disables collection for a derived collection. It now also opts out of reclaiming a collection that synced without ever gaining a subscriber, so say so, along with the fact that `gcTime: 0` means the opposite in TanStack Query -- prompt collection there, none here.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe change updates collection garbage collection for unsubscribed sync, pending and warm preloads, cleanup timers, detached observer revisions, and React rendering. It adds regression tests and documents the updated behavior. ChangesCollection garbage-collection lifecycle
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant CollectionSyncManager
participant CollectionLifecycleManager
participant CleanupQueue
participant DetachedObserver
CollectionSyncManager->>CollectionLifecycleManager: Start sync or preload
CollectionLifecycleManager->>CleanupQueue: Schedule or cancel GC
CleanupQueue->>CollectionLifecycleManager: Recheck GC eligibility
CollectionLifecycleManager->>CollectionSyncManager: Clean up eligible collection
CollectionSyncManager->>DetachedObserver: Publish updated state revision
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The cleanup revision change and detached-observer reload coverage do not leave an identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 15 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
More templates
@tanstack/angular-db
@tanstack/browser-db-sqlite-persistence
@tanstack/capacitor-db-sqlite-persistence
@tanstack/cloudflare-durable-objects-db-sqlite-persistence
@tanstack/db
@tanstack/db-ivm
@tanstack/db-sqlite-persistence-core
@tanstack/electric-db-collection
@tanstack/electron-db-sqlite-persistence
@tanstack/expo-db-sqlite-persistence
@tanstack/node-db-sqlite-persistence
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/react-native-db-sqlite-persistence
@tanstack/react-router-with-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/tauri-db-sqlite-persistence
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Contributor
|
Size Change: +250 B (+0.15%) Total Size: 164 kB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 7.34 kB ℹ️ View Unchanged
|
tannerlinsley
approved these changes
Sep 11, 2026
tannerlinsley
left a comment
tannerlinsley
left a comment
Member
There was a problem hiding this comment.
Reviewed unused sync collection, pending preload retention, timer cleanup, and the abandoned render coverage. The lifecycle guards preserve pending preloads while allowing unused sync to be collected. The selected core suites passed locally, 176 tests, along with all 4 uncommitted render tests. CI is green. Looks good to me.
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
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.
Collections that start syncing before anything subscribes now release their source subscriptions after the GC delay. This fixes query graphs retained by discarded React renders while keeping pending
preload()calls alive until they settle.Builds on @edzis's work in #1744 and the reproduction in #1178, preserving the original commits.
Root cause and approach
GC previously started only when the last subscriber left. A live query constructed during rendering starts sync immediately, but a suspended or throwing render may never commit and subscribe. With no subscriber departure, that query retained its source subscriptions indefinitely.
Arm GC when sync starts without an owner. A subscriber or pending preload owns the collection; automatic cleanup requires neither to exist. Starting a preload cancels both queued GC and pending idle cleanup, and settlement starts a fresh unused retention period. Preloading already-ready data also renews that period, including when returning a cached preload promise. Explicit
cleanup()can still abort a preload.The initial delay is
max(gcTime, 50ms). This grace period reduces cleanup between render and commit; it does not guarantee React will commit within 50ms. A later subscriber restarts a reclaimed query. Once the last subscriber leaves, the configuredgcTimeapplies without the floor.gcTime: 0continues to disable automatic GC.The shared cleanup queue clears its timer when its last task is canceled and calls
unref()on Node timer handles so background GC does not keep a finished process alive. Browser timer handles remain supported.Cleanup also advances the observable state revision. A detached observer that misses the entire cleanup/restart cycle therefore discards its old snapshot even when the reload is empty and returns to the same ready status.
This keeps ownership and reclamation in the collection lifecycle rather than adding framework-specific disposal. It does not change adapter defaults or promise to retain a query for an arbitrarily delayed commit.
Tests and verification
Lifecycle tests use fake-clock boundaries, source subscription counts, and projection-call assertions. The changed fixtures release their collections. Electric refresh tests verify cancellation of the specific refresh timer instead of counting unrelated shared timers.
Coverage includes discarded renders and late subscribers, orphan reclamation and restart, disabled GC, slow/shared preloads, repeated warm preloads and cached-promise reuse, preload failure and explicit cleanup, pending idle cleanup, Node process exit, and detached wholesale/granular snapshots across an unobserved cleanup and empty reload. Red/green controls reproduced the pending/warm preload, detached-observer, and Node regressions, disabled orphan GC, omitted final-task timer retirement, omitted Electric refresh timeout cancellation, and enabled erroneous child-facade GC while its root remained subscribed; the corresponding regressions failed and passed after restoration.
no-shadowwarnings remain.Review map
collection/sync.tsandcollection/lifecycle.ts: initial GC scheduling and preload ownership through all cleanup stages.collection/changes.ts: cleanup revision invalidation for detached snapshots.collection/cleanup-queue.ts: last-task cancellation and Node process lifetime.gcTimedocs, skills, and changeset: retention behavior and unchanged GC opt-out.Fixes #1178
Summary by CodeRabbit
Bug Fixes
gcTimevalues.Documentation
Tests