From b992a3aaac7e3b30222e0bf1df09093f18e25e6a Mon Sep 17 00:00:00 2001 From: Caleb Ristvedt Date: Sat, 8 Aug 2020 11:25:57 -0500 Subject: [PATCH 3/6] deduplication: retry on ENOENT. It's possible for the garbage collector to remove the "canonical" link after it's been detected as existing by 'deduplicate'. This would cause an ENOENT error when replace-with-link attempts to create the temporary link. This changes it so that it will properly handle that by retrying. * guix/store/deduplication.scm (replace-with-link): renamed to canonicalize-with-link, now also handles the case where the target link doesn't exist yet, and retries on ENOENT. (deduplicate): modified to use canonicalize-with-link. --- guix/store/deduplication.scm | 103 ++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 50 deletions(-) diff --git a/guix/store/deduplication.scm b/guix/store/deduplication.scm index 0655ceb890..035a4218fb 100644 --- a/guix/store/deduplication.scm +++ b/guix/store/deduplication.scm @@ -115,9 +115,9 @@ store." ;; things link to this" (EMLINK), "this link already exists" (EEXIST), and ;; "can't fit more stuff in this directory" (ENOSPC). -(define* (replace-with-link target to-replace - #:key (swap-directory (dirname target)) - (store (%store-directory))) +(define* (canonicalize-with-link target to-replace + #:key (swap-directory (dirname target)) + (store (%store-directory))) "Atomically replace the file TO-REPLACE with a link to TARGET. Use SWAP-DIRECTORY as the directory to store temporary hard links. Upon ENOSPC and EMLINK, TO-REPLACE is left unchanged. @@ -126,7 +126,32 @@ Note: TARGET, TO-REPLACE, and SWAP-DIRECTORY must be on the same file system." (define temp-link (catch 'system-error (lambda () - (get-temp-link target swap-directory)) + (let retry () + (if (file-exists? target) + (catch 'system-error + (lambda () + (get-temp-link target swap-directory)) + (lambda args + (let ((errno (system-error-errno args))) + (if (= errno ENOENT) + ;; either SWAP-DIRECTORY has missing directory + ;; components or TARGET was deleted - this is a + ;; fundamental ambiguity to the errno produced by + ;; link() + (if (file-exists? swap-directory) + ;; we must assume link failed because target doesn't + ;; exist, so create it. + (retry) + (apply throw args)) + (apply throw args))))) + (catch 'system-error + (lambda () + (link to-replace target) + #f) + (lambda args + (if (= (system-error-errno args) EEXIST) + (retry) + (apply throw args))))))) (lambda args ;; We get ENOSPC when we can't fit an additional entry in ;; SWAP-DIRECTORY. If it's EMLINK, then TARGET has reached its @@ -155,49 +180,27 @@ under STORE." (define links-directory (string-append store "/.links")) - (mkdir-p links-directory) - (let loop ((path path) - (type (stat:type (lstat path))) - (hash hash)) - (if (eq? 'directory type) - ;; Can't hardlink directories, so hardlink their atoms. - (for-each (match-lambda - ((file . properties) - (unless (member file '("." "..")) - (let* ((file (string-append path "/" file)) - (type (match (assoc-ref properties 'type) - ((or 'unknown #f) - (stat:type (lstat file))) - (type type)))) - (loop file type - (and (not (eq? 'directory type)) - (nar-sha256 file))))))) - (scandir* path)) - (let ((link-file (string-append links-directory "/" - (bytevector->nix-base32-string hash)))) - (if (file-exists? link-file) - (replace-with-link link-file path - #:swap-directory links-directory - #:store store) - (catch 'system-error - (lambda () - (link path link-file)) - (lambda args - (let ((errno (system-error-errno args))) - (cond ((= errno EEXIST) - ;; Someone else put an entry for PATH in - ;; LINKS-DIRECTORY before we could. Let's use it. - (replace-with-link path link-file - #:swap-directory - links-directory - #:store store)) - ((= errno ENOSPC) - ;; There's not enough room in the directory index for - ;; more entries in .links, but that's fine: we can - ;; just stop. - #f) - ((= errno EMLINK) - ;; PATH has reached the maximum number of links, but - ;; that's OK: we just can't deduplicate it more. - #f) - (else (apply throw args))))))))))) + (mkdir-p links-directory) + (let loop ((path path) + (type (stat:type (lstat path))) + (hash hash)) + (if (eq? 'directory type) + ;; Can't hardlink directories, so hardlink their atoms. + (for-each (match-lambda + ((file . properties) + (unless (member file '("." "..")) + (let* ((file (string-append path "/" file)) + (type (match (assoc-ref properties 'type) + ((or 'unknown #f) + (stat:type (lstat file))) + (type type)))) + (loop file type + (and (not (eq? 'directory type)) + (nar-sha256 file))))))) + (scandir* path)) + (let ((link-file (string-append links-directory "/" + (bytevector->nix-base32-string + hash)))) + (canonicalize-with-link link-file path + #:swap-directory links-directory + #:store store))))) -- 2.28.0