From 496a863df20fbd1dcf856e8ab5d0f01a0dfb6a2d Mon Sep 17 00:00:00 2001 From: David-Sarah Hopwood Date: Thu, 21 Feb 2013 22:14:27 +0000 Subject: [PATCH] cloud_common.py: generalize ContainerRetryMixin to allow the container class to specify what to retry. Signed-off-by: David-Sarah Hopwood --- .../storage/backends/cloud/cloud_common.py | 34 +++++++++++++------ src/allmydata/test/test_storage.py | 2 +- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/allmydata/storage/backends/cloud/cloud_common.py b/src/allmydata/storage/backends/cloud/cloud_common.py index 00004c28..3d38f230 100644 --- a/src/allmydata/storage/backends/cloud/cloud_common.py +++ b/src/allmydata/storage/backends/cloud/cloud_common.py @@ -323,16 +323,28 @@ class ContainerListing(object): }) -BACKOFF_SECONDS_FOR_5XX = (0, 2, 10) +BACKOFF_SECONDS_BEFORE_RETRY = (0, 2, 10) class ContainerRetryMixin: """ I provide a helper method for performing an operation on a cloud container that will retry up to - len(BACKOFF_SECONDS_FOR_5XX) times (not including the initial try). If the initial try fails, a + len(BACKOFF_SECONDS_FOR_RETRY) times (not including the initial try). If the initial try fails, a single incident will be triggered after the operation has succeeded or failed. + + Subclasses should define: + ServiceError: + The error class to trap (CloudServiceError or similar). + + and can override: + _react_to_error(self, response_code): + Returns True if the error should be retried. May perform side effects before the retry. """ + def _react_to_error(self, response_code): + # The default policy is to retry on 5xx errors. + return response_code >= 500 and response_code < 600 + def _do_request(self, description, operation, *args, **kwargs): d = defer.maybeDeferred(operation, *args, **kwargs) def _retry(f): @@ -369,20 +381,22 @@ class ContainerRetryMixin: if first_err_and_tb is None: first_err_and_tb = (err, tb) - if trynum > len(BACKOFF_SECONDS_FOR_5XX): + if trynum > len(BACKOFF_SECONDS_BEFORE_RETRY): # If we run out of tries, raise the error we got on the first try (which *may* have # a more useful traceback). (first_err, first_tb) = first_err_and_tb raise first_err.__class__, first_err, first_tb fargs = f.value.args - if len(fargs) > 0 and int(fargs[0]) >= 500 and int(fargs[0]) < 600: - # Retry on 5xx errors. - d = task.deferLater(reactor, BACKOFF_SECONDS_FOR_5XX[trynum-1], operation, *args, **kwargs) - d.addErrback(self._handle_error, trynum+1, first_err_and_tb, description, operation, *args, **kwargs) - return d - - # If we get an error response other than a 5xx, raise that error even if it was on a retry. + if len(fargs) > 0: + retry = self._react_to_error(int(fargs[0])) + if retry: + d = task.deferLater(reactor, BACKOFF_SECONDS_BEFORE_RETRY[trynum-1], operation, *args, **kwargs) + d.addErrback(self._handle_error, trynum+1, first_err_and_tb, description, operation, *args, **kwargs) + return d + + # If we get an error response for which _react_to_error says we should not retry, + # raise that error even if the request was itself a retry. raise err.__class__, err, tb diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 1f43ffff..35363717 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -1837,7 +1837,7 @@ class ServerWithMockCloudBackend(WithMockCloudBackend, ServerTest, unittest.Test return getattr(LogEvent, 'LEVELMAP', {}).get(level, str(level)) def _test_cloud_retry(self, name, failure_count, levels): - self.patch(cloud_common, 'BACKOFF_SECONDS_FOR_5XX', (0, 0.1, 0.2)) + self.patch(cloud_common, 'BACKOFF_SECONDS_BEFORE_RETRY', (0, 0.1, 0.2)) t = {'count': 0} old_put_object = MockContainer._put_object -- 2.45.2