Skip to content

fix race condition in loading joined collections into live query#451

Merged
samwillis merged 4 commits intomainfrom
samwillis/fix-livequery-race
Aug 26, 2025
Merged

fix race condition in loading joined collections into live query#451
samwillis merged 4 commits intomainfrom
samwillis/fix-livequery-race

Conversation

@samwillis
Copy link
Collaborator

@samwillis samwillis commented Aug 26, 2025

Fixes #438

The issue was that the subscription to the live changes could start emitting results before we had first done any join processing. The join called either loadKeys or loadInitialState depending on if there was an index it could use. These now set a flag to indicate that we now actually want to start tailing the live changes, until then they are filtered out. We still have to send an empty array of changes when there are any before then so that the query is notified of any collection status change.

Failed test run with repo before it's then fixed: https://github.com/TanStack/db/actions/runs/17233860827/job/48894002331?pr=451

@changeset-bot
Copy link

changeset-bot bot commented Aug 26, 2025

🦋 Changeset detected

Latest commit: 30f29e4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@tanstack/db Patch
@tanstack/electric-db-collection Patch
@tanstack/query-db-collection Patch
@tanstack/react-db Patch
@tanstack/solid-db Patch
@tanstack/svelte-db Patch
@tanstack/trailbase-db-collection Patch
@tanstack/vue-db Patch

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Aug 26, 2025

More templates

@tanstack/db

npm i https://pkg.pr.new/@tanstack/db@451

@tanstack/db-ivm

npm i https://pkg.pr.new/@tanstack/db-ivm@451

@tanstack/electric-db-collection

npm i https://pkg.pr.new/@tanstack/electric-db-collection@451

@tanstack/query-db-collection

npm i https://pkg.pr.new/@tanstack/query-db-collection@451

@tanstack/react-db

npm i https://pkg.pr.new/@tanstack/react-db@451

@tanstack/solid-db

npm i https://pkg.pr.new/@tanstack/solid-db@451

@tanstack/svelte-db

npm i https://pkg.pr.new/@tanstack/svelte-db@451

@tanstack/trailbase-db-collection

npm i https://pkg.pr.new/@tanstack/trailbase-db-collection@451

@tanstack/vue-db

npm i https://pkg.pr.new/@tanstack/vue-db@451

commit: 30f29e4

// filter out deletes for keys that have not been sent
continue
}
this.sentKeys.add(change.key)
Copy link
Collaborator Author

@samwillis samwillis Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were only adding keys to the sentKeys when they were requested, not when they were send due to just watching the live changes.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2025

Size Change: +25 B (+0.04%)

Total Size: 64.3 kB

Filename Size Change
./packages/db/dist/esm/query/live/collection-subscriber.js 2.27 kB +25 B (+1.11%)
ℹ️ View Unchanged
Filename Size
./packages/db/dist/esm/change-events.js 1.13 kB
./packages/db/dist/esm/collection.js 10.6 kB
./packages/db/dist/esm/deferred.js 230 B
./packages/db/dist/esm/errors.js 3 kB
./packages/db/dist/esm/index.js 1.52 kB
./packages/db/dist/esm/indexes/auto-index.js 745 B
./packages/db/dist/esm/indexes/base-index.js 605 B
./packages/db/dist/esm/indexes/btree-index.js 1.74 kB
./packages/db/dist/esm/indexes/lazy-index.js 1.25 kB
./packages/db/dist/esm/local-only.js 827 B
./packages/db/dist/esm/local-storage.js 2.03 kB
./packages/db/dist/esm/optimistic-action.js 294 B
./packages/db/dist/esm/proxy.js 4.19 kB
./packages/db/dist/esm/query/builder/functions.js 575 B
./packages/db/dist/esm/query/builder/index.js 3.79 kB
./packages/db/dist/esm/query/builder/ref-proxy.js 890 B
./packages/db/dist/esm/query/compiler/evaluators.js 1.48 kB
./packages/db/dist/esm/query/compiler/expressions.js 631 B
./packages/db/dist/esm/query/compiler/group-by.js 2.06 kB
./packages/db/dist/esm/query/compiler/index.js 2.15 kB
./packages/db/dist/esm/query/compiler/joins.js 2.36 kB
./packages/db/dist/esm/query/compiler/order-by.js 1.17 kB
./packages/db/dist/esm/query/compiler/select.js 655 B
./packages/db/dist/esm/query/ir.js 466 B
./packages/db/dist/esm/query/live-query-collection.js 333 B
./packages/db/dist/esm/query/live/collection-config-builder.js 2.48 kB
./packages/db/dist/esm/query/optimizer.js 2.63 kB
./packages/db/dist/esm/SortedMap.js 1.24 kB
./packages/db/dist/esm/transactions.js 2.29 kB
./packages/db/dist/esm/utils.js 419 B
./packages/db/dist/esm/utils/btree.js 6.02 kB
./packages/db/dist/esm/utils/comparison.js 718 B
./packages/db/dist/esm/utils/index-optimization.js 1.62 kB

compressed-size-action::db-package-size

@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2025

Size Change: 0 B

Total Size: 1.16 kB

ℹ️ View Unchanged
Filename Size
./packages/react-db/dist/esm/index.js 152 B
./packages/react-db/dist/esm/useLiveQuery.js 1.01 kB

compressed-size-action::react-db-package-size

@samwillis samwillis requested a review from kevin-dp August 26, 2025 09:37
@samwillis samwillis changed the title WIP fix race condition in loading joined collections into live query fix race condition in loading joined collections into live query Aug 26, 2025
@samwillis samwillis marked this pull request as ready for review August 26, 2025 09:37
Copy link
Contributor

@kevin-dp kevin-dp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, this was a really hard one to find!
I left two very minor comments.

this.sendVisibleChangesToPipeline(changes, loadedInitialState)
// We are filtering the changes out when `sendChanges` is false, but still sending
// an empty array to the pipeline. This is needed to ensure that the pipeline
// receives the status update that the collection is now ready.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add some more comments explaining why the filtering is needed:

We filter out changes when sendChanges is false to ensure that we don't send any changes from the live subscription until the join operator requests either the initial state or its first key. This is needed otherwise it could receive changes which are then later subsumed by the initial state (and that would lead to weird bugs due to the data being received twice).

type Player = { id: number; name: string }
type Challenge = { id: number; value: number }

let playerBeginCallback: (() => void) | undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code for creating the collections and storing their callbacks is the same for Players and challenges. Could define a helper function that does this such that we don't have to duplicate the code. And then just call it once per collection.

@samwillis samwillis merged commit 48d0889 into main Aug 26, 2025
8 of 10 checks passed
@samwillis samwillis deleted the samwillis/fix-livequery-race branch August 26, 2025 10:58
@github-actions github-actions bot mentioned this pull request Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple joins not follow SQL semantics

2 participants