689894 - Improve concurrency by having separate connections for writers and readers

Status: RESOLVED WONTFIX

(Toolkit :: Places, defect)

Description

[Marco Bonardo [:mak]]

Split from the discussion in bug 689229.

Using sync and async Storage APIs on the same connection has mutex implications we'd like to avoid, since the main reason to do that is TEMP stuff and we removed most of it we can start doing this, and once ready completely split the two paths.

Actually we have 2 temp triggers on moz_historyvisits, so any query writing to this table should live in the same connection, apart that, should be fine to split any other usage.
Once bug 431274 is fixed we can move to a disruptive downgrade path where we throw away and rebuild the database, at that point we can also remove this last requirement.

Comment 1

[Marco Bonardo [:mak]]

So, we cannot do this in a single step and most likely not now, the problem is the sync API, it will often return SQLITE_BUSY when there are multiple concurrent writers. That's not a big deal for async API since it can just retry later, but that's not feasible for sync API.
What may be feasible is to have a connection for writers, a connection for sync readers and a connection for async readers, but this on the other side will increase memory usage, since each connection has its own cache. Should evaluate if we can drop some of the clones by having more connections in the base, we cannot clearly drop the locationbar connection, maybe we could survive delaying isvisited state for links and reuse that connection?
Btw, the current structure makes hard to do rearchitecturing, thus I'm splitting in a separate bug all the refactoring work I've done so far, that splits out a Database class out of history, with specific statements cache and usable by all Places services, having a single point of access will be useful for a lot of the future refactorings.

Comment 2

[Marco Bonardo [:mak]]

I'm temporary leaving this on the back burner while refactoring other parts, the new Database class will simplify this work though. A thing to remember here is that incrementing connections we are also incrementing memory usage.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Is this still on or should we rather focus on getting rid of sync readers/writers?

Comment 4

[Marco Bonardo [:mak]]

we should definitely get rid of sync readers and writers, this was filed as an interim solution, but ended up being tricky due to concurrency problems and lack of resources.