Ludovic Courtès writes: > Hi, > > Thanks for the thorough investigation and for the patches! > > Caleb Ristvedt skribis: > >> From cce653c590be1506e15044e445aa9805370ac759 Mon Sep 17 00:00:00 2001 >> From: Caleb Ristvedt >> Date: Mon, 1 Jun 2020 18:50:07 -0500 >> Subject: [PATCH 1/4] database: work around guile-sqlite3 bug preventing >> statement reset >> MIME-Version: 1.0 >> Content-Type: text/plain; charset=UTF-8 >> Content-Transfer-Encoding: 8bit >> >> guile-sqlite3 provides statement caching, making it unnecessary for sqlite to >> keep re-preparing statements that are frequently used. Unfortunately it >> doesn't quite emulate the semantics of sqlite_finalize properly, because it >> doesn't cause a commit if the statement being finalized is the last "active" >> statement. We work around this by wrapping sqlite-finalize with our own >> version that ensures sqlite-reset is called, which does The Right Thing™. >> >> * guix/store/database.scm (sqlite-finalize): new procedure that shadows the >> sqlite-finalize from (sqlite3). > > Nice. It would be great if you could report it upstream (Danny and/or > myself can then patch it directly in guile-sqlite3 and push out a > release) and refer to the issue from here. Reported as https://notabug.org/guile-sqlite3/guile-sqlite3/issues/12, with corresponding pull request https://notabug.org/guile-sqlite3/guile-sqlite3/pulls/13. > We can have this patch locally in the meantime, unless it would break > once the new guile-sqlite3 is out. WDYT? I've attached an updated patch series that both includes the guile-sqlite3 fix as a patch to the guile-sqlite3 package and adopts the workaround for situations where older guile-sqlite3's must be used (for example, when building guix from scratch on foreign distros that haven't incorporated the fix yet). The only changes are the addition of the now-second patch and fixing up some spacing in the comment in the first patch. >> From 7d34c27c33aed3e8a49b9796a62a8c19d352e653 Mon Sep 17 00:00:00 2001 >> From: Caleb Ristvedt >> Date: Mon, 1 Jun 2020 21:43:14 -0500 >> Subject: [PATCH 3/4] database: ensure update-or-insert is run within a >> transaction >> >> update-or-insert can break if an insert occurs between when it decides whether >> to update or insert and when it actually performs that operation. Putting the >> check and the update/insert operation in the same transaction ensures that the >> update/insert will only succeed if no other write has occurred in the middle. >> >> * guix/store/database.scm (call-with-savepoint): new procedure. >> (update-or-insert): use call-with-savepoint to ensure the read and the >> insert/update occur within the same transaction. > > That’s a bit beyond my understanding, but I think you can also push this > one. :-) Basically, it's like combining the body of two separate compare-and-swap loops into a single compare-and-swap loop. This ensures that the view is consistent (since if it isn't, the "compare" will fail and we'll retry). It addresses a problem that doesn't exist in practice yet, since update-or-insert is only called from within a call-with-transaction currently. But if someone ever wanted to call it from outside of a call-with-transaction, this would ensure that it still worked correctly. > Make sure “make check TESTS=tests/store-database.scm” is still happy. Works on my system. Related question: does berlin export /var/guix over NFS as per http://hpc.guix.info/blog/2017/11/installing-guix-on-a-cluster? If so, that could interact poorly with our use of WAL mode: "All processes using a database must be on the same host computer; WAL does not work over a network filesystem." - https://sqlite.org/wal.html. - reepca