Consolidate subprocess retry logic into gclient_utils.

Add retry for all git operations that speak to a remote.  This
should smooth out issues with the git/gerrit-on-borg service.

Review URL: https://codereview.chromium.org/26234004

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@229219 0039d316-1c4b-4281-b951-d872f2087c98

gclient_scm.py

@@ -281,7 +281,7 @@ class GitWrapper(SCMWrapper):
 
     fetch_cmd = [
       '-c', 'core.deltaBaseCacheLimit=2g', 'fetch', 'origin', '--prune']
-    self._Run(fetch_cmd + quiet, options)
+    self._Run(fetch_cmd + quiet, options, retry=True)
     self._Run(['reset', '--hard', revision] + quiet, options)
     self.UpdateSubmoduleConfig()
     if file_list is not None:
@@ -802,7 +802,7 @@ class GitWrapper(SCMWrapper):
 
         self._Run(cmd + [url, folder],
                   options, git_filter=True, filter_fn=filter_fn,
-                  cwd=self.cache_dir)
+                  cwd=self.cache_dir, retry=True)
       else:
         # For now, assert that host/path/to/repo.git is identical. We may want
         # to relax this restriction in the future to allow for smarter cache
@@ -819,7 +819,8 @@ class GitWrapper(SCMWrapper):
         # Would normally use `git remote update`, but it doesn't support
         # --progress, so use fetch instead.
         self._Run(['fetch'] + v + ['--multiple', '--progress', '--all'],
-                  options, git_filter=True, filter_fn=filter_fn, cwd=folder)
+                  options, git_filter=True, filter_fn=filter_fn, cwd=folder,
+                  retry=True)
 
       # If the clone has an object dependency on the existing repo, break it
       # with repack and remove the linkage.
@@ -858,16 +859,8 @@ class GitWrapper(SCMWrapper):
         dir=parent_dir)
     try:
       clone_cmd.append(tmp_dir)
-      for i in xrange(3):
-        try:
-          self._Run(clone_cmd, options, cwd=self._root_dir, git_filter=True)
-          break
-        except subprocess2.CalledProcessError as e:
-          gclient_utils.rmtree(os.path.join(tmp_dir, '.git'))
-          if e.returncode != 128 or i == 2:
-            raise
-          print(str(e))
-          print('Retrying...')
+      self._Run(clone_cmd, options, cwd=self._root_dir, git_filter=True,
+                retry=True)
       gclient_utils.safe_makedirs(self.checkout_path)
       gclient_utils.safe_rename(os.path.join(tmp_dir, '.git'),
                                 os.path.join(self.checkout_path, '.git'))
@@ -1034,24 +1027,15 @@ class GitWrapper(SCMWrapper):
   def _UpdateBranchHeads(self, options, fetch=False):
     """Adds, and optionally fetches, "branch-heads" refspecs if requested."""
     if hasattr(options, 'with_branch_heads') and options.with_branch_heads:
-      backoff_time = 5
-      for _ in range(3):
-        try:
-          config_cmd = ['config', 'remote.origin.fetch',
-                        '+refs/branch-heads/*:refs/remotes/branch-heads/*',
-                        '^\\+refs/branch-heads/\\*:.*$']
-          self._Run(config_cmd, options)
-          if fetch:
-            fetch_cmd = ['-c', 'core.deltaBaseCacheLimit=2g', 'fetch', 'origin']
-            if options.verbose:
-              fetch_cmd.append('--verbose')
-            self._Run(fetch_cmd, options)
-          break
-        except subprocess2.CalledProcessError, e:
-          print(str(e))
-          print('Retrying in %.1f seconds...' % backoff_time)
-          time.sleep(backoff_time)
-          backoff_time *= 1.3
+      config_cmd = ['config', 'remote.origin.fetch',
+                    '+refs/branch-heads/*:refs/remotes/branch-heads/*',
+                    '^\\+refs/branch-heads/\\*:.*$']
+      self._Run(config_cmd, options)
+      if fetch:
+        fetch_cmd = ['-c', 'core.deltaBaseCacheLimit=2g', 'fetch', 'origin']
+        if options.verbose:
+          fetch_cmd.append('--verbose')
+        self._Run(fetch_cmd, options, retry=True)
 
   def _Run(self, args, _options, git_filter=False, **kwargs):
     kwargs.setdefault('cwd', self.checkout_path)

gclient_utils.py

@@ -21,6 +21,10 @@ import urlparse
 import subprocess2
 
 
+RETRY_MAX = 3
+RETRY_INITIAL_SLEEP = 0.5
+
+
 class Error(Exception):
   """gclient exception class."""
   def __init__(self, msg, *args, **kwargs):
@@ -407,7 +411,7 @@ class GClientChildren(object):
 
 def CheckCallAndFilter(args, stdout=None, filter_fn=None,
                        print_stdout=None, call_filter_on_first_line=False,
-                       **kwargs):
+                       retry=False, **kwargs):
   """Runs a command and calls back a filter function if needed.
 
   Accepts all subprocess2.Popen() parameters plus:
@@ -416,62 +420,74 @@ def CheckCallAndFilter(args, stdout=None, filter_fn=None,
                of the subprocess2's output. Each line has the trailing newline
                character trimmed.
     stdout: Can be any bufferable output.
+    retry: If the process exits non-zero, sleep for a brief interval and try
+           again, up to RETRY_MAX times.
 
   stderr is always redirected to stdout.
   """
   assert print_stdout or filter_fn
   stdout = stdout or sys.stdout
   filter_fn = filter_fn or (lambda x: None)
-  kid = subprocess2.Popen(
-      args, bufsize=0, stdout=subprocess2.PIPE, stderr=subprocess2.STDOUT,
-      **kwargs)
 
-  GClientChildren.add(kid)
+  sleep_interval = RETRY_INITIAL_SLEEP
+  run_cwd = kwargs.get('cwd', os.getcwd())
+  for _ in xrange(RETRY_MAX + 1):
+    kid = subprocess2.Popen(
+        args, bufsize=0, stdout=subprocess2.PIPE, stderr=subprocess2.STDOUT,
+        **kwargs)
 
-  # Do a flush of stdout before we begin reading from the subprocess2's stdout
-  stdout.flush()
+    GClientChildren.add(kid)
 
-  # Also, we need to forward stdout to prevent weird re-ordering of output.
-  # This has to be done on a per byte basis to make sure it is not buffered:
-  # normally buffering is done for each line, but if svn requests input, no
-  # end-of-line character is output after the prompt and it would not show up.
-  try:
-    in_byte = kid.stdout.read(1)
-    if in_byte:
-      if call_filter_on_first_line:
-        filter_fn(None)
-      in_line = ''
-      while in_byte:
-        if in_byte != '\r':
-          if print_stdout:
-            stdout.write(in_byte)
-          if in_byte != '\n':
-            in_line += in_byte
+    # Do a flush of stdout before we begin reading from the subprocess2's stdout
+    stdout.flush()
+
+    # Also, we need to forward stdout to prevent weird re-ordering of output.
+    # This has to be done on a per byte basis to make sure it is not buffered:
+    # normally buffering is done for each line, but if svn requests input, no
+    # end-of-line character is output after the prompt and it would not show up.
+    try:
+      in_byte = kid.stdout.read(1)
+      if in_byte:
+        if call_filter_on_first_line:
+          filter_fn(None)
+        in_line = ''
+        while in_byte:
+          if in_byte != '\r':
+            if print_stdout:
+              stdout.write(in_byte)
+            if in_byte != '\n':
+              in_line += in_byte
+            else:
+              filter_fn(in_line)
+              in_line = ''
           else:
             filter_fn(in_line)
             in_line = ''
-        else:
+          in_byte = kid.stdout.read(1)
+        # Flush the rest of buffered output. This is only an issue with
+        # stdout/stderr not ending with a \n.
+        if len(in_line):
           filter_fn(in_line)
-          in_line = ''
-        in_byte = kid.stdout.read(1)
-      # Flush the rest of buffered output. This is only an issue with
-      # stdout/stderr not ending with a \n.
-      if len(in_line):
-        filter_fn(in_line)
-    rv = kid.wait()
-
-    # Don't put this in a 'finally,' since the child may still run if we get an
-    # exception.
-    GClientChildren.remove(kid)
-
-  except KeyboardInterrupt:
-    print >> sys.stderr, 'Failed while running "%s"' % ' '.join(args)
-    raise
-
-  if rv:
-    raise subprocess2.CalledProcessError(
-        rv, args, kwargs.get('cwd', None), None, None)
-  return 0
+      rv = kid.wait()
+
+      # Don't put this in a 'finally,' since the child may still run if we get
+      # an exception.
+      GClientChildren.remove(kid)
+
+    except KeyboardInterrupt:
+      print >> sys.stderr, 'Failed while running "%s"' % ' '.join(args)
+      raise
+
+    if rv == 0:
+      return 0
+    if not retry:
+      break
+    print ("WARNING: subprocess '%s' in %s failed; will retry after a short "
+           'nap...' % (' '.join('"%s"' % x for x in args), run_cwd))
+    sys.sleep(sleep_interval)
+    sleep_interval *= 2
+  raise subprocess2.CalledProcessError(
+      rv, args, kwargs.get('cwd', None), None, None)
 
 
 def FindGclientRoot(from_dir, filename='.gclient'):

tests/gclient_utils_test.py

@@ -34,8 +34,8 @@ class GclientUtilsUnittest(GclientUtilBase):
         'GetGClientRootAndEntries', 'GetEditor', 'IsDateRevision',
         'MakeDateRevision', 'MakeFileAutoFlush', 'MakeFileAnnotated',
         'PathDifference', 'ParseCodereviewSettingsContent', 'NumLocalCpus',
-        'PrintableObject', 'RunEditor', 'GCLIENT_CHILDREN',
-        'GCLIENT_CHILDREN_LOCK', 'GClientChildren',
+        'PrintableObject', 'RETRY_INITIAL_SLEEP', 'RETRY_MAX', 'RunEditor',
+        'GCLIENT_CHILDREN', 'GCLIENT_CHILDREN_LOCK', 'GClientChildren',
         'SplitUrlRevision', 'SyntaxErrorToError', 'UpgradeToHttps', 'Wrapper',
         'WorkItem', 'codecs', 'lockedmethod', 'logging', 'os', 'pipes', 'Queue',
         're', 'rmtree', 'safe_makedirs', 'safe_rename', 'stat', 'subprocess', 
@@ -51,8 +51,9 @@ class CheckCallAndFilterTestCase(GclientUtilBase):
     def __init__(self, test_string):
       self.stdout = StringIO.StringIO(test_string)
       self.pid = 9284
+    # pylint: disable=R0201
     def wait(self):
-      pass
+      return 0
 
   def _inner(self, args, test_string):
     cwd = 'bleh'
@@ -68,6 +69,7 @@ class CheckCallAndFilterTestCase(GclientUtilBase):
         stderr=subprocess2.STDOUT,
         bufsize=0).AndReturn(self.ProcessIdMock(test_string))
 
+    os.getcwd()
     self.mox.ReplayAll()
     compiled_pattern = gclient_utils.re.compile(r'a(.*)b')
     line_list = []