electric-sql / electric

Sync little subsets of your Postgres data into local apps and services.
https://electric-sql.com
Apache License 2.0
6.38k stars 152 forks source link

Ensure SQL queries required for the UI have higher priority than sync queries #667

Closed gorbak25 closed 11 months ago

gorbak25 commented 11 months ago

Right now all db accesses are serialized by a mutex https://github.com/electric-sql/electric/blob/435a2ad5295c6079355772c65130cb278976ddd1/clients/typescript/src/drivers/wa-sqlite/database.ts#L40C31-L40C46 This is ok when there is a few concurrent transactions. The problem is when electric syncs a lot of small transactions using the satelite protocol and at the same time there is a really important select query which MUST finish within a few hundred ms or otherwise my app appears broken. Right now when there's a sync with a few hundred small transaction the queries for the UI take 10-20 seconds to finish .-.

I would expect for queries initiated by the UI to have priority against sync queries. The problem would also be mitigated when batching updates on the satelite protocol would be introduced.

Consider replacing the mutex with a priority mutex and introduce a distinction between "background queries" and "interactive queries". With background queries we don't care when they finish, with interactive queries we need them to finish asap.

kevin-dp commented 11 months ago

Hi @gorbak25, prioritising the UI looks like a good idea. The async-mutex library we use does not seem to support priority-aware mutexes. If we want to support this efficiently we will need to implement the mutex ourselves (which should be relatively simple given JS' model (event loop, single threaded)).

thruflo commented 11 months ago

We should 💯 do this.

kevin-dp commented 11 months ago

Closing this one as i created a Linear issue https://github.com/electric-sql/electric/issues/674 to track this