Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not notify listeners of updates until after xCommit #48

Open
tantaman opened this issue Dec 12, 2023 · 2 comments
Open

Do not notify listeners of updates until after xCommit #48

tantaman opened this issue Dec 12, 2023 · 2 comments

Comments

@tantaman
Copy link
Contributor

We currently call reactivity listeners on xUpdate

We should hold off and not do this till xCommit given the transaction could be rolled back.

@tantaman
Copy link
Contributor Author

This is generally not a problem right now since listeners enqueue a query after being notified. That query is not allowed to run until the next transaction but if the transaction was reverted there was no reason to issue the query in the first place.

Also -- batching xUpdates callbacks into a single callback at the end of a transaction would improve perf.

cr-sqlite uses xCommit internally so we can't register a second xCommit callback since SQLite only allows one to be registered at a time. We need to instead expose a xCommitMulti that allows many commit listeners.

@tantaman
Copy link
Contributor Author

tantaman commented Dec 12, 2023

This is generally not a problem right now since listeners enqueue a query after being notified. That query is not allowed to run until the next transaction

hmm, this is be a bug in the cross tab scenario where a query issued due to a notification from xUpdate will not wait until transaction commit to read.

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

No branches or pull requests

1 participant