avoid using temp file size when deciding whether to retry failed transfer
authorJoey Hess <joeyh@joeyh.name>
Fri, 25 Jun 2021 15:53:28 +0000 (11:53 -0400)
committerJoey Hess <joeyh@joeyh.name>
Fri, 25 Jun 2021 16:04:23 +0000 (12:04 -0400)
When stall detection is enabled, and a transfer is in progress,
it would display a doubled message:

(transfer already in progress, or unable to take transfer lock) (transfer already in progress, or unable to take transfer lock)

That happened because the forward retry decider had a start size of 0,
and an end size of whatever amount of the object the other process had
downloaded. So it incorrectly thought that the transferrer process had
made progress, when it had in fact immediately given up with that
message.

Instead, use the reported value from the progress meter. If a remote
does not report progress, this will mean it doesn't forward retry, in a
situation where it used to. But most remotes do report progress, and any
remote that does not can be fixed to, by using watchFileSize when
downloading. Also, some remotes might preallocate the temp file (eg
bittorrent), so relying on statting its size at this level to get
progress is dubious.

The same change was made to Annex/Transfer.hs, although only
Annex/TransferrerPool.hs needed to be changed to avoid the duplicate
message.

(An alternate fix would have been to start the retry decider with the
size of the object file before downloading begins, rather than 0.)

Sponsored-by: Brett Eisenberg on Patreon
Annex/Transfer.hs
Annex/TransferrerPool.hs
Messages/Serialized.hs

index 76d93df00f56e9725896b1c764a817539d95253e..e6a6e4c8064418095cc8ae0fc2f0613241316503 100644 (file)
@@ -205,13 +205,8 @@ runTransfer' ignorelock t afile stalldetection retrydecider transferaction =
                                , return observeFailure
                                )
 
-       getbytescomplete metervar
-               | transferDirection t == Upload =
-                       liftIO $ maybe 0 fromBytesProcessed 
-                               <$> readTVarIO metervar
-               | otherwise = do
-                       f <- fromRepo $ gitAnnexTmpObjectLocation (transferKey t)
-                       liftIO $ catchDefaultIO 0 $ getFileSize f
+       getbytescomplete metervar = liftIO $
+               maybe 0 fromBytesProcessed <$> readTVarIO metervar
 
 detectStallsAndSuggestConfig :: Maybe StallDetection -> TVar (Maybe BytesProcessed) -> Annex a -> Annex a
 detectStallsAndSuggestConfig Nothing _ a = a
index 01f479881de23c6238c86d6ece86ed165b9a92e7..513dc1f28e6b49b7181a84b086e453f72158104f 100644 (file)
@@ -133,12 +133,8 @@ performTransfer stalldetection level runannex r t info transferrer = do
        ifM (catchBoolIO $ bracket setup cleanup (go bpv))
                ( return (Right ())
                , do
-                       n <- case transferDirection t of
-                               Upload -> liftIO $ atomically $ 
-                                       fromBytesProcessed <$> readTVar bpv
-                               Download -> do
-                                       f <- runannex $ fromRepo $ gitAnnexTmpObjectLocation (transferKey t)
-                                       liftIO $ catchDefaultIO 0 $ getFileSize f
+                       n <- liftIO $ atomically $
+                               fromBytesProcessed <$> readTVar bpv
                        return $ Left $ info { bytesComplete = Just n }
                )
   where
index bf1d09578a5bb004c89ef7b182f3cdd4f9298289..0f5faddc9fd3ad4c8a138d50256543c9655815c2 100644 (file)
@@ -32,7 +32,7 @@ relaySerializedOutput
        -> (SerializedOutputResponse -> m ())
        -- ^ Send response to child process.
        -> (Maybe BytesProcessed -> m ())
-       -- ^ When a progress meter is running, is updated with
+       -- ^ When a progress meter is running, it is updated with
        -- progress meter values sent by the process.
        -- When a progress meter is stopped, Nothing is sent.
        -> (forall a. Annex a -> m a)