From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2.migadu.com ([2001:41d0:303:e224::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms8.migadu.com with LMTPS id QGGlNmEA1WXrNwAAe85BDQ:P1 (envelope-from ) for ; Tue, 20 Feb 2024 20:41:22 +0100 Received: from aspmx1.migadu.com ([2001:41d0:303:e224::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2.migadu.com with LMTPS id QGGlNmEA1WXrNwAAe85BDQ (envelope-from ) for ; Tue, 20 Feb 2024 20:41:22 +0100 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=none; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1708458081; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=/QzBvZPxWeAj2RWnRu4xX9VPMbbA8LdfPT66QlxNEcU=; b=RjR1dbPysLgfbsy3E2oRZwfxyn1pi3/QG+yePO5fNzny5wjOKKC0S5P1g3tcGrmEry8uXA lBGf9oOBelb+uKhsH7MamcyY9LwDx3djYM1+xw2s0lWGwD280Uj+hTqJvhphV7nd2jnwbH 25mdw2z3Q6H4u3VTaeAjv5kMUfbowtNTJmrVgVIkRoKKPik4w9yaHatLMJ61kHfgBqWcVv N3zRRY9eDtfrKS2WqeZ7TA3o3u0BeT6Pg7+yRLGJ90lPsBUoe16BGQ+dZDYIGS7p0h7uZP 3I8o22Iak6uLMKdPvztFzrC4Ocon+qlHX9u7ZRhBI1Pd/GeV0fUnou6sGg+kyA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=none ARC-Seal: i=1; s=key1; d=yhetil.org; t=1708458081; a=rsa-sha256; cv=none; b=h0FHaXRZ1JIF9MjzV9gHB6uGr1guELFNIiHK1IzF6Zc1GiyO3vaUEPDFxdGM2ZXsIa96Dh Bj6BNYgwtDaB40BmbBuQRnctBlOQwxUHqGLEpZlkZcBENNKukIwXq4ZUKTHtpRSQXAgwed sTlcaA+f8rCg2oNTrUhjQYHSRcOMU5SnDlWpEqK6RHbTPloTWQcHM3dOtB3UQJ6YyENJSH zEnPd5CUziAopzKzhHYecXwYGybzswZlCovNMyAVNbJQiZP8nC2eG5ZyW3hDovXnFVs6b3 gfUKH0XUSxt6jRtp4DRDUu2zSwZEigj86GP9ebbRCbMpvRdvdclnCT4JHAXwkg== 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 95554184F6 for ; Tue, 20 Feb 2024 20:41:21 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rcVzL-00036x-SK; Tue, 20 Feb 2024 14:41:11 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rcVys-0002kd-Em for guix-patches@gnu.org; Tue, 20 Feb 2024 14:40:43 -0500 Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rcVys-0000oz-6M; Tue, 20 Feb 2024 14:40:42 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rcVzB-0002Up-Nm; Tue, 20 Feb 2024 14:41:01 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#69292] [PATCH 1/6] store: database: Remove call-with-savepoint and associated code. References: <87h6i3lyph.fsf@cbaines.net> In-Reply-To: <87h6i3lyph.fsf@cbaines.net> Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix@cbaines.net, dev@jpoiret.xyz, ludo@gnu.org, othacehe@gnu.org, rekado@elephly.net, zimon.toutoune@gmail.com, me@tobias.gr, guix-patches@gnu.org Resent-Date: Tue, 20 Feb 2024 19:41:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 69292 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 69292@debbugs.gnu.org Cc: Christopher Baines , Josselin Poiret , Ludovic =?UTF-8?Q?Court=C3=A8s?= , Mathieu Othacehe , Ricardo Wurmus , Simon Tournier , Tobias Geerinckx-Rice X-Debbugs-Original-Xcc: Christopher Baines , Josselin Poiret , Ludovic =?UTF-8?Q?Court=C3=A8s?= , Mathieu Othacehe , Ricardo Wurmus , Simon Tournier , Tobias Geerinckx-Rice Received: via spool by 69292-submit@debbugs.gnu.org id=B69292.17084580049454 (code B ref 69292); Tue, 20 Feb 2024 19:41:01 +0000 Received: (at 69292) by debbugs.gnu.org; 20 Feb 2024 19:40:04 +0000 Received: from localhost ([127.0.0.1]:46619 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rcVyE-0002S8-Np for submit@debbugs.gnu.org; Tue, 20 Feb 2024 14:40:04 -0500 Received: from mira.cbaines.net ([212.71.252.8]:43146) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rcVyC-0002RR-5F for 69292@debbugs.gnu.org; Tue, 20 Feb 2024 14:40:01 -0500 Received: from localhost (unknown [212.132.255.10]) by mira.cbaines.net (Postfix) with ESMTPSA id A98CC27BBE2 for <69292@debbugs.gnu.org>; Tue, 20 Feb 2024 19:39:07 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 1616ca35 for <69292@debbugs.gnu.org>; Tue, 20 Feb 2024 19:39:07 +0000 (UTC) From: Christopher Baines Date: Tue, 20 Feb 2024 19:39:01 +0000 Message-ID: <4b6a268daab5e0b307dff2229d551a47c9fe1ebc.1708457946.git.mail@cbaines.net> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-bounces+larch=yhetil.org@gnu.org X-Migadu-Country: US X-Migadu-Flow: FLOW_IN X-Migadu-Scanner: mx12.migadu.com X-Migadu-Spam-Score: -3.48 X-Spam-Score: -3.48 X-Migadu-Queue-Id: 95554184F6 X-TUID: um9Nr+aBt6R/ While care does need to be taken with making updates or inserts to the ValidPaths table, I think that trying to ensure this within update-or-insert is the wrong approach. Instead, when working with the store database, only one connection should be used to make changes to the database and those changes should happen in transactions that ideally begin immediately. This reverts commit 37545de4a3bf59611c184b31506fe9a16abe4c8b. * .dir-locals.el (scheme-mode): Remove entries for call-with-savepoint and call-with-retrying-savepoint. * guix/store/database.scm (call-with-savepoint, call-with-retrying-savepoint): Remove procedures. (update-or-insert): Remove use of call-with-savepoint. Change-Id: I2f986e8623d8235a90c40d5f219c1292c1ab157b --- .dir-locals.el | 2 -- guix/store/database.scm | 75 +++++++---------------------------------- 2 files changed, 13 insertions(+), 64 deletions(-) diff --git a/.dir-locals.el b/.dir-locals.el index d18e6ba760..f135eb69a5 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -133,8 +133,6 @@ (eval . (put 'call-with-transaction 'scheme-indent-function 1)) (eval . (put 'with-statement 'scheme-indent-function 3)) (eval . (put 'call-with-retrying-transaction 'scheme-indent-function 1)) - (eval . (put 'call-with-savepoint 'scheme-indent-function 1)) - (eval . (put 'call-with-retrying-savepoint 'scheme-indent-function 1)) (eval . (put 'call-with-container 'scheme-indent-function 1)) (eval . (put 'container-excursion 'scheme-indent-function 1)) diff --git a/guix/store/database.scm b/guix/store/database.scm index 2968f13492..3093fd816a 100644 --- a/guix/store/database.scm +++ b/guix/store/database.scm @@ -151,39 +151,11 @@ (define* (call-with-transaction db proc #:key restartable?) (false-if-exception (exec "rollback;")) (apply throw args)))) -(define* (call-with-savepoint db proc - #:optional (savepoint-name "SomeSavepoint")) - "Call PROC after creating a savepoint named SAVEPOINT-NAME. If PROC exits -abnormally, rollback to that savepoint. In all cases, remove the savepoint -prior to returning." - (define (exec sql) - (with-statement db sql stmt - (sqlite-fold cons '() stmt))) - - (dynamic-wind - (lambda () - (exec (string-append "SAVEPOINT " savepoint-name ";"))) - (lambda () - (catch #t - proc - (lambda args - (exec (string-append "ROLLBACK TO " savepoint-name ";")) - (apply throw args)))) - (lambda () - (exec (string-append "RELEASE " savepoint-name ";"))))) - (define* (call-with-retrying-transaction db proc #:key restartable?) (call-with-SQLITE_BUSY-retrying (lambda () (call-with-transaction db proc #:restartable? restartable?)))) -(define* (call-with-retrying-savepoint db proc - #:optional (savepoint-name - "SomeSavepoint")) - (call-with-SQLITE_BUSY-retrying - (lambda () - (call-with-savepoint db proc savepoint-name)))) - (define %default-database-file ;; Default location of the store database. (string-append %store-database-directory "/db.sqlite")) @@ -261,40 +233,19 @@ (define* (update-or-insert db #:key path deriver hash nar-size time) (assert-integer "update-or-insert" positive? #:nar-size nar-size) (assert-integer "update-or-insert" (cut >= <> 0) #:time time) - ;; It's important that querying the path-id and the insert/update operation - ;; take place in the same transaction, as otherwise some other - ;; process/thread/fiber could register the same path between when we check - ;; whether it's already registered and when we register it, resulting in - ;; duplicate paths (which, due to a 'unique' constraint, would cause an - ;; exception to be thrown). With the default journaling mode this will - ;; prevent writes from occurring during that sensitive time, but with WAL - ;; mode it will instead arrange to return SQLITE_BUSY when a write occurs - ;; between the start of a read transaction and its upgrading to a write - ;; transaction (see https://sqlite.org/rescode.html#busy_snapshot). - ;; Experimentally, it seems this SQLITE_BUSY will ignore a busy_timeout and - ;; immediately return (makes sense, since waiting won't change anything). - - ;; Note that when that kind of SQLITE_BUSY error is returned, it will keep - ;; being returned every time we try to upgrade the same outermost - ;; transaction to a write transaction. So when retrying, we have to restart - ;; the *outermost* write transaction. We can't inherently tell whether - ;; we're the outermost write transaction, so we leave the retry-handling to - ;; the caller. - (call-with-savepoint db - (lambda () - (let ((id (path-id db path))) - (if id - (with-statement db update-sql stmt - (sqlite-bind-arguments stmt #:id id - #:deriver deriver - #:hash hash #:size nar-size #:time time) - (sqlite-fold cons '() stmt)) - (with-statement db insert-sql stmt - (sqlite-bind-arguments stmt - #:path path #:deriver deriver - #:hash hash #:size nar-size #:time time) - (sqlite-fold cons '() stmt))) - (last-insert-row-id db))))) + (let ((id (path-id db path))) + (if id + (with-statement db update-sql stmt + (sqlite-bind-arguments stmt #:id id + #:deriver deriver + #:hash hash #:size nar-size #:time time) + (sqlite-fold cons '() stmt)) + (with-statement db insert-sql stmt + (sqlite-bind-arguments stmt + #:path path #:deriver deriver + #:hash hash #:size nar-size #:time time) + (sqlite-fold cons '() stmt))) + (last-insert-row-id db))) (define add-reference-sql "INSERT OR REPLACE INTO Refs (referrer, reference) VALUES (:referrer, :reference);") base-commit: f4af19b037826cad90bbcfe400ad864f028cc7d8 -- 2.41.0