From b593c39e9d98762574a8dd7ba4e6114d4ab74f75 Mon Sep 17 00:00:00 2001 From: Scott Zawalski Date: Tue, 16 Nov 2010 10:00:07 -0800 Subject: [PATCH 1/2] Add a RevGitWithRetry method to retry repo sync and git push Modify repo sync to just sync the repo we are in, this is what preflight does and is a proven acceptable approach over the complete 'repo sync' approach. BUG=8987 TEST=Ran retry command over dummy directories to be sure it retries as expected. Review URL: http://codereview.chromium.org/5082002 --- prebuilt.py | 52 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/prebuilt.py b/prebuilt.py index a029159ed4..ee9ecf1887 100755 --- a/prebuilt.py +++ b/prebuilt.py @@ -10,6 +10,7 @@ import os import re import sys import tempfile +import time from chromite.lib import cros_build_lib """ @@ -73,6 +74,9 @@ class UnknownBoardFormat(Exception): """Raised when a function finds an unknown board format.""" pass +class GitPushFailed(Exception): + """Raised when a git push failed after retry.""" + def UpdateLocalFile(filename, value, key='PORTAGE_BINHOST'): """Update the key in file with the value passed. @@ -117,21 +121,43 @@ def UpdateLocalFile(filename, value, key='PORTAGE_BINHOST'): new_file_fh.close() -def RevGitFile(filename, value): +def RevGitPushWithRetry(retries=5): + """Repo sync and then push git changes in flight. + + Args: + retries: The number of times to retry before giving up, default: 5 + + Raises: + GitPushFailed if push was unsuccessful after retries + """ + for retry in range(1, retries+1): + try: + cros_build_lib.RunCommand('repo sync .', shell=True) + cros_build_lib.RunCommand('git push', shell=True) + break + except cros_build_lib.RunCommandError: + if retry < retries: + print 'Error pushing changes trying again (%s/%s)' % (retry, retries) + time.sleep(5*retry) + else: + raise GitPushFailed('Failed to push change after %s retries' % retries) + + +def RevGitFile(filename, value, retries=5): """Update and push the git file. - Args: - filename: file to modify that is in a git repo already - key: board or host package type e.g. x86-dogfood - value: string representing the version of the prebuilt that has been - uploaded. + Args: + filename: file to modify that is in a git repo already + value: string representing the version of the prebuilt that has been + uploaded. + retries: The number of times to retry before giving up, default: 5 """ prebuilt_branch = 'prebuilt_branch' old_cwd = os.getcwd() os.chdir(os.path.dirname(filename)) - cros_build_lib.RunCommand('repo sync', shell=True) - cros_build_lib.RunCommand('repo start %s .' % prebuilt_branch, shell=True) + cros_build_lib.RunCommand('repo sync .', shell=True) + cros_build_lib.RunCommand('repo start %s .' % prebuilt_branch, shell=True) git_ssh_config_cmd = ( 'git config url.ssh://git@gitrw.chromium.org:9222.pushinsteadof ' 'http://git.chromium.org/git') @@ -142,8 +168,7 @@ def RevGitFile(filename, value): UpdateLocalFile(filename, value) cros_build_lib.RunCommand('git config push.default tracking', shell=True) cros_build_lib.RunCommand('git commit -am "%s"' % description, shell=True) - cros_build_lib.RunCommand('repo sync', shell=True) - cros_build_lib.RunCommand('git push', shell=True) + RevGitPushWithRetry(retries) finally: cros_build_lib.RunCommand('repo abandon %s .' % prebuilt_branch, shell=True) os.chdir(old_cwd) @@ -381,7 +406,7 @@ def DetermineMakeConfFile(target): def UploadPrebuilt(build_path, upload_location, version, binhost_base_url, - board=None, git_sync=False): + board=None, git_sync=False, git_sync_retries=5): """Upload Host prebuilt files to Google Storage space. Args: @@ -391,6 +416,9 @@ def UploadPrebuilt(build_path, upload_location, version, binhost_base_url, host packages. git_sync: If set, update make.conf of target to reference the latest prebuilt packages genereated here. + git_sync_retries: How many times to retry pushing when updating git files. + This helps avoid failures when multiple bots are modifying the same Repo. + default: 5 """ if not board: @@ -426,7 +454,7 @@ def UploadPrebuilt(build_path, upload_location, version, binhost_base_url, if git_sync: url_value = '%s/%s/' % (binhost_base_url, url_suffix) - RevGitFile(git_file, url_value) + RevGitFile(git_file, url_value, retries=git_sync_retries) def usage(parser, msg): From 8ce0100b03463c8fd9d5f324595c7e75ed04aae0 Mon Sep 17 00:00:00 2001 From: David James Date: Tue, 16 Nov 2010 10:20:08 -0800 Subject: [PATCH 2/2] Retry in parallel_emerge for 5xx error codes. When the server returns a 5xx error code, we should retry. Such errors are often due to flakiness, so retries resolve the issue. BUG=chromium-os:9217 TEST=Tested parallel_emerge with fake URL to verify it retries. Change-Id: Ie5cf003562b5261005be9929624769394b4802a1 Review URL: http://codereview.chromium.org/5056003 --- parallel_emerge | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/parallel_emerge b/parallel_emerge index ab342334c2..c912a115ac 100755 --- a/parallel_emerge +++ b/parallel_emerge @@ -667,9 +667,10 @@ class DepGraphGenerator(object): return {} def retry_urlopen(url, tries=3): - """Open the specified url, retrying if we run into network errors. + """Open the specified url, retrying if we run into temporary errors. - We do not retry for HTTP errors. + We retry for both network errors and 5xx Server Errors. We do not retry + for HTTP errors with a non-5xx code. Args: url: The specified url. @@ -682,12 +683,17 @@ class DepGraphGenerator(object): try: return urllib2.urlopen(url) except urllib2.HTTPError as e: - raise - except urllib2.URLError as e: - if i + 1 == tries: + if i + 1 >= tries or e.code < 500: raise else: - print "Cannot GET %s: %s" % (url, e) + print "Cannot GET %s: %s" % (url, str(e)) + except urllib2.URLError as e: + if i + 1 >= tries: + raise + else: + print "Cannot GET %s: %s" % (url, str(e)) + print "Sleeping for 10 seconds before retrying..." + time.sleep(10) url = os.path.join(binhost_url, "Packages") try: