From 7eb3742e4b76d1d7a487c2c53bf25cda4ee5df43 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 11 Aug 2021 14:42:49 -0400 Subject: [PATCH] incremental verify for chunked remotes Simply feed each chunk in turn to the incremental verifier. When resuming an interrupted retrieve, it does not do incremental verification. That would need to read the file, up to the resume point, and feed it to the incremental verifier. That seems easy to get wrong. Also it would mean extra work done before the transfer can start. Which would complicate displaying progress, and would perhaps not appear to the user as if it was resuming from where it left off. Instead, in that situation, return UnVerified, and let the verification be done in a separate pass. Granted, Annex.CopyFile does manage all that, but it's not complicated by dealing with chunks too. Sponsored-by: Dartmouth College's DANDI project --- CHANGELOG | 3 +- Remote/Helper/Chunked.hs | 64 +++++++++++-------- ..._a19823409648828b8b9f7a0f2fa7d935._comment | 10 +++ 3 files changed, 47 insertions(+), 30 deletions(-) create mode 100644 doc/todo/OPT__58_____34__bundle__34___get_+_check___40__of_checksum__41___in_a_single_operation/comment_13_a19823409648828b8b9f7a0f2fa7d935._comment diff --git a/CHANGELOG b/CHANGELOG index c3c4b4b1ce..60e29e662f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -10,8 +10,7 @@ git-annex (8.20210804) UNRELEASED; urgency=medium git-annex's own progress display. * Several special remotes verify content while it is being retrieved, avoiding a separate checksum pass. They are: S3, bup, ddar, - and gcrypt (with a local repository). This optimisation is not yet - available when chunks are used. + and gcrypt (with a local repository). -- Joey Hess Tue, 03 Aug 2021 12:22:45 -0400 diff --git a/Remote/Helper/Chunked.hs b/Remote/Helper/Chunked.hs index 4e17672dd0..56271df5ac 100644 --- a/Remote/Helper/Chunked.hs +++ b/Remote/Helper/Chunked.hs @@ -269,33 +269,28 @@ retrieveChunks retriever u vc chunkconfig encryptor basek dest basep enc encc -- that are likely not there. iv <- startVerifyKeyContentIncrementally vc basek tryNonAsync (getunchunked iv) >>= \case - Right Nothing -> return UnVerified - Right (Just iv') -> - ifM (liftIO $ finalizeIncremental iv') - ( return Verified - , return UnVerified - ) - Left e -> do - go (Just e) =<< chunkKeysOnly u chunkconfig basek - return UnVerified - | otherwise = do - go Nothing =<< chunkKeys u chunkconfig basek - return UnVerified + Right r -> finalize r + Left e -> go (Just e) + =<< chunkKeysOnly u chunkconfig basek + | otherwise = go Nothing + =<< chunkKeys u chunkconfig basek where go pe cks = do let ls = map chunkKeyList cks currsize <- liftIO $ catchMaybeIO $ getFileSize (toRawFilePath dest) let ls' = maybe ls (setupResume ls) currsize if any null ls' - then noop -- dest is already complete - else firstavail pe currsize ls' + then finalize Nothing -- dest is already complete + else finalize =<< firstavail pe currsize ls' firstavail Nothing _ [] = giveup "unable to determine the chunks to use for this remote" firstavail (Just e) _ [] = throwM e firstavail pe currsize ([]:ls) = firstavail pe currsize ls firstavail _ currsize ((k:ks):ls) - | k == basek = void (getunchunked Nothing) - `catchNonAsync` (\e -> firstavail (Just e) currsize ls) + | k == basek = do + iv <- startVerifyKeyContentIncrementally vc basek + getunchunked iv + `catchNonAsync` (\e -> firstavail (Just e) currsize ls) | otherwise = do let offset = resumeOffset currsize k let p = maybe basep @@ -303,36 +298,42 @@ retrieveChunks retriever u vc chunkconfig encryptor basek dest basep enc encc offset v <- tryNonAsync $ retriever (encryptor k) p $ \content -> - bracketIO (maybe opennew openresume offset) hClose $ \h -> do - void $ retrieved Nothing (Just h) p content + bracket (maybe opennew openresume offset) (liftIO . hClose . fst) $ \(h, iv) -> do + iv' <- retrieved iv (Just h) p content let sz = toBytesProcessed $ fromMaybe 0 $ fromKey keyChunkSize k - getrest p h sz sz ks + getrest p h iv' sz sz ks case v of Left e | null ls -> throwM e | otherwise -> firstavail (Just e) currsize ls Right r -> return r - getrest _ _ _ _ [] = noop - getrest p h sz bytesprocessed (k:ks) = do + getrest _ _ iv _ _ [] = return iv + getrest p h iv sz bytesprocessed (k:ks) = do let p' = offsetMeterUpdate p bytesprocessed liftIO $ p' zeroBytesProcessed - retriever (encryptor k) p' $ - void . retrieved Nothing (Just h) p' - getrest p h sz (addBytesProcessed bytesprocessed sz) ks + iv' <- retriever (encryptor k) p' $ + retrieved iv (Just h) p' + getrest p h iv' sz (addBytesProcessed bytesprocessed sz) ks getunchunked iv = retriever (encryptor basek) basep $ retrieved iv Nothing basep - opennew = openBinaryFile dest WriteMode + opennew = do + iv <- startVerifyKeyContentIncrementally vc basek + h <- liftIO $ openBinaryFile dest WriteMode + return (h, iv) -- Open the file and seek to the start point in order to resume. openresume startpoint = do -- ReadWriteMode allows seeking; AppendMode does not. - h <- openBinaryFile dest ReadWriteMode - hSeek h AbsoluteSeek startpoint - return h + h <- liftIO $ openBinaryFile dest ReadWriteMode + liftIO $ hSeek h AbsoluteSeek startpoint + -- No incremental verification when resuming, since that + -- would need to read up to the startpoint. + let iv = Nothing + return (h, iv) {- Progress meter updating is a bit tricky: If the Retriever - populates a file, it is responsible for updating progress @@ -349,6 +350,13 @@ retrieveChunks retriever u vc chunkconfig encryptor basek dest basep enc encc p' | isByteContent content = Just p | otherwise = Nothing + + finalize Nothing = return UnVerified + finalize (Just iv) = + ifM (liftIO $ finalizeIncremental iv) + ( return Verified + , return UnVerified + ) {- Writes retrieved file content to the provided Handle, decrypting it - first if necessary. diff --git a/doc/todo/OPT__58_____34__bundle__34___get_+_check___40__of_checksum__41___in_a_single_operation/comment_13_a19823409648828b8b9f7a0f2fa7d935._comment b/doc/todo/OPT__58_____34__bundle__34___get_+_check___40__of_checksum__41___in_a_single_operation/comment_13_a19823409648828b8b9f7a0f2fa7d935._comment new file mode 100644 index 0000000000..d206602758 --- /dev/null +++ b/doc/todo/OPT__58_____34__bundle__34___get_+_check___40__of_checksum__41___in_a_single_operation/comment_13_a19823409648828b8b9f7a0f2fa7d935._comment @@ -0,0 +1,10 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 13""" + date="2021-08-11T18:16:41Z" + content=""" +Some special remotes now support incremental update. So far, limited to +ones that use the byteRetriever interface. +Others, that use fileRetriever, including external special remotes, still +need work. +"""]] -- 2.30.2