From patchwork Thu Dec 21 20:39:47 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [5,of,7] lfs: narrow the exceptions that trigger a transfer retry
From: Matt Harbison <mharbison72@gmail.com>
X-Patchwork-Id: 26388
Message-Id: <3ffb73f5ab3b12511008.1513888787@Envy>
To: mercurial-devel@mercurial-scm.org
Date: Thu, 21 Dec 2017 15:39:47 -0500

# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1512441664 18000
#      Mon Dec 04 21:41:04 2017 -0500
# Node ID 3ffb73f5ab3b12511008dc8496c913bd639f3f38
# Parent  51a98a20b44618eade6d9227cc8c5473b44450f5
lfs: narrow the exceptions that trigger a transfer retry

The retries were added to workaround TCP RESETs in fb-experimental fc8c131314a9.
I have no idea if that's been debugged yet, but this wide net caught local I/O
errors, bad hostnames and other things that shouldn't be retried.  The next
patch will validate objects as they are uploaded, and there's no need to retry
those errors.

The spec[1] does mention that certain http errors can be retried, including 500.
But let's work through the corruption detection issues first.

[1] https://github.com/git-lfs/git-lfs/blob/master/docs/api/batch.md

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -10,6 +10,7 @@
 import json
 import os
 import re
+import socket
 
 from mercurial.i18n import _
 
@@ -286,7 +287,7 @@
                         self._basictransfer(obj, action, localstore)
                         yield 1, obj.get('oid')
                         break
-                    except Exception as ex:
+                    except socket.error as ex:
                         if retry > 0:
                             if self.ui.verbose:
                                 self.ui.write(
diff --git a/tests/test-lfs-test-server.t b/tests/test-lfs-test-server.t
--- a/tests/test-lfs-test-server.t
+++ b/tests/test-lfs-test-server.t
@@ -146,12 +146,7 @@
   pushing to ../repo1
   searching for changes
   lfs: uploading e659058e26b07b39d2a9c7145b3f99b41f797b6621c8076600e9cb7ee88291f0 (17 bytes)
-  lfs: failed: LfsRemoteError('HTTP error: HTTP Error 500: Internal Server Error (oid=e659058e26b07b39d2a9c7145b3f99b41f797b6621c8076600e9cb7ee88291f0, action=upload)',) (remaining retry 5)
-  lfs: failed: LfsRemoteError('HTTP error: HTTP Error 404: Not Found (oid=e659058e26b07b39d2a9c7145b3f99b41f797b6621c8076600e9cb7ee88291f0, action=upload)',) (remaining retry 4)
-  lfs: failed: LfsRemoteError('HTTP error: HTTP Error 404: Not Found (oid=e659058e26b07b39d2a9c7145b3f99b41f797b6621c8076600e9cb7ee88291f0, action=upload)',) (remaining retry 3)
-  lfs: failed: LfsRemoteError('HTTP error: HTTP Error 404: Not Found (oid=e659058e26b07b39d2a9c7145b3f99b41f797b6621c8076600e9cb7ee88291f0, action=upload)',) (remaining retry 2)
-  lfs: failed: LfsRemoteError('HTTP error: HTTP Error 404: Not Found (oid=e659058e26b07b39d2a9c7145b3f99b41f797b6621c8076600e9cb7ee88291f0, action=upload)',) (remaining retry 1)
-  abort: HTTP error: HTTP Error 404: Not Found (oid=e659058e26b07b39d2a9c7145b3f99b41f797b6621c8076600e9cb7ee88291f0, action=upload)!
+  abort: HTTP error: HTTP Error 500: Internal Server Error (oid=e659058e26b07b39d2a9c7145b3f99b41f797b6621c8076600e9cb7ee88291f0, action=upload)!
   [255]
 
 Check error message when the remote missed a blob: