From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id uXSaNtxL316SSQAA0tVLHw (envelope-from ) for ; Tue, 09 Jun 2020 08:44:12 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id qHoyMtxL317TWAAA1q6Kng (envelope-from ) for ; Tue, 09 Jun 2020 08:44:12 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 15743940391 for ; Tue, 9 Jun 2020 08:44:12 +0000 (UTC) Received: from localhost ([::1]:32824 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jiZrd-00073R-CD for larch@yhetil.org; Tue, 09 Jun 2020 04:44:09 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48316) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jiZqY-0005w2-OE for guix-patches@gnu.org; Tue, 09 Jun 2020 04:43:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:47344) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jiZqX-0002a6-Rl for guix-patches@gnu.org; Tue, 09 Jun 2020 04:43:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jiZqX-0005to-Oi for guix-patches@gnu.org; Tue, 09 Jun 2020 04:43:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#41658] [PATCH] fixes / improvements for (guix store database) Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 09 Jun 2020 08:43:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 41658 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Caleb Ristvedt Cc: Danny Milosavljevic , 41658@debbugs.gnu.org Received: via spool by 41658-submit@debbugs.gnu.org id=B41658.159169214322633 (code B ref 41658); Tue, 09 Jun 2020 08:43:01 +0000 Received: (at 41658) by debbugs.gnu.org; 9 Jun 2020 08:42:23 +0000 Received: from localhost ([127.0.0.1]:58890 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jiZpr-0005sv-Gx for submit@debbugs.gnu.org; Tue, 09 Jun 2020 04:42:23 -0400 Received: from eggs.gnu.org ([209.51.188.92]:40160) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jiZpm-0005sf-0e for 41658@debbugs.gnu.org; Tue, 09 Jun 2020 04:42:17 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:37419) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jiZpg-0002Rx-6w; Tue, 09 Jun 2020 04:42:08 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=59270 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1jiZpZ-0004tv-Cn; Tue, 09 Jun 2020 04:42:05 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <87ftbernat.fsf@cune.org> <87a71i943g.fsf@gnu.org> <87o8pu5cjx.fsf@cune.org> Date: Tue, 09 Jun 2020 10:42:00 +0200 In-Reply-To: <87o8pu5cjx.fsf@cune.org> (Caleb Ristvedt's message of "Mon, 08 Jun 2020 00:52:50 -0500") Message-ID: <87h7vkmy07.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Spam-Score: -3.3 (---) X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Spam-Score: -1.01 X-TUID: 2XiL80CAi9L5 Hi, Caleb Ristvedt skribis: [...] >> 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. Awesome, thank you. >> 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. OK. >>> * 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=E2=80=99s a bit beyond my understanding, but I think you can also p= ush 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. Makes sense, thanks for explaining. > 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: No, it doesn=E2=80=99t. (Also, in the setup described above, there=E2=80= =99s only one guix-daemon instance and it accesses the database via the local file system.) > From 614213c80a7ea15f7aab9502e6c33206ac089d05 Mon Sep 17 00:00:00 2001 > From: Caleb Ristvedt > Date: Mon, 1 Jun 2020 18:50:07 -0500 > Subject: [PATCH 1/5] database: work around guile-sqlite3 bug preventing > statement reset > MIME-Version: 1.0 > Content-Type: text/plain; charset=3DUTF-8 > Content-Transfer-Encoding: 8bit > > guile-sqlite3 provides statement caching, making it unnecessary for sqlit= e 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 "acti= ve" > statement (see https://notabug.org/guile-sqlite3/guile-sqlite3/issues/12)= . We > work around this by wrapping sqlite-finalize with our own version that en= sures > sqlite-reset is called, which does The Right Thing=E2=84=A2. > > * guix/store/database.scm (sqlite-finalize): new procedure that shadows t= he > sqlite-finalize from (sqlite3). [...] > +(define (sqlite-finalize stmt) > + ;; As of guile-sqlite3 0.1.0, cached statements aren't reset when > + ;; sqlite-finalize is invoked on them (see > + ;; https://notabug.org/guile-sqlite3/guile-sqlite3/issues/12). This c= an > + ;; cause problems with automatically-started transactions: I think it=E2=80=99s enough to link to the upstream issue, which has the pr= oblem well documented. > From e3cf7be4491f465d3041933596d3caad1ea64e83 Mon Sep 17 00:00:00 2001 > From: Caleb Ristvedt > Date: Sun, 7 Jun 2020 22:30:41 -0500 > Subject: [PATCH 2/5] gnu: guile-sqlite3: add patch to fix sqlite-finalize= bug > > Adds patch that fixes > https://notabug.org/guile-sqlite3/guile-sqlite3/issues/12. This can be > discarded once the patch is integrated into the next guile-sqlite3 releas= e. > Note that the patch is identical to the pull request at > https://notabug.org/guile-sqlite3/guile-sqlite3/pulls/13. > > * gnu/packages/patches/guile-sqlite3-reset-on-sqlite-finalize.patch: new > patch. > * gnu/packages/guile.scm (guile-sqlite3): use it. > * gnu/local.mk (dist_patch_DATA): add it. I=E2=80=99d skip it: we have a workaround and the release may be out soon. Danny, thoughts on getting a new release out? The rest is still fine with me, thank you! Ludo=E2=80=99.