From 00be3f595edd7aad57a6e0968b4cdf78eb4cda23 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 15 Nov 2018 11:56:36 -0800 Subject: [PATCH] s3api: Allow concurrent multi-deletes Previously, a thousand-item multi-delete request would consider each object to delete serially, and not start trying to delete one until the previous was deleted (or hit an error). Now, allow operators to configure a concurrency factor to allow multiple deletes at the same time. Default the concurrency to 2, like we did for slo and bulk. See also: http://lists.openstack.org/pipermail/openstack-dev/2016-May/095737.html Change-Id: If235931635094b7251e147d79c8b7daa10cdcb3d Related-Change: I128374d74a4cef7a479b221fd15eec785cc4694a --- etc/proxy-server.conf-sample | 4 ++ .../s3api/controllers/multi_delete.py | 35 +++++++---- swift/common/middleware/s3api/s3api.py | 2 + test/functional/s3api/test_multi_delete.py | 3 - .../middleware/s3api/test_multi_delete.py | 62 +++++++++++++++++++ 5 files changed, 90 insertions(+), 16 deletions(-) diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index 89ff1750ba..4776c85f8d 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -504,6 +504,10 @@ use = egg:swift#s3api # operation. # max_multi_delete_objects = 1000 # +# Set the number of objects to delete at a time with the Multi-Object Delete +# operation. +# multi_delete_concurrency = 2 +# # If set to 'true', s3api uses its own metadata for ACLs # (e.g. X-Container-Sysmeta-S3Api-Acl) to achieve the best S3 compatibility. # If set to 'false', s3api tries to use Swift ACLs (e.g. X-Container-Read) diff --git a/swift/common/middleware/s3api/controllers/multi_delete.py b/swift/common/middleware/s3api/controllers/multi_delete.py index e12dc82672..04a6ed37f2 100644 --- a/swift/common/middleware/s3api/controllers/multi_delete.py +++ b/swift/common/middleware/s3api/controllers/multi_delete.py @@ -13,8 +13,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy + from swift.common.constraints import MAX_OBJECT_NAME_LENGTH -from swift.common.utils import public +from swift.common.utils import public, StreamingPile from swift.common.middleware.s3api.controllers.base import Controller, \ bucket_operation @@ -102,11 +104,13 @@ class MultiObjectDeleteController(Controller): body = self._gen_error_body(error, elem, delete_list) return HTTPOk(body=body) - for key, version in delete_list: - if version is not None: - # TODO: delete the specific version of the object - raise S3NotImplemented() + if any(version is not None for _key, version in delete_list): + # TODO: support deleting specific versions of objects + raise S3NotImplemented() + def do_delete(base_req, key, version): + req = copy.copy(base_req) + req.environ = copy.copy(base_req.environ) req.object_name = key try: @@ -115,15 +119,20 @@ class MultiObjectDeleteController(Controller): except NoSuchKey: pass except ErrorResponse as e: - error = SubElement(elem, 'Error') - SubElement(error, 'Key').text = key - SubElement(error, 'Code').text = e.__class__.__name__ - SubElement(error, 'Message').text = e._msg - continue + return key, {'code': e.__class__.__name__, 'message': e._msg} + return key, None - if not self.quiet: - deleted = SubElement(elem, 'Deleted') - SubElement(deleted, 'Key').text = key + with StreamingPile(self.conf.multi_delete_concurrency) as pile: + for key, err in pile.asyncstarmap(do_delete, ( + (req, key, version) for key, version in delete_list)): + if err: + error = SubElement(elem, 'Error') + SubElement(error, 'Key').text = key + SubElement(error, 'Code').text = err['code'] + SubElement(error, 'Message').text = err['message'] + elif not self.quiet: + deleted = SubElement(elem, 'Deleted') + SubElement(deleted, 'Key').text = key body = tostring(elem) diff --git a/swift/common/middleware/s3api/s3api.py b/swift/common/middleware/s3api/s3api.py index ac03e472d7..9a755bbc9f 100644 --- a/swift/common/middleware/s3api/s3api.py +++ b/swift/common/middleware/s3api/s3api.py @@ -195,6 +195,8 @@ class S3ApiMiddleware(object): conf.get('max_parts_listing', 1000)) self.conf.max_multi_delete_objects = config_positive_int_value( conf.get('max_multi_delete_objects', 1000)) + self.conf.multi_delete_concurrency = config_positive_int_value( + conf.get('multi_delete_concurrency', 2)) self.conf.s3_acl = config_true_value( conf.get('s3_acl', False)) self.conf.storage_domain = conf.get('storage_domain', '') diff --git a/test/functional/s3api/test_multi_delete.py b/test/functional/s3api/test_multi_delete.py index f6c9d47f44..1c73077033 100644 --- a/test/functional/s3api/test_multi_delete.py +++ b/test/functional/s3api/test_multi_delete.py @@ -33,9 +33,6 @@ def tearDownModule(): class TestS3ApiMultiDelete(S3ApiBase): - def setUp(self): - super(TestS3ApiMultiDelete, self).setUp() - def _prepare_test_delete_multi_objects(self, bucket, objects): self.conn.make_request('PUT', bucket) for obj in objects: diff --git a/test/unit/common/middleware/s3api/test_multi_delete.py b/test/unit/common/middleware/s3api/test_multi_delete.py index a1ba1483a3..69241d9295 100644 --- a/test/unit/common/middleware/s3api/test_multi_delete.py +++ b/test/unit/common/middleware/s3api/test_multi_delete.py @@ -95,6 +95,43 @@ class TestS3ApiMultiDelete(S3ApiTestCase): ('DELETE', '/v1/AUTH_test/bucket/Key3?multipart-manifest=delete'), ]) + @s3acl + def test_object_multi_DELETE_with_error(self): + self.swift.register('HEAD', '/v1/AUTH_test/bucket/Key3', + swob.HTTPForbidden, {}, None) + self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key1', + swob.HTTPNoContent, {}, None) + self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key2', + swob.HTTPNotFound, {}, None) + + elem = Element('Delete') + for key in ['Key1', 'Key2', 'Key3']: + obj = SubElement(elem, 'Object') + SubElement(obj, 'Key').text = key + body = tostring(elem, use_s3ns=False) + content_md5 = md5(body).digest().encode('base64').strip() + + req = Request.blank('/bucket?delete', + environ={'REQUEST_METHOD': 'POST'}, + headers={'Authorization': 'AWS test:tester:hmac', + 'Content-Type': 'multipart/form-data', + 'Date': self.get_date_header(), + 'Content-MD5': content_md5}, + body=body) + status, headers, body = self.call_s3api(req) + self.assertEqual(status.split()[0], '200') + + elem = fromstring(body) + self.assertEqual(len(elem.findall('Deleted')), 2) + self.assertEqual(len(elem.findall('Error')), 1) + self.assertEqual(self.swift.calls, [ + ('HEAD', '/v1/AUTH_test/bucket'), + ('HEAD', '/v1/AUTH_test/bucket/Key1'), + ('DELETE', '/v1/AUTH_test/bucket/Key1'), + ('HEAD', '/v1/AUTH_test/bucket/Key2'), + ('HEAD', '/v1/AUTH_test/bucket/Key3'), + ]) + @s3acl def test_object_multi_DELETE_quiet(self): self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key1', @@ -146,6 +183,31 @@ class TestS3ApiMultiDelete(S3ApiTestCase): status, headers, body = self.call_s3api(req) self.assertEqual(self._get_error_code(body), 'UserKeyMustBeSpecified') + @s3acl + def test_object_multi_DELETE_versioned(self): + self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key1', + swob.HTTPNoContent, {}, None) + self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key2', + swob.HTTPNotFound, {}, None) + + elem = Element('Delete') + SubElement(elem, 'Quiet').text = 'true' + for key in ['Key1', 'Key2']: + obj = SubElement(elem, 'Object') + SubElement(obj, 'Key').text = key + SubElement(obj, 'VersionId').text = 'not-supported' + body = tostring(elem, use_s3ns=False) + content_md5 = md5(body).digest().encode('base64').strip() + + req = Request.blank('/bucket?delete', + environ={'REQUEST_METHOD': 'POST'}, + headers={'Authorization': 'AWS test:tester:hmac', + 'Date': self.get_date_header(), + 'Content-MD5': content_md5}, + body=body) + status, headers, body = self.call_s3api(req) + self.assertEqual(self._get_error_code(body), 'NotImplemented') + @s3acl def test_object_multi_DELETE_with_invalid_md5(self): elem = Element('Delete')