From 12f5848e79b0ede95babebea240264b32e39812c Mon Sep 17 00:00:00 2001
From: Caleb Ristvedt <caleb.ristvedt@cune.org>
Date: Sat, 8 Aug 2020 11:25:57 -0500
Subject: [PATCH] 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. Also modified to support
canonicalizing symbolic links, though it is the caller's responsibility to
ensure that the system supports hardlinking to a symbolic link (on Linux it
does).
(deduplicate): modified to use canonicalize-with-link.
---
guix/store/deduplication.scm | 125 ++++++++++++++++++++---------------
1 file changed, 70 insertions(+), 55 deletions(-)
@@ -115,26 +115,63 @@ 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.
Note: TARGET, TO-REPLACE, and SWAP-DIRECTORY must be on the same file system."
- (define temp-link
+ (define (file-or-symlink-exists? filename)
+ ;; Like 'file-exists?', but doesn't follow symlinks.
(catch 'system-error
(lambda ()
- (get-temp-link target swap-directory))
+ (lstat filename)
+ #t)
(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
- ;; maximum number of links.
- (if (memv (system-error-errno args) `(,ENOSPC ,EMLINK))
+ (if (= (system-error-errno args) ENOENT)
#f
(apply throw args)))))
+ (define temp-link
+ (let retry ()
+ (if (file-or-symlink-exists? target)
+ (catch 'system-error
+ (lambda ()
+ (get-temp-link target swap-directory))
+ (lambda args
+ (let ((errno (system-error-errno args)))
+ (cond
+ ((= 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)))
+ ((memv errno `(,ENOSPC ,EMLINK))
+ ;; We get ENOSPC when we can't fit an additional entry
+ ;; in SWAP-DIRECTORY. If it's EMLINK, then TARGET has
+ ;; reached its maximum number of links. In both cases,
+ ;; skip deduplication.
+ #f)
+ (else (apply throw args))))))
+ (catch 'system-error
+ (lambda ()
+ (link to-replace target)
+ #f)
+ (lambda args
+ (cond
+ ((= (system-error-errno args) EEXIST)
+ (retry))
+ ((memv (system-error-errno args) `(,ENOSPC ,EMLINK))
+ #f)
+ (else (apply throw args))))))))
+
;; If we couldn't create TEMP-LINK, that's OK: just don't do the
;; replacement, which means TO-REPLACE won't be deduplicated.
(when temp-link
@@ -155,49 +192,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