From 51c696679f0a39c5044f9e5910e4f1a5aa437e50 Mon Sep 17 00:00:00 2001
From: Joey Hess <joeyh@joeyh.name>
Date: Fri, 25 Jun 2021 11:53:28 -0400
Subject: [PATCH] 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
---
 Annex/Transfer.hs        | 9 ++-------
 Annex/TransferrerPool.hs | 8 ++------
 Messages/Serialized.hs   | 2 +-
 3 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/Annex/Transfer.hs b/Annex/Transfer.hs
index 76d93df00f..e6a6e4c806 100644
--- a/Annex/Transfer.hs
+++ b/Annex/Transfer.hs
@@ -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
diff --git a/Annex/TransferrerPool.hs b/Annex/TransferrerPool.hs
index 01f479881d..513dc1f28e 100644
--- a/Annex/TransferrerPool.hs
+++ b/Annex/TransferrerPool.hs
@@ -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
diff --git a/Messages/Serialized.hs b/Messages/Serialized.hs
index bf1d09578a..0f5faddc9f 100644
--- a/Messages/Serialized.hs
+++ b/Messages/Serialized.hs
@@ -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)