Skip to content

Conversation

rkistner
Copy link
Contributor

@rkistner rkistner commented Oct 17, 2024

This ensures that:

  1. Update notifications are only fired outside transactions.
  2. Update notifications are throttled on the web version, similar to the native version.

If update notifications are fired inside transactions, watch queries running concurrently with the transaction will not see the applied updates. This is not an issue for single statements, but causes very confusing behavior when using transactions. It was not always an issue if a transaction completed quickly (within 10ms), but if there was ever a delay of more than 10ms (the previous throttle time) between the last update and the commit, the issue would occur.

This now reduces the throttle time to 1ms. Since notifications are not fired during transactions, there would still only be a single notification for an entire transaction.

For the web implementation, it is important for performance to implement the throttling inside the worker process. Otherwise, individual update notifications send over the MessagePort adds a very large overhead. To keep the API the same here, we still use the SqliteUpdate class. However, we merge updates with the same table and kind, setting rowId = 0. Since this is only used internally in sqlite_async, the rowId is never used anyway.

See powersync-ja/powersync-js#266 for a discussion of the same issue in PowerSync's web SDK.

Benchmarks using PowerSync with Flutter Web indicate this roughly halves the time required to sync 50k operations when using an in-memory database, from 10s -> 5s (using the IndexedDB VFS is still much slower).

Copy link
Contributor

@stevensJourney stevensJourney left a comment

Choose a reason for hiding this comment

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

Looks good to me

@rkistner rkistner marked this pull request as ready for review October 17, 2024 14:29
@rkistner rkistner merged commit e197601 into main Oct 17, 2024
6 checks passed
@rkistner rkistner deleted the fix-watch-transactions branch October 17, 2024 14:44
@rkistner rkistner mentioned this pull request Oct 28, 2024
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.

2 participants