Merge "Stop retrying every deletes in container-sync"

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)
 

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
 from swift.common.middleware.versioned_writes.object_versioning import (
     SYSMETA_VERSIONS_CONT, SYSMETA_VERSIONS_SYMLINK)
@@ -570,7 +570,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

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'

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