Re: [Monotone-devel] mtn: error: sqlite error: database is locked

[Markus Schiltknecht]

Hi Zack,

Zack Weinberg wrote:
> 1) Take the 'db' member out of class app_state.  A database object
> should be allocated on the stack by each CMD() function that actually
> needs it, and passed down to all underlying layers as a function
> argument.  At this stage it would also be good to refactor the
> database object using the pointer-to-opaque idiom, so that none of its
> instance variables are visible.

This is mostly done in n.v.m.experiment.encapsulation. Unfortunately I 
didn't merge for a while and there are multiple conflicts.

> 2) Split class database into class ro_database and class rw_database,
> the former exposing only those member functions that do read-only
> database operations.  Adjust all CMD functions to create the
> appropriate one for what they do; adjust lower layer function
> prototypes to request the one they need.  rw_database should be a
> subclass of ro_database so that it's an acceptable argument to
> functions that take ro_database.

I once tried that, but I'm unsure how well that fits. Lots of commands 
unexpectedly write to the database, and be it only setting the default 
database path or branch.

As Alvaro pointed out downthread, it's a pretty SQLite-like view on a 
database, which monotone is suffering already anyway. Instead of hacking 
around limitations of SQLite, I'd rather vote for a more generally 
usable SQL interface.

> 3) Change transaction_guard so that it takes out a read lock if
> created from a ro_database.  Change the "unlocked" sql execute
> functions to do the same.

Again, this only applies to SQLite. For any kind of a real database 
(i.e. which supports concurrently writing transactions), you should 
rather be able to cope with serialization failures from the database and 
try to reapply your transaction.

As conflicts can safely be expected to be very rare, we should better go 
that optimistic way, instead of the trying to manually do more fine 
grained locking (pessimistic) on top of SQLite.

Thus, instead of a ro_database and a rw_database class, which can by 
design only serve SQLite, I'm rather voting for a unified database 
interface.

> 4) It may be necessary to make changes to sqlite so that it opens the
> database file read-only, and uses shared file locks, if it's only
> being asked to do read operations.

Doesn't SQLite already support that?

Regards

Markus