04/10: database: register-items: reduce transaction scope.

[guix-commits]

civodul pushed a commit to branch master
in repository guix.

commit 2932591b8aeec89732c8f8faa0d3f8ef900e68d2
Author: Christopher Baines <mail@cbaines.net>
AuthorDate: Tue Jun 23 17:36:49 2020 +0100

    database: register-items: reduce transaction scope.
    
    It was made transactional in a4678c6ba18d8dbd79d931f80426eebf61be7ebe, with
    the reasoning to prevent broken intermediate states from being visible. I
    think this means something like an entry being in ValidPaths, but the Refs 
not
    being inserted.
    
    Using a transaction for this makes sense, but I think using one single
    transaction for the whole register-items call is unnecessary to avoid broken
    states from being visible, and could block other writes to the store 
database
    while register-items is running. Because the deduplication and resetting
    timestamps happens within the transaction as well, even though these things
    don't involve the database, writes to the database will still be blocked 
while
    this is happening.
    
    To reduce the potential for register-items to block other writers to the
    database for extended periods, this commit moves the transaction to just 
wrap
    the call to sqlite-register. This is the one place where writes occur, so 
that
    should prevent the broken intermediate states issue above. The one 
difference
    this will make is some of the registered items will be visible to other
    connections while others may be still being added. I think this is OK, as 
it's
    equivalent to just registering different items.
    
    * guix/store/database.scm (register-items): Reduce transaction scope.
    
    Signed-off-by: Ludovic Courtès <ludo@gnu.org>
---
 guix/store/database.scm | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/guix/store/database.scm b/guix/store/database.scm
index e39a160..2ea63b1 100644
--- a/guix/store/database.scm
+++ b/guix/store/database.scm
@@ -457,24 +457,25 @@ typically by adding them as temp-roots."
       (when reset-timestamps?
         (reset-timestamps real-file-name))
       (let-values (((hash nar-size) (nar-sha256 real-file-name)))
-        (sqlite-register db #:path to-register
-                         #:references (store-info-references item)
-                         #:deriver (store-info-deriver item)
-                         #:hash (string-append "sha256:"
-                                               (bytevector->base16-string 
hash))
-                         #:nar-size nar-size
-                         #:time registration-time)
+        (call-with-retrying-transaction db
+          (lambda ()
+            (sqlite-register db #:path to-register
+                             #:references (store-info-references item)
+                             #:deriver (store-info-deriver item)
+                             #:hash (string-append
+                                     "sha256:"
+                                     (bytevector->base16-string hash))
+                             #:nar-size nar-size
+                             #:time registration-time)))
         (when deduplicate?
           (deduplicate real-file-name hash #:store store-dir)))))
 
-  (call-with-retrying-transaction db
-      (lambda ()
-        (let* ((prefix   (format #f "registering ~a items" (length items)))
-               (progress (progress-reporter/bar (length items)
-                                                prefix log-port)))
-          (call-with-progress-reporter progress
-            (lambda (report)
-              (for-each (lambda (item)
-                          (register db item)
-                          (report))
-                        items)))))))
+  (let* ((prefix   (format #f "registering ~a items" (length items)))
+         (progress (progress-reporter/bar (length items)
+                                          prefix log-port)))
+    (call-with-progress-reporter progress
+      (lambda (report)
+        (for-each (lambda (item)
+                    (register db item)
+                    (report))
+                  items)))))