SQLite queries happen (block) on the reactor thread
reivilibre opened this issue · 3 comments
They should really be put on a thread pool so the reactor can continue and await on the results properly (like we do in Synapse).
Even if that was a single-threaded threadpool.
That said, it would be good to think about allowing multiple read-only queries to run at the same time, since they only need a shared lock and can coexist happily.
(N.B. WAL mode #472 allows some readers to coexist with a writer too — we may consider turning that on as well and what implications that has. Perhaps we should just exclude queries that are going to do updates — that way, we reduce the risk of 'database is locked' errors and so on. Or maybe we should just retry queries when that happens.)
There's https://github.com/omnilib/aiosqlite FWIW, though I have no idea how hard it would be to make twisted and asyncio coexist. Presumably horribly hard.
There's omnilib/aiosqlite FWIW, though I have no idea how hard it would be to make twisted and asyncio coexist. Presumably horribly hard.
My experience says to avoid it at most costs — likely we should just use the same bits of Twisted that Synapse does for database queries.
Also worth noting that fixing this by doing something closer to Synapse's style might mean we can turn up the logging for SQL queries, which would have been useful now.