diff --git a/swift/common/internal_client.py b/swift/common/internal_client.py index 7908424939..e1f2f17941 100644 --- a/swift/common/internal_client.py +++ b/swift/common/internal_client.py @@ -28,7 +28,7 @@ from zlib import compressobj from swift.common.constraints import AUTO_CREATE_ACCOUNT_PREFIX from swift.common.exceptions import ClientException from swift.common.http import (HTTP_NOT_FOUND, HTTP_MULTIPLE_CHOICES, - is_server_error) + is_client_error, is_server_error) from swift.common.swob import Request, bytes_to_wsgi from swift.common.utils import quote, closing_if_possible from swift.common.wsgi import loadapp, pipeline_property @@ -930,14 +930,17 @@ class SimpleClient(object): self.attempts += 1 try: return self.base_request(method, **kwargs) + except urllib2.HTTPError as err: + if is_client_error(err.getcode() or 500): + raise ClientException('Client error', + http_status=err.getcode()) + elif self.attempts > retries: + raise ClientException('Raise too many retries', + http_status=err.getcode()) except (socket.error, httplib.HTTPException, urllib2.URLError) \ as err: if self.attempts > retries: - if isinstance(err, urllib2.HTTPError): - raise ClientException('Raise too many retries', - http_status=err.getcode()) - else: - raise + raise sleep(backoff) backoff = min(backoff * 2, self.max_backoff) diff --git a/swift/container/sync.py b/swift/container/sync.py index 92c3d740c0..b793f8a4dc 100644 --- a/swift/container/sync.py +++ b/swift/container/sync.py @@ -42,7 +42,7 @@ from swift.common.utils import ( FileLikeIter, get_logger, hash_path, quote, validate_sync_to, whataremyips, Timestamp, decode_timestamps) from swift.common.daemon import Daemon -from swift.common.http import HTTP_UNAUTHORIZED, HTTP_NOT_FOUND +from swift.common.http import HTTP_UNAUTHORIZED, HTTP_NOT_FOUND, HTTP_CONFLICT from swift.common.wsgi import ConfigString @@ -561,7 +561,8 @@ class ContainerSync(Daemon): logger=self.logger, timeout=self.conn_timeout) except ClientException as err: - if err.http_status != HTTP_NOT_FOUND: + if err.http_status not in ( + HTTP_NOT_FOUND, HTTP_CONFLICT): raise self.container_deletes += 1 self.container_stats['deletes'] += 1 diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py index 597e30e3d9..e3e72a7fb5 100644 --- a/test/unit/common/test_internal_client.py +++ b/test/unit/common/test_internal_client.py @@ -1600,6 +1600,36 @@ class TestSimpleClient(unittest.TestCase): self.assertEqual(mock_sleep.call_count, 1) self.assertEqual(mock_urlopen.call_count, 2) + @mock.patch.object(urllib2, 'urlopen') + def test_delete_object_with_404_no_retry(self, mock_urlopen): + mock_response = mock.MagicMock() + mock_response.read.return_value = b'' + err_args = [None, 404, None, None, None] + mock_urlopen.side_effect = urllib2.HTTPError(*err_args) + + with mock.patch('swift.common.internal_client.sleep') as mock_sleep, \ + self.assertRaises(exceptions.ClientException) as caught: + internal_client.delete_object('http://127.0.0.1', + container='con', name='obj') + self.assertEqual(caught.exception.http_status, 404) + self.assertEqual(mock_sleep.call_count, 0) + self.assertEqual(mock_urlopen.call_count, 1) + + @mock.patch.object(urllib2, 'urlopen') + def test_delete_object_with_409_no_retry(self, mock_urlopen): + mock_response = mock.MagicMock() + mock_response.read.return_value = b'' + err_args = [None, 409, None, None, None] + mock_urlopen.side_effect = urllib2.HTTPError(*err_args) + + with mock.patch('swift.common.internal_client.sleep') as mock_sleep, \ + self.assertRaises(exceptions.ClientException) as caught: + internal_client.delete_object('http://127.0.0.1', + container='con', name='obj') + self.assertEqual(caught.exception.http_status, 409) + self.assertEqual(mock_sleep.call_count, 0) + self.assertEqual(mock_urlopen.call_count, 1) + def test_proxy(self): # check that proxy arg is passed through to the urllib Request scheme = 'http' diff --git a/test/unit/container/test_sync.py b/test/unit/container/test_sync.py index 9446ea727a..b61fdd0170 100644 --- a/test/unit/container/test_sync.py +++ b/test/unit/container/test_sync.py @@ -909,6 +909,24 @@ class TestContainerSync(unittest.TestCase): self.assertEqual(cs.container_deletes, 2) self.assertEqual(len(exc), 3) self.assertEqual(str(exc[-1]), 'test client exception: 404') + + def fake_delete_object(*args, **kwargs): + exc.append(ClientException('test client exception', + http_status=409)) + raise exc[-1] + + sync.delete_object = fake_delete_object + # Success because our tombstone is out of date + self.assertTrue(cs.container_sync_row( + {'deleted': True, + 'name': 'object', + 'created_at': '1.2'}, 'http://sync/to/path', + 'key', FakeContainerBroker('broker'), + {'account': 'a', 'container': 'c', 'storage_policy_index': 0}, + realm, realm_key)) + self.assertEqual(cs.container_deletes, 3) + self.assertEqual(len(exc), 4) + self.assertEqual(str(exc[-1]), 'test client exception: 409') finally: sync.uuid = orig_uuid sync.delete_object = orig_delete_object