Make Codebase object thread-safe #3195
Conversation
|
|
||
| runTransaction :: Sqlite.Transaction a -> m a | ||
| runTransaction action = | ||
| withConn \conn -> Sqlite.runTransaction conn action |
There was a problem hiding this comment.
I'm sure it's fine, but is it possible that using a single connection shared across the codebase gave us any benefits in terms of in-memory caching?
If there's anywhere we're querying the codebase in a tight-loop (there's probably at least some spots that do this) we're now acquiring a new connection for each and every query rather than using a single one.
I have no idea whether this is likely to actually cause problems in reality though 🤷🏼♂️
There was a problem hiding this comment.
I do think it'd be wiser to reuse connections wherever possible - @tstat suggested we just use a connection pool. In the meantime, I'm not sure about where we might be hitting these functions in a tight loop - I did time loading the root branch of full-history base into memory, and there wasn't any noticeable slowdown. Push/pull to Share, at least, aren't using this Codebase interface for storing terms, so we don't have a blowup of connection creation/destruction over there.
There was a problem hiding this comment.
Yeah, I thought about a pool but wasn't sure whether it makes sense or not given how much lighter-weight sqlite connections are. Wouldn't hurt though, and probably would be easy to wire into what you've got here right?
If it's not showing any noticeable difference maybe it's fine?
Overview
This PR makes the Codebase object thread-safe by making each operation use a new underlying connection to SQLite, rather than a shared connection that the Codebase closes over.
Fixes #3190, which was caused by the local UI and ucm simultaneously using the same SQLite connection.