fsck: avoid redundant checksum when transfer is Verified

When downloading content from a remote, if the content is able to be
verified during the transfer, skip checksumming it a second time.

Note that in this case, the fsck output does not include "(checksum)"
which it does when the checksumming is done separately from the download.

This commit was sponsored by Brock Spratlen on Patreon.

diff --git a/CHANGELOG b/CHANGELOG
index e87377981744da43030f6bc039c94e53a860314c..2887c033c99659dbfe663c02879e31b9743089d1 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -7,6 +7,8 @@ git-annex (8.20210331) UNRELEASED; urgency=medium
   * diffdriver: Support unlocked files.
   * forget: Preserve currently exported trees, avoiding problems with
     exporttree remotes in some unusual circumstances.
+  * fsck: When downloading content from a remote, if the content is able
+    to be verified during the transfer, skip checksumming it a second time.
 
  -- Joey Hess <id@joeyh.name>  Thu, 01 Apr 2021 12:17:26 -0400
 

diff --git a/Command/Fsck.hs b/Command/Fsck.hs
index 30f5db2638e698ec0bb7a36c9e31c20ba17cff2c..9a2b2407b57b90d0f99a07be6bd36664afb0c115 100644 (file)
--- a/Command/Fsck.hs
+++ b/Command/Fsck.hs
@@ -156,17 +156,20 @@ performRemote key afile backend numcopies remote =
        dispatch (Right True) = withtmp $ \tmpfile ->
                getfile tmpfile >>= \case
                        Nothing -> go True Nothing
-                       Just True -> go True (Just tmpfile)
-                       Just False -> do
+                       Just (Right verification) -> go True (Just (tmpfile, verification))
+                       Just (Left _) -> do
                                warning "failed to download file from remote"
                                void $ go True Nothing
                                return False
        dispatch (Right False) = go False Nothing
-       go present localcopy = check
+       go present lv = check
                [ verifyLocationLogRemote key ai remote present
                , verifyRequiredContent key ai
-               , withLocalCopy localcopy $ checkKeySizeRemote key remote ai
-               , withLocalCopy localcopy $ checkBackendRemote backend key remote ai
+               , withLocalCopy (fmap fst lv) $ checkKeySizeRemote key remote ai
+               , case fmap snd lv of
+                       Just Verified -> return True
+                       _ -> withLocalCopy (fmap fst lv) $
+                               checkBackendRemote backend key remote ai
                , checkKeyNumCopies key afile numcopies
                ]
        ai = mkActionItem (key, afile)
@@ -185,13 +188,13 @@ performRemote key afile backend numcopies remote =
                cleanup `after` a tmp
        getfile tmp = ifM (checkDiskSpace (Just (P.takeDirectory tmp)) key 0 True)
                ( ifM (getcheap tmp)
-                       ( return (Just True)
+                       ( return (Just (Right UnVerified))
                        , ifM (Annex.getState Annex.fast)
                                ( return Nothing
-                               , Just . isRight <$> tryNonAsync (getfile' tmp)
+                               , Just <$> tryNonAsync (getfile' tmp)
                                )
                        )
-               , return (Just False)
+               , return Nothing
                )
        getfile' tmp = Remote.retrieveKeyFile remote key (AssociatedFile Nothing) (fromRawFilePath tmp) dummymeter
        dummymeter _ = noop

diff --git a/doc/todo/Fsck_remote_files_in-flight/comment_1_1a70ae7c9821d664ddf72fd4c431be29._comment b/doc/todo/Fsck_remote_files_in-flight/comment_1_1a70ae7c9821d664ddf72fd4c431be29._comment
new file mode 100644 (file)
index 0000000..7c6f486
--- /dev/null
+++ b/doc/todo/Fsck_remote_files_in-flight/comment_1_1a70ae7c9821d664ddf72fd4c431be29._comment
@@ -0,0 +1,28 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2021-04-14T17:07:50Z"
+ content="""
+Only some remotes support checksums in-flight; this recently includes
+downloads from other git-annex repositories over ssh. Progress
+on that front is being tracked at
+<https://git-annex.branchable.com/todo/OPT__58_____34__bundle__34___get_+_check___40__of_checksum__41___in_a_single_operation/>
+Most special remotes can't yet, but that should change eventually
+for at least some of them.
+
+I've made fsck notice when content was able to be verified as part of a
+transfer, and avoid a redundant checksum of them.
+
+What I've not done, and don't think I will be able to, is make the file
+not be written to disk by fsck in that case. Since the `retrieveKeyFile`
+interface is explicitly about writing to a file on disk, it would take ether
+a whole separate interface being implemented for all remotes that avoids
+writing to the file when they can checksum in flight, or it would need
+some change to the `retrieveKeyFile` interface to do the same.
+
+Neither seems worth the complication to implement just to reduce disk IO in
+this particular case. And it seems likely that, for files that fit in
+memory, it never actually reaches disk before it's deleted. Also if this is
+a concern for you, you can I guess avoid fscking remotes too frequently or
+use a less fragile medium?
+"""]]