avoid using temp file size when deciding whether to retry failed transfer

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
This commit is contained in:
Joey Hess 2021-06-25 11:53:28 -04:00
parent 847aa88bcb
commit 51c696679f
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
3 changed files with 5 additions and 14 deletions

View file

@ -205,13 +205,8 @@ runTransfer' ignorelock t afile stalldetection retrydecider transferaction =
, return observeFailure , return observeFailure
) )
getbytescomplete metervar getbytescomplete metervar = liftIO $
| transferDirection t == Upload = maybe 0 fromBytesProcessed <$> readTVarIO metervar
liftIO $ maybe 0 fromBytesProcessed
<$> readTVarIO metervar
| otherwise = do
f <- fromRepo $ gitAnnexTmpObjectLocation (transferKey t)
liftIO $ catchDefaultIO 0 $ getFileSize f
detectStallsAndSuggestConfig :: Maybe StallDetection -> TVar (Maybe BytesProcessed) -> Annex a -> Annex a detectStallsAndSuggestConfig :: Maybe StallDetection -> TVar (Maybe BytesProcessed) -> Annex a -> Annex a
detectStallsAndSuggestConfig Nothing _ a = a detectStallsAndSuggestConfig Nothing _ a = a

View file

@ -133,12 +133,8 @@ performTransfer stalldetection level runannex r t info transferrer = do
ifM (catchBoolIO $ bracket setup cleanup (go bpv)) ifM (catchBoolIO $ bracket setup cleanup (go bpv))
( return (Right ()) ( return (Right ())
, do , do
n <- case transferDirection t of n <- liftIO $ atomically $
Upload -> liftIO $ atomically $ fromBytesProcessed <$> readTVar bpv
fromBytesProcessed <$> readTVar bpv
Download -> do
f <- runannex $ fromRepo $ gitAnnexTmpObjectLocation (transferKey t)
liftIO $ catchDefaultIO 0 $ getFileSize f
return $ Left $ info { bytesComplete = Just n } return $ Left $ info { bytesComplete = Just n }
) )
where where

View file

@ -32,7 +32,7 @@ relaySerializedOutput
-> (SerializedOutputResponse -> m ()) -> (SerializedOutputResponse -> m ())
-- ^ Send response to child process. -- ^ Send response to child process.
-> (Maybe BytesProcessed -> m ()) -> (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. -- progress meter values sent by the process.
-- When a progress meter is stopped, Nothing is sent. -- When a progress meter is stopped, Nothing is sent.
-> (forall a. Annex a -> m a) -> (forall a. Annex a -> m a)