Repository navigation
fix(db): publish a retired replay's rows instead of keeping stale ones - #2121
Merged
Merged
Conversation
The publication oracle's model let no request change a row the subscriber had already seen. After a replay retired because its last owner was released, the source kept a newer value and the subscriber kept showing the old one indefinitely. Production reconciles that row on the next request, which subscription.ts documents as intended. The model now expects the update, the law prose says why, and seed -136205996 is a named history. The witness fails against the previous model and against a production mutant that skips the reconciliation. Co-authored-by: Isaac <no-reply@databricks.com>
When the last requested subset was released during a truncate replay, the replay retired without publishing, and the subscriber kept values the source had written, replaced or deleted during it. A later request updated only its own row; other rows stayed stale until a write changed them, and a delete was lost. A replay whose current attempt has not failed now publishes the rows it holds when it retires. A failed replay keeps the last public snapshot (law 9), now even after its failed owner is released: releasing a demand deleted its failure, so that guard needs a flag that outlives the release. Both subscription oracles' models follow the new law. Named histories cover a value written, rewritten or deleted during the replay, a row no demand names, and a failed replay; they fail on main, and a mutant that publishes failed replays fails the failed-replay witness. ARCHITECTURE.md and the coverage map state the rule. Co-authored-by: Isaac <no-reply@databricks.com>
A retired replay seeded its private rows from the public snapshot, so a row retained by an earlier failed replay and deleted by the source stayed visible and lost its stale marker. Share flushTruncateReplay's publish step, which removes stale keys first, with retireEmptyReplay. Add the two ghost-row histories to the publication oracle, and witnesses for each synchronous failure site at retirement to the replay oracle. Compare the retire batch's contents in the replay oracle, not only its count. Co-authored-by: Isaac <no-reply@databricks.com>
A retired replay that has not failed now finishes through flushTruncateReplay, so its completion resolves with the published rows instead of rejecting with AbortError. No production consumer reads that completion on this path; the tests that pinned the abort now expect it to resolve. Co-authored-by: Isaac <no-reply@databricks.com>
🦋 Changeset detectedLatest commit: a4d1ef0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 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 |
Contributor
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (2 skipped: 2… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description check The description follows the repository template. It explains the change and motivation, confirms local testing, and identifies the generated changeset. Title check The title clearly and concisely describes the main change: publishing a retired replay's rows instead of retaining stale rows. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 2
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- 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.
Contributor
Incremental update benchmarkComparing Overall median write time vs base: 1.05× · cold hydrate time: 1.03× (geometric mean of per-case ratios; lower is faster). Writes: 3 regression(s), 1 improvement(s) (threshold: ±20% and >0.05ms). Cold hydrate: 2 regression(s), 1 improvement(s) (threshold: ±50% and >5ms). Per-case flags are noisy on shared runners. Read the geometric means first. Writes
Cold hydrate
Each row aggregates the 14.4 scale/index/write-mode configurations of that query; per-configuration tables below. 100 rows/collection | source indexes: none | synced writes — geomean 1.14×, cold 1.00×
100 rows/collection | source indexes: none | optimistic writes — geomean 1.13×, cold 1.02×
100 rows/collection | source indexes: manual | synced writes — geomean 0.91×, cold 0.93×
100 rows/collection | source indexes: manual | optimistic writes — geomean 1.09×, cold 0.83×
1,000 rows/collection | source indexes: none | synced writes — geomean 1.28×, cold 0.87×, 3 change(s)
1,000 rows/collection | source indexes: none | optimistic writes — geomean 0.87×, cold 0.98×
1,000 rows/collection | source indexes: manual | synced writes — geomean 1.16×, cold 1.12×, 1 change(s)
1,000 rows/collection | source indexes: manual | optimistic writes — geomean 1.01×, cold 1.30×
10,000 rows/collection | source indexes: none | synced writes — geomean 0.89×, cold 0.97×
10,000 rows/collection | source indexes: none | optimistic writes — geomean 1.00×, cold 1.03×
10,000 rows/collection | source indexes: manual | synced writes — geomean 1.12×, cold 1.53×, 1 change(s)
10,000 rows/collection | source indexes: manual | optimistic writes — geomean 1.11×, cold 0.96×, 2 change(s)
Runner: node v24.8.0, linux 6.17.0-1022-azure, AMD EPYC 9V45 96-Core Processor. Timings on shared CI runners are noisy; treat small deltas as indicative only. |
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: +149 B (+0.07%) Total Size: 215 kB 📦 View Changed
ℹ️ View Unchanged
|
…quest-event # Conflicts: # docs/contributing/oracle-coverage.md
Co-authored-by: Isaac <no-reply@databricks.com>
Merged
Contributor
|
🎉 This PR has been released! Thank you for your contribution! |
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
1 participant
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.
🎯 Changes
A subscriber no longer keeps stale rows after a truncate replay that loses its last demand.
A truncate starts a replay. The subscription reloads each subset it still requests into a private buffer. It publishes the buffer when every reload finishes. If the app releases the last requested subset while the replay is loading, nothing can finish the replay. Before this change, the subscriber then kept the rows from before the truncate, even when the source replaced or deleted them during the replay.
The law
When the last demand of a replay is released:
pendingTruncateReplacement) resolves.lastErrorandloadSubset:error, so an app can show that the refresh failed.Implementation
retireEmptyReplayfinishes a replay that has not failed throughflushTruncateReplay, so both paths publish the same way.flushTruncateReplaymoves its publish step intopublishReplayRows. This step drops stale retained keys and then publishes the diff.TruncateReplayStatehas afailedflag for the current attempt. Thefailuresmap cannot keep this record: release drops a demand's entry, so that the map does not keep a reference to that demand.subscription.tsis +33/−6.Evidence
everFailedfield applies only at full retirement, so a released demand's failure still cannot stop a successful replacement for the remaining demands.failedflag and compares the contents of the retirement batch with the expected batch.failedassignment at a site, the witness for that site fails. Without the assignment on the asynchronous path, five publication-oracle histories fail.AbortErrorfor this retirement now expect the completion to resolve. Only these tests read this outcome. The live-query builder and effects use delegated publication, which this path does not change. Releasing a demand already rejects that caller's request promise.@tanstack/db: 12,084 tests pass, with no type errors.Limits
The generated grammars rarely reach a failed retirement followed by another retired replay. The named histories own that sequence. The coverage map records this limit.
✅ Checklist
pnpm test.🚀 Release Impact
This pull request and its description were written by Isaac.
Summary by CodeRabbit