[bug#33210] Cuirass: Use a SQLite in single-thread mode

[Danny Milosavljevic <dannym@scratchpost.org>]

[-- Attachment #1: Type: text/plain, Size: 3820 bytes --]

Hi Clément,

> rather basic way.  But if I understand correctly, the overall spent time
> is more or less the same: only the order of the requests differs.

Yeah, right now users can query something using the web interface while
a build is updating (or running) at the cost of the returned data being
potentially very strange.

The "one-line fix" would make it worse in that users cannot query while
a build is running, making them wait until the build is done (approx.
30 min) before the query succeeds.  The upside is that the returned
data is consistent at all times.  This is how DBMSes did it in the 90s,
too.

What I'd like to eventually have is the proper fix where users can query
while a build is running, *and* the build doesn't have to wait either.
This works just fine using two transactions with WAL mode of sqlite,
which means it uses MVCC in order to keep both "world views", one for the
querier and one for the builder (easily extended to an arbitrary
number of queriers and builders at once by just having more transactions)
while they are both using "the world".

> is an essential difference however: if we take care of the scheduling,
> we won't have SQLITE_BUSY blocking the Fibers scheduler all the time.
> And blocking the Fibers scheduler has an impact on all other possibly
> unrelated Fibers clients.

Right.  I just wanted to make sure we understand the possible implications here.

In the end I'm not sure we even need multithreading even for my scenario -
maybe (probably) just having an extra sqlite_open would be enough, threads
or not.  On the other hand there are shared caches etc and this change here
could cause some very tricky problems then.

I have to say I liked the external evaluator much more since then all
this complexity would be contained in the external program and it would
just magically work without special-casing any of this stuff.

>  When guile-sqlite3 handles SQLITE_BUSY
> correctly, I'll be happy to switch back to a multi-threading SQLite.
> While waiting for it, I believe our users want a fast Cuirass, and I'd
> like the time spent in the Fibers scheduler to be balanced by removing
> the SQLite now useless mutexes.

That makes sense.

It's difficult for guile-sqlite3 to handle SQLITE_BUSY correctly since
sqlite also uses SQLITE_BUSY to indicate errors that you are supposed to
fail on.

In the non-presence of a busy handler, it's not possible to distinguish
whether the SQLITE_BUSY was of the "please retry" kind or of the "don't
you retry" kind.

It would mean that guile-sqlite3 would have to have its own flag that
indicates whether the busy handler was called, and check this one.
Resetting this flag would also have to be potentially thread-safe
(for other users of guile-sqlite3).

That's always assuming that sqlite3 undos whatever it was trying to
do before returning SQLITE_BUSY so it actually makes sense to retry
the call later.

So something like this:

guile_sqlite_handle_busy(...) {
  guile_struct->busy_handler_called = true;
  return 0; // fail
}

guile_sqlite_open {
  int rc = native_sqlite_open(...);
  native_sqlite_set_busy_handler(..., guile_sqlite_handle_busy);
  // FIXME: check for errors here and fail on error
  guile_struct->busy_handler_called = false;
}

guile_sqlite_method {
  int rc, busy_handler_called;
  do {
    rc = native_sqlite_method(...);
  } while (rc == SQLITE_BUSY && (busy_handler_called = test-and-reset(guile_struct->busy_handler_called), yield));
  return rc;
}

Hmmmmmmmm.  I think that can be done.

Notes for myself:

pager.c
        busyHandler
                btreeInvokeBusyHandler
                        sqlite3BtreeBeginTrans
        sqlite3PagerSetBusyhandler
        SQLITE_FCNTL_BUSYHANDLER 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]