Repository navigation
fix(db): decide facade writes from the adapter's own row record - #2092
Merged
Merged
Conversation
The graph retracts every row it sent before it retires a bucket, so a row the adapter still holds from the graph is a contradictory signal. Retirement now throws (code 236) before it changes anything, and the flush restores the facades. The adapter's own written-row map detects the case even while a persisting transaction holds the retractions; a mutant that reads the stored rows instead fails both campaigns. The architecture also records the readiness limit after a failed flush. Co-authored-by: Isaac <no-reply@databricks.com>
🦋 Changeset detectedLatest commit: 6def9f5 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
Incremental update benchmarkComparing Overall median write time vs base: 1.00× · cold hydrate time: 0.96× (geometric mean of per-case ratios; lower is faster). Writes: 3 regression(s), 1 improvement(s) (threshold: ±20% and >0.05ms). Cold hydrate: 1 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.00×, cold 1.03×
100 rows/collection | source indexes: none | optimistic writes — geomean 1.06×, cold 1.09×
100 rows/collection | source indexes: manual | synced writes — geomean 1.06×, cold 0.80×, 1 change(s)
100 rows/collection | source indexes: manual | optimistic writes — geomean 0.98×, cold 0.94×
1,000 rows/collection | source indexes: none | synced writes — geomean 1.16×, cold 0.99×, 2 change(s)
1,000 rows/collection | source indexes: none | optimistic writes — geomean 0.88×, cold 0.90×
1,000 rows/collection | source indexes: manual | synced writes — geomean 1.04×, cold 0.91×, 1 change(s)
1,000 rows/collection | source indexes: manual | optimistic writes — geomean 0.97×, cold 1.03×
10,000 rows/collection | source indexes: none | synced writes — geomean 0.93×, cold 0.93×, 1 change(s)
10,000 rows/collection | source indexes: none | optimistic writes — geomean 1.03×, cold 1.12×, 1 change(s)
10,000 rows/collection | source indexes: manual | synced writes — geomean 0.95×, cold 0.97×
10,000 rows/collection | source indexes: manual | optimistic writes — geomean 0.98×, cold 0.86×
Runner: node v24.8.0, linux 6.17.0-1022-azure, AMD EPYC 7763 64-Core Processor. Timings on shared CI runners are noisy; treat small deltas as indicative only. |
Contributor
|
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:
📝 Walkthrough
|
🎯 Changes
A live query with Collection-valued includes keeps one child collection (a facade) for each bucket of child rows. The query graph retires a bucket when no parent uses it. Before it retires the bucket, the graph removes each row that it sent to the bucket.
A row that the graph sent and did not remove is therefore a contradictory signal from the graph. Before this change, retirement removed such rows silently, so a fault in the graph was hidden. Now retirement throws (code 240) before it changes anything, and the flush restores the child collections, as for any failed flush.
A retired child collection can still show rows that the graph did not send. Examples are an optimistic row from a user transaction and a sync commit that a persisting transaction holds. Those rows have no synced row, so they leave when their transactions settle. Retirement no longer writes removals for them. Those removals did nothing: with the full test suite and the oracle, a version without them passed every test.
A lost row behind a persisting transaction (also on
main)The adapter decided each write from the rows that the child collection shows. When a user transaction on a child collection persists, the child collection holds its sync commits, so the rows that it shows lag behind:
Now the adapter reads the projected synced rows of the child collection. These are the synced rows after every queued sync transaction applies, including one that a persisting transaction holds. The adapter uses them for each write, for the rollback snapshot, and for the retirement check. A key that the graph inserts and removes in one flush, absent before and after, gets no write. A rollback does not write a removal for it either.
A failed child commit left staged rows
A child commit that throws before it commits leaves its sync transaction open. Its staged writes are part of the projected rows. The rollback opened a second sync transaction, so the staged writes stayed and appeared later as extra rows, or as a false code 240. The rollback now writes into the open sync transaction and commits it.
A holder cannot clean up a child collection
Several parent rows can share one child collection. Before, a holder's
cleanup()stopped it for every parent that shared it. New parents then received the dead child collection, and they never saw its rows. Nowcleanup()on a child collection returns a rejected promise (code 241). The live query owns its child collections and disposes them itself. Ordinary Collections keep their cleanup and restart.An optimistic update of a child row threw (also on
main)A user can update a row of a child collection inside a transaction. Every such update threw "Bucket facade row has no public key" (code 146). Core builds the updated row as a copy of the stored row. The child collection found keys by object identity, so the copy had no key. Now each child row carries its key in
$key, the same value that readers already saw. The copy keeps the key, and the identity map is gone.Evidence
The rollback oracle (
packages/db/tests/query/bucket-facade-rollback-oracle.property.test.ts) generates these histories:prepare(). One kind of throw comes from a child commit that throws without committing.contradictoutcome). The flush must throw code 240 and restore every child collection.After each step it compares the rows, events, layout revisions, readiness and read work. It also checks that a retired child collection shows exactly its visible optimistic rows. Each history ends when the optimistic rows settle, and then the oracle compares all rows again.
live.cleanup()inside a root subscriber. The child collection's prepared publication must not reach its subscribers. A mutant whose adapter cleanup does not dispose its child collections fails it.@tanstack/db: 11,594 tests pass, with no type errors.bucket-facade-adapter.tsandcollection/state.ts.Limits
ARCHITECTURE.mdstates the law. A child collection thatresolve()created before its bucket became active can stay ready and empty after a failed flush. The next flush writes its rows.preload(),toArrayWhenReady()orsubscribeChanges(). Then the child collection starts again and staysloading. Nothing writes to it again.mainhas the same behavior.✅ Checklist
pnpm test.🚀 Release Impact
This pull request and its description were written by Isaac.