feat(sqlite-persistence): prune applied_tx by default with safe replay recovery#1572
Conversation
createNodeSQLitePersistence left appliedTxPruneMaxRows/appliedTxPruneMaxAgeSeconds unset, so the applied_tx log grew without bound for every consumer. Default them to 1,000 rows and a 24h age backstop (both overridable; pass 0 to disable), exported as DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS / DEFAULT_APPLIED_TX_PRUNE_MAX_AGE_SECONDS.
|
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:
📝 WalkthroughWalkthroughAdds default applied_tx pruning (1,000 rows, 24h), exports the default constants, applies them during adapter option resolution across wrappers, makes pullSince return requiresFullReload when replay rows were pruned, documents the behavior, and adds tests covering pruning and disabling it. ChangesDefault applied transaction pruning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/node-db-sqlite-persistence/tests/node-persistence.test.ts (1)
130-243: ⚡ Quick winConsider adding test coverage for row-based pruning.
The existing tests verify age-based pruning and disabled pruning, but don't test the row cap (
DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS = 1_000). Adding a test that inserts more than 1,000 rows and verifies that only the most recent 1,000 are retained would complete the coverage of this feature's limit edge cases.As per coding guidelines, test corner cases including limit/offset edge cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/node-db-sqlite-persistence/tests/node-persistence.test.ts` around lines 130 - 243, Add a new test that verifies row-count based pruning: create a persistence via createNodeSQLitePersistence (or with appliedTxPruneMaxRows set explicitly to DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS), insert more than DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS transactions by repeatedly calling persistence.adapter.applyCommittedTx with increasing seq values, then query the applied_tx table (SELECT seq FROM applied_tx WHERE collection_id = ? ORDER BY seq ASC) and assert that only the most recent DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS seq values remain (i.e., lowest seq values were pruned); use the collectionId, applyCommittedTx, applied_tx and seq identifiers to locate relevant code paths and ensure cleanup (database.close and rmSync) as in the other tests.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/node-db-sqlite-persistence/tests/node-persistence.test.ts`:
- Around line 130-243: Add a new test that verifies row-count based pruning:
create a persistence via createNodeSQLitePersistence (or with
appliedTxPruneMaxRows set explicitly to DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS),
insert more than DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS transactions by repeatedly
calling persistence.adapter.applyCommittedTx with increasing seq values, then
query the applied_tx table (SELECT seq FROM applied_tx WHERE collection_id = ?
ORDER BY seq ASC) and assert that only the most recent
DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS seq values remain (i.e., lowest seq values
were pruned); use the collectionId, applyCommittedTx, applied_tx and seq
identifiers to locate relevant code paths and ensure cleanup (database.close and
rmSync) as in the other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9bd2f0d-b3df-4557-9a91-5254bc843291
📒 Files selected for processing (5)
.changeset/node-default-applied-tx-pruning.mdpackages/node-db-sqlite-persistence/README.mdpackages/node-db-sqlite-persistence/src/index.tspackages/node-db-sqlite-persistence/src/node-persistence.tspackages/node-db-sqlite-persistence/tests/node-persistence.test.ts
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/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/tauri-db-sqlite-persistence
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/react-native-db-sqlite-persistence/src/mobile-persistence.ts (1)
77-91: ⚡ Quick winExtract shared adapter default-resolution logic into a core utility.
Line 77-Line 91 duplicates near-identical base-option resolution already repeated across multiple wrappers; centralizing this would reduce drift risk when defaults evolve again.
As per coding guidelines: "Extract common logic into utility functions when identical or near-identical code blocks appear in multiple places".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-db-sqlite-persistence/src/mobile-persistence.ts` around lines 77 - 91, The function resolveAdapterBaseOptions duplicates base-option defaulting logic across wrappers; extract this logic into a shared utility (e.g., create a function like buildSQLiteCoreAdapterBaseOptions or normalizeAdapterBaseOptions) in the core utilities and have resolveAdapterBaseOptions call that central utility instead of inlining defaults; ensure the new utility accepts MobileSQLitePersistenceOptions (or a common options type) and returns Omit<SQLiteCoreAdapterOptions, `driver` | `schemaVersion` | `schemaMismatchPolicy`>, reusing DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS, DEFAULT_APPLIED_TX_PRUNE_MAX_AGE_SECONDS and handling pullSinceReloadThreshold the same way so all wrappers reference the single implementation.packages/browser-db-sqlite-persistence/src/browser-persistence.ts (1)
76-90: 🏗️ Heavy liftExtract shared pruning-default resolution to avoid wrapper drift.
resolveAdapterBaseOptionsnow duplicates the same applied-tx defaulting block that also exists in multiple wrapper packages. Consider centralizing this into a small shared helper (core or shared internal module) and reusing it from each wrapper to keep semantics aligned over time.As per coding guidelines: "Extract common logic into utility functions when identical or near-identical code blocks appear in multiple places."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/browser-db-sqlite-persistence/src/browser-persistence.ts` around lines 76 - 90, The resolveAdapterBaseOptions function duplicates applied-tx defaulting (appliedTxPruneMaxRows, appliedTxPruneMaxAgeSeconds using DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS / DEFAULT_APPLIED_TX_PRUNE_MAX_AGE_SECONDS); extract that logic into a small shared helper (e.g., getAppliedTxPruneDefaults or resolveAppliedTxPruneOptions) in a common/core/shared internal module and have resolveAdapterBaseOptions call the helper and merge its result (keep pullSinceReloadThreshold here), then update all wrapper packages that currently reimplement the same defaults to import and use the new helper so semantics stay consistent across packages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/browser-db-sqlite-persistence/src/browser-persistence.ts`:
- Around line 76-90: The resolveAdapterBaseOptions function duplicates
applied-tx defaulting (appliedTxPruneMaxRows, appliedTxPruneMaxAgeSeconds using
DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS / DEFAULT_APPLIED_TX_PRUNE_MAX_AGE_SECONDS);
extract that logic into a small shared helper (e.g., getAppliedTxPruneDefaults
or resolveAppliedTxPruneOptions) in a common/core/shared internal module and
have resolveAdapterBaseOptions call the helper and merge its result (keep
pullSinceReloadThreshold here), then update all wrapper packages that currently
reimplement the same defaults to import and use the new helper so semantics stay
consistent across packages.
In `@packages/react-native-db-sqlite-persistence/src/mobile-persistence.ts`:
- Around line 77-91: The function resolveAdapterBaseOptions duplicates
base-option defaulting logic across wrappers; extract this logic into a shared
utility (e.g., create a function like buildSQLiteCoreAdapterBaseOptions or
normalizeAdapterBaseOptions) in the core utilities and have
resolveAdapterBaseOptions call that central utility instead of inlining
defaults; ensure the new utility accepts MobileSQLitePersistenceOptions (or a
common options type) and returns Omit<SQLiteCoreAdapterOptions, `driver` |
`schemaVersion` | `schemaMismatchPolicy`>, reusing
DEFAULT_APPLIED_TX_PRUNE_MAX_ROWS, DEFAULT_APPLIED_TX_PRUNE_MAX_AGE_SECONDS and
handling pullSinceReloadThreshold the same way so all wrappers reference the
single implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e792d977-9c72-4c84-abfd-e4d981e1adf1
📒 Files selected for processing (15)
.changeset/node-default-applied-tx-pruning.mdpackages/browser-db-sqlite-persistence/src/browser-persistence.tspackages/browser-db-sqlite-persistence/src/index.tspackages/capacitor-db-sqlite-persistence/src/capacitor-persistence.tspackages/capacitor-db-sqlite-persistence/src/index.tspackages/cloudflare-durable-objects-db-sqlite-persistence/src/do-persistence.tspackages/cloudflare-durable-objects-db-sqlite-persistence/src/index.tspackages/db-sqlite-persistence-core/src/sqlite-core-adapter.tspackages/expo-db-sqlite-persistence/src/expo.tspackages/expo-db-sqlite-persistence/src/index.tspackages/node-db-sqlite-persistence/src/node-persistence.tspackages/react-native-db-sqlite-persistence/src/index.tspackages/react-native-db-sqlite-persistence/src/mobile-persistence.tspackages/tauri-db-sqlite-persistence/src/index.tspackages/tauri-db-sqlite-persistence/src/tauri-persistence.ts
✅ Files skipped from review due to trivial changes (4)
- packages/capacitor-db-sqlite-persistence/src/index.ts
- packages/tauri-db-sqlite-persistence/src/index.ts
- packages/cloudflare-durable-objects-db-sqlite-persistence/src/index.ts
- .changeset/node-default-applied-tx-pruning.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/node-db-sqlite-persistence/src/node-persistence.ts
Problem
The SQLite core adapter supports
appliedTxPruneMaxRows/appliedTxPruneMaxAgeSeconds, but most SQLite persistence wrappers left them unset by default. That means theapplied_txreplay log can grow without bound for consumers that don't opt in.This bit a downstream Electron app (Superset): the local sync DB (
~/.superset/tanstack-db.sqlite) bloated to hundreds of MB (501kapplied_txrows / 428 MB / 34 days on one real machine, ~97% no-op writes), which made endpoint-security agents continuously re-hash the file. The app fixed it by passing the two prune options, but this isn't app-specific — SQLite-backed persistence should not grow its replay cache forever by default.Change
SQLite persistence wrappers that construct the shared SQLite core adapter now default the prune options when the caller omits them:
appliedTxPruneMaxRows: 1_000— per-collection row cap (safety ceiling)appliedTxPruneMaxAgeSeconds: 86_400— 24h age backstopThis applies to:
@tanstack/browser-db-sqlite-persistence@tanstack/capacitor-db-sqlite-persistence@tanstack/cloudflare-durable-objects-db-sqlite-persistence@tanstack/expo-db-sqlite-persistence@tanstack/node-db-sqlite-persistence@tanstack/react-native-db-sqlite-persistence@tanstack/tauri-db-sqlite-persistenceThe existing per-collection prune already runs inside each write transaction, so every collection self-trims on its next sync. Both values stay fully overridable, and passing
0disables that limit (explicit0is preserved via??). The defaults are exported asDEFAULT_APPLIED_TX_PRUNE_MAX_ROWSandDEFAULT_APPLIED_TX_PRUNE_MAX_AGE_SECONDSfrom@tanstack/db-sqlite-persistence-coreand re-exported by wrappers.This PR also makes the shared SQLite core adapter retention-aware during
pullSincerecovery. Sinceapplied_txis a bounded replay cache,pullSincenow forcesrequiresFullReload: truewhen the requestedfromRowVersionpredates the retained replay window, instead of returning partial replay deltas. This preserves correctness when pruning removes older replay entries.Note: the Electron package is an IPC bridge around a main-process persistence adapter, so there is no separate SQLite adapter construction point there to default; it inherits the behavior of whichever main-process SQLite persistence adapter is supplied.
Verification
The downstream fix was verified on a copy of a real 448 MB / 503k-row DB: prune brought it to ~14k rows (
integrity_check: ok, all collections registered, no data loss). In this repo:node-persistence.test.tscover default age pruning, explicit row-cap pruning, disabled pruning, and full-reload fallback whenpullSincestarts before pruned replay rows.pnpm --filter @tanstack/node-db-sqlite-persistence test -- node-persistence.test.ts --runInBand@tanstack/db-sqlite-persistence-core@tanstack/browser-db-sqlite-persistence@tanstack/capacitor-db-sqlite-persistence@tanstack/cloudflare-durable-objects-db-sqlite-persistence@tanstack/expo-db-sqlite-persistence@tanstack/react-native-db-sqlite-persistence@tanstack/tauri-db-sqlite-persistence@tanstack/electron-db-sqlite-persistenceChangeset included.
Summary by CodeRabbit
New Features
Behavior Changes
Documentation