Repository navigation
fix(db): keep a collection from garbage collection while its transaction persists - #2258
Conversation
…ion persists Automatic GC checked subscribers and pending preloads but not transactions. A collection whose last subscriber left while a mutation handler was still running was cleaned up after gcTime: its optimistic rows disappeared and waits on the sync run, such as Electric's awaitTxId, rejected, so the write failed even when the server accepted it. A persisting transaction that touches the collection now holds it, and the gcTime delay starts again when the last one completes or fails. Explicit cleanup() is unchanged. A new generated-history oracle checks retirement against an independent deadline model across subscribe, unsubscribe, insert, settlement, clock steps and explicit cleanup.
…ections The GC oracle now runs every history over two Collections. An insert writes to one Collection through its onInsert handler, or to both through createTransaction with a manual commit, and the model keeps a deadline per Collection. A settlement that restarts the delay only for the first Collection a transaction touched now fails the oracle; the one-Collection grammar let it pass. The oracle also runs in the package's test:oracles campaign.
🦋 Changeset detectedLatest commit: ccbcbec The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Retain collections through transaction settlement packages/db/src/collection/cleanup-queue.ts, packages/db/src/collection/lifecycle.ts, packages/db/src/transactions.ts, packages/db/src/collection/state.ts, packages/db/src/types.ts, packages/db/mangle-cache.json, .changeset/gc-waits-for-persisting-transactions.md, docs/guides/mutations.md |
Automatic GC waits while a transaction persists. Settlement or failure starts GC scheduling with gcTime and any remaining startup grace. Cleanup-state handling skips replaying settled transaction mutations after cleanup starts. |
Model and test GC retention boundaries packages/db/tests/collection-gc-retention-oracle.ts, packages/db/tests/collection-gc-retention-oracle*.test.ts, packages/db/tests/collection-gc-transaction-oracle.property.test.ts, packages/db/tests/collection-gc-mutation-oracle.property.test.ts, packages/db/tests/oracle-config.ts, packages/db/tests/oracle-replay-manifest.ts, packages/db/package.json, docs/contributing/oracle-coverage.md, docs/contributing/oracle-reviews/pr-2258-gc-retention* |
Independent models and generated histories check retirement, transaction states, and visible rows. Oracle registration, coverage documentation, and review records describe the checks and their limits. |
Exercise native idle retention scenarios packages/db/e2e/collection-gc/*, packages/db/playwright.collection-gc.config.ts, packages/db/tsconfig.json, .github/workflows/e2e-tests.yml |
Chromium tests exercise idle, startup, and pending-transaction scenarios. The workflow runs the browser checks and collects their evidence. |
Verify retention with Electric and PostgreSQL packages/electric-db-collection/e2e/collection-gc.e2e.test.ts |
The end-to-end test checks that an Electric collection remains retained while PostgreSQL transactions await provider confirmation, then checks settlement and idle cleanup. |
Priority:
Estimated code review effort: 4 (Complex) | ~45 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- TanStack/db#2256 — The change prevents automatic cleanup while a touching transaction persists and starts GC scheduling after settlement.
Suggested reviewers: kyleamathews
Merge Risk: 
baa7d
-
This change keeps a Collection alive while a transaction touching it is persisting, and it adds model-based, browser, and Electric/PostgreSQL checks around that behavior. No merge-blocking issue was identified in the supplied review context.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 16 files. (4 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title names the main behavior change: a collection is kept from garbage collection while a transaction it touches is persisting. It matches the raw summary, which describes a persisting transactio… Linked Issues check Issue #2256requires that a Collection stay available while a transaction touching it persists. Its optimistic rows andawaitTxId/awaitMatchwaits must survive until settlement. GC timing may star…Out of Scope Changes check Production changes are limited to the GC retention and cleanup path for Issue #2256:
-lifecycle.ts: persisting-hold gate, startup grace, renewed deadline.
-transactions.ts: hold release on settl…Description check The pull request description provides complete sections for changes, oracle coverage, validation, checklist, and release impact. It documents the motivation, behavior, evidence, test results, known li… Full details: Title check
-
Explanation
The title names the main behavior change: a collection is kept from garbage collection while a transaction it touches is persisting. It matches the raw summary, which describes a persisting transaction acting as a retention hold, and the PR objectives, which describe the same fix for issue
#2256. The title is in the conventional commit form and fits the scope (db package).
Full details: Linked Issues check
-
Explanation
Issue
#2256requires that a Collection stay available while a transaction touching it persists. Its optimistic rows andawaitTxId/awaitMatchwaits must survive until settlement. GC timing may start after completion or rollback. Explicitcleanup()keeps its current behavior.Whole-PR diff (merge base 427a7c3 to head baa7d7d) shows the following implementation:
packages/db/src/collection/lifecycle.ts:canGarbageCollect()now also requires!this.state.hasPersistingTransaction(). Automatic GC therefore cannot start while a persisting transaction exists.packages/db/src/transactions.ts:touchCollection(releasedHold)callsstartGCTimerAfterSettlement()for each touched Collection when a persisting hold is released. This covers successful commits and failures. Rollback passes this flag only when the transaction was persisting, so a rollback of a pending manual transaction keeps its existing deadline. This matches the issue text and the documented boundary.startGCTimerAfterSettlement()cancels obsolete scheduled work before it schedules the renewed deadline. This prevents stale idle cleanup from running on the old deadline.- Explicit
cleanup()is not gated by the new hold. The PR description and thehasStartedCleanup()guard instate.tskeep explicit cleanup authoritative.
Test coverage is claimed in the PR: property oracles for retention and mutation shapes, a native Chromium harness, and an Electric/Postgres E2E test that checks
awaitTxIdacross the GC deadline. I did not run these tests. The status of this check depends on the stated validation results. The full local DB run had one replay child timeout, and its isolated rerun passed.All issue requirements are coding requirements. No non-coding requirement applies. The issue's public hold API request is conditional and does not apply, because the PR chose transaction-based retention.
Full details: Out of Scope Changes check
-
Explanation
Production changes are limited to the GC retention and cleanup path for Issue
#2256:lifecycle.ts: persisting-hold gate, startup grace, renewed deadline.transactions.ts: hold release on settlement and rollback.cleanup-queue.ts: sharedcleanupNow()clock. This supports the absolute handoff deadline.state.ts: skips replay of settled-transaction mutations after cleanup starts. This prevents discarded rows from returning. The change is needed because retention can now keep a transaction alive past cleanup. It does not alter explicit cleanup timing.types.tsanddocs/guides/mutations.md: document the new contract.mangle-cache.json: mangled names for the new private members.
Test and harness files support the same behavior: oracle property tests and models,
oracle-config.tsandoracle-replay-manifest.tsregistrations, the Chromium GC receiver (e2e/collection-gc/*, Playwright config,tsconfig.json,e2e-tests.yml), and the Electric E2E test. These are supporting tests and infrastructure. Their connection to the objective is explicit.The review-record documents and
sources.jsonfingerprints are process artifacts. They describe the same GC behavior and do not add unrelated product features. They increase review volume, but they do not show an unrelated change. The changeset is a patch for@tanstack/db, which matches the behavior change.No changed code shows an unrelated feature.
Full details: Description check
-
Explanation
The pull request description provides complete sections for changes, oracle coverage, validation, checklist, and release impact. It documents the motivation, behavior, evidence, test results, known limits, changeset, and issue closure. The headings differ slightly from the template, but all required information is present.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 16 files. (4 skipped: 4 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
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/preact-db
@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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔇 Additional comments: 19
packages/db/src/collection/cleanup-queue.ts: 1
3-6: LGTM!Also applies to: 38-38, 83-83, 96-96
packages/db/src/collection/lifecycle.ts: 1
13-13: LGTM!Also applies to: 54-54, 289-290, 308-320, 334-334
packages/db/src/transactions.ts: 1
679-679: LGTM!Also applies to: 698-698, 711-711, 720-720, 783-783, 838-838
packages/db/mangle-cache.json: 1
418-419: LGTM!packages/db/tests/collection-gc-retention-oracle.ts: 1
1-185: LGTM!packages/db/tests/collection-gc-transaction-oracle.property.test.ts: 1
8-8: LGTM!Also applies to: 14-16, 38-40, 104-104, 191-191, 253-253, 259-259, 301-301, 307-308, 321-321, 326-337, 396-398, 412-414
packages/db/tests/collection-gc-retention-oracle.property.test.ts: 1
1-434: LGTM!packages/db/package.json: 1
26-26: LGTM!Also applies to: 29-29
packages/db/tests/oracle-config.ts: 1
55-55: LGTM!packages/db/tests/oracle-replay-manifest.ts: 1
246-249: LGTM!Also applies to: 280-287
packages/db/tsconfig.json: 1
29-29: LGTM!packages/db/e2e/collection-gc/browser.html: 1
1-6: LGTM!packages/db/e2e/collection-gc/browser.ts: 1
1-193: LGTM!packages/db/e2e/collection-gc/retention.spec.ts: 1
1-46: LGTM!packages/db/playwright.collection-gc.config.ts: 1
1-19: LGTM!.github/workflows/e2e-tests.yml: 1
117-120: LGTM!Also applies to: 129-129
docs/contributing/oracle-reviews/pr-2258-gc-retention.md: 1
1-147: LGTM!docs/contributing/oracle-reviews/pr-2258-gc-retention.sources.json: 1
1-24: LGTM!docs/contributing/oracle-coverage.md-619-619: 1
619-619:Minor · Maintainability & Code Quality · Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Clarify the compound term.
Change
sync entrytosync-entry processingorentry into sync. LanguageTool flags the missing hyphen, and the current wording is ambiguous.Source: Linters/SAST tools
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: defaults
- Review profile: CHILL
- Plan: Advanced
- Run ID:
6f9e4c68-9aff-4b64-9e91-956e765d9a2d
-
.changeset/gc-waits-for-persisting-transactions.md.github/workflows/e2e-tests.ymldocs/contributing/oracle-coverage.mddocs/contributing/oracle-reviews/pr-2258-gc-retention.mddocs/contributing/oracle-reviews/pr-2258-gc-retention.sources.jsonpackages/db/e2e/collection-gc/browser.htmlpackages/db/e2e/collection-gc/browser.tspackages/db/e2e/collection-gc/retention.spec.tspackages/db/mangle-cache.jsonpackages/db/package.jsonpackages/db/playwright.collection-gc.config.tspackages/db/src/collection/cleanup-queue.tspackages/db/src/collection/lifecycle.tspackages/db/src/transactions.tspackages/db/src/types.tspackages/db/tests/collection-gc-retention-oracle.property.test.tspackages/db/tests/collection-gc-retention-oracle.tspackages/db/tests/collection-gc-transaction-oracle.property.test.tspackages/db/tests/oracle-config.tspackages/db/tests/oracle-replay-manifest.tspackages/db/tsconfig.json
-
- packages/db/src/types.ts
- .changeset/gc-waits-for-persisting-transactions.md
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 1 remain after this review.
-
There was a problem hiding this comment.
🔇 Additional comments: 1
docs/contributing/oracle-coverage.md-619-619: 1
619-619:Minor · Maintainability & Code Quality · Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Reword the ready-callback sentence.
The phrase
during entry into syncis difficult to parse and triggered the hyphenation check. Rewrite it as:A ready callback that throws while entering sync surfaces when the sync function returns.Source: Linters/SAST tools
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: defaults
- Review profile: CHILL
- Plan: Advanced
- Run ID:
4a6db227-9cac-4948-91ea-538063e97622
-
docs/contributing/oracle-coverage.md
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 1 remain after this review.
-
…ersisting-transactions
…ersisting-transactions # Conflicts: # packages/db/mangle-cache.json
Merges main at b8d8ee0 (#2258 GC waits for persisting transactions, #2257 electric shape-stream listener cleanup). - lifecycle: main's GC grace/handoff and persisting-transaction hold kept beside the PR's health state; main's syncError field is replaced by the PR's statusError. - load-subset replay oracle: main's keyed-request/reacquisition properties kept under the PR's request-error law: after initial-query readiness a fresh keyed request failure is a request error, not terminal, and a failed replay ends nothing. - GC retention oracle uses fail() for its first-load failure. - test:oracles runs both main's GC oracles and the PR's hydration oracle; mangle cache regenerated. Co-authored-by: Isaac <no-reply@databricks.com>
#2168 splits Collection status from sync errors. Its law 16 makes every lazy demand failure after initial-query readiness a request error, not a terminal one, and its design already starts new demand before it releases retired coverage, and treats a synchronous loadSubset throw as a rejection whose owner's release unloads it. The five conflicted production files take #2168's version, with only the fixes it lacks ported onto it: - the shared replay barrier across a query's subscriptions to one source (siblingReplays), and the failure hold behind a sibling's failed replay (failReplacement); - the deferred window move's wait for every replay that holds publication. A move now also rejects when a reload ends the sync run of the query or of a source, which #2168's source-health oracle requires. Dropped as superseded: the error-228 rule outside a replay, the replay a truncate opened without demand, this branch's synchronous-throw and aborted-request handling, and the window-request reset (a move can no longer throw, and deleting it failed nothing). Replay oracle: the strict terminal-error law becomes law 16 (a terminal error only after a load failed while the query was loading, at each history cut and at convergence), with #2168's request-error witness, and the dead excusal machinery goes. A thrown load counts as acquired, and a synchronous setWindow throw is an escaped error. Mutants: without the deferred-move loop 8 tests fail (#2221 trace 2 among them); without failReplacement the two-failures replay fails; without the sibling wait that replay and both failed-replay campaigns fail. ARCHITECTURE.md, the glossary's acquisition-attempt entry, the coverage map, and the changeset follow. Evidence: @tanstack/db 298 files, 15,520 of 15,521 tests (one random-seed reach miss in the GC oracle from #2258, which passes 3 of 3 reruns); query-db 42 files, 1,492 tests (2 skipped); tsc, ESLint, Prettier, and the mangle check pass. Co-authored-by: Isaac <no-reply@databricks.com>
Changes
Keep a Collection alive while a transaction that touches it is
persisting. A screen can submit a write and close while its mutation handler waits for server confirmation. Onorigin/main, automatic garbage collection can then clear the optimistic row and end the sync run before the handler settles. With Electric, cleanup can rejectawaitTxIdwithStreamAbortedErroreven after the server accepts the write. Issue #2256 reports this failure in web apps and Expo apps on iOS and Android.Every touched Collection now treats a persisting transaction as a retention hold, alongside active subscribers and pending preloads. After the final persisting hold ends, an otherwise inactive Collection starts a fresh
gcTimedelay. This keeps its optimistic state and sync run available while the mutation handler waits. A transaction that touches several Collections retains each one.Retention deadlines and cleanup
Transaction settlement notifies all Collections that track the transaction or appear in its mutations. The lifecycle manager cancels obsolete timer and idle callbacks before it schedules the renewed deadline. The cleanup queue and lifecycle manager share one elapsed clock. An absolute handoff deadline preserves the remaining 50ms startup or preload grace when a transaction settles quickly. A real last unsubscribe uses
gcTimewithout that startup minimum.Explicit
cleanup()still starts immediately, including while a transaction persists. Cleanup clears public rows synchronously, then waits for adapter cleanup before publishingcleaned-upstatus. Later transaction settlement now keeps another transaction's discarded optimistic rows absent during and after cleanup. The state manager still prunes settled transactions.The public configuration and mutation guide document these boundaries:
pendingpreserves its existing GC deadline.gcTimedisables automatic GC.The implementation shares transaction completion and mutation-merging paths. One timer entry point distinguishes settlement from last unsubscribe. Idle cleanup and queue processing no longer need separate dispatch methods. State calculation counts directly, and settled transactions use deferred sorted-map deletion. Current and prior rows share keyed reads and virtual-property snapshots, preserving upstream fields and previous-value semantics. These changes eliminate duplicate branches and temporary arrays while preserving the retention, publication, and cleanup contracts.
Oracle Coverage
Coverage-map owner: Collection lifecycle. The laws come from
BaseCollectionConfig.gcTime, existing startup/preload, ownership and cleanup contracts, and the explicit pending-draft and pending-rollback decisions.Independent models compute retention from logical holds, their last release, transaction phases, and the absolute handoff deadline. They import no production scheduler or lifecycle classifier. The boundary property runs 250 fixed-seed and 250 random histories. It requires all 33 declared reach states. Histories combine multiple subscribers, up to seven transactions, delayed commit, three persisting siblings, non-FIFO settlement, preloads, and held or rejecting automatic cleanup. The original two-Collection property requires all seven of its reach states.
A mutation receiving driver shares the retention model and adds an independent projection from source rows and mutation intent. It generates direct updates/deletes, shared update/delete, and canceled insert/delete ownership with a surviving peer update. Both Collection orientations, subscriber states, and completion/failure/rollback outcomes run across 150 fixed-seed and 150 random histories. All 11 declared states must be reached. A fully empty transaction cannot substitute for a genuinely persisting handler.
Comparisons check exact public keys and values, Collection status, transaction phases, caller receipts, and tracked unsettled transactions at the relevant cuts. Cleanup checks synchronous row discard separately from final status after adapter settlement. The driver verifies that idle delivery starts adapter cleanup before joining its promise. Rejection checks the cleanup error wrapper and exact adapter cause.
RED/GREEN evidence: The retention oracle rejects automatic GC during persistence without the fix and passes with the final implementation. Hostile source mutants fail the intended comparisons. They include missing retention, obsolete idle cleanup, lost startup grace, rows returning after cleanup, skipped transaction pruning, collapsed subscribers, and ownership limited to surviving mutations. An exact guarded seed/path replay rejects premature automatic-cleanup status, then passes with production restored.
Three Chromium histories pass with native idle provenance, budget, and cancellation evidence. Twelve Electric/Postgres paths pass with real provider I/O and controlled idle delivery. They cross
gcTime10/100ms with absent subscribers or a final unsubscribe. Eight paths use two separate SQL transactions and both local settlement orders. After the first genuine acknowledgement, the survivor remains SQL-open, unacknowledged, persisting, and caller-pending beyond the first-release deadline. Final confirmation checks source rows, virtual fields, caller success, and renewed GC. A partial-hold runtime mutant fails all eight sibling paths. A missing-hold mutant fails the four one-write paths. CI runs both receiving drivers and records native browser evidence.These finite histories establish native-host and real-provider behavior separately. They do not establish combined browser/provider scheduling, three-or-more provider writers, provider failure/recovery, or persistence-wrapper combinations. Fresh optimistic keys exclude combined GC histories with same-key rollback cascades or queued sync transactions. The coverage map names the receiving witness needed before extending those claims. Paced/offline restoration and restoration of discarded pending drafts remain outside this coverage.
Retention evidence, cleanup/provider evidence and contract decisions, interaction and mutation receiving evidence, and source fingerprints record experiments, observation cuts, reach counts, and limits.
Validation
pnpm size:compare --packages db --filescompares the whole PR with merge-base1640de834onorigin/main. DB decreases by 170B gzip / 39B Brotli. The typical app decreases by 182B gzip / 72B Brotli. Every measured feature scenario also decreases in both formats. The integration size record records every scenario, the shared-state refactors, calibration and verification evidence. No tests or expected results changed for this refactor.ccbcbecc22131383c6d3d0f3ed891ca22eee535a, including browser E2E and its native GC oracle. The latest CodeRabbit review coversbaa7d7d730329fdbf1fab30cc25a0932abc757da. Automated review is paused, so it does not cover the size refactor.Checklist
Release Impact
@tanstack/db.Closes #2256