respect bulk delete page size and fix logic error

Previously, using SwiftService to delete "many" objects would use
bulk delete if available, but it would not respect the bulk delete
page size. If the number of objects to delete exceeded the bulk delete
page size, SwiftService would ignore the error and nothing would be
deleted.

This patch changes _should_bulk_delete() to be _bulk_delete_page_size();
instead of returning a simple True/False, it returns the page size for
the bulk deleter, or 1 if objects should be deleted one at a time.
Delete SDK calls are then spread across multiple bulk DELETEs if the
requested number of objects to delete exceeds the returned page size.

Fixed the logic in _should_bulk_delete() so that if the object list
is exactly 2x the thread count, it will not bulk delete. This is the
natural conclusion following the logic that existed previously: if
the delete request can be satisfied by every worker thread doing one
or two tasks, don't bulk delete. But if it requires a worker thread
to do three or more tasks, do a bulk delete instead. Previously, the
logic would mean that if every worker thread did exactly two tasks, it
would bulk delete. This patch changes a "<" to a "<=".

Closes-Bug: 1679851
Change-Id: I3c18f89bac1170dc62187114ef06dbe721afcc2e

1 files changed, 30 insertions, 12 deletions

diff --git a/swiftclient/service.py b/swiftclient/service.py
index ce0c4fc..223641b 100644
--- a/swiftclient/service.py
+++ b/swiftclient/service.py
@@ -43,7 +43,8 @@ from swiftclient.command_helpers import (
 )
 from swiftclient.utils import (
     config_true_value, ReadableToIterable, LengthWrapper, EMPTY_ETAG,
-    parse_api_response, report_traceback, n_groups, split_request_headers
+    parse_api_response, report_traceback, n_groups, split_request_headers,
+    n_at_a_time
 )
 from swiftclient.exceptions import ClientException
 from swiftclient.multithreading import MultiThreadingManager
@@ -2151,11 +2152,15 @@ class SwiftService(object):
                 rq = Queue()
                 obj_dels = {}
 
-                if self._should_bulk_delete(objects):
-                    for obj_slice in n_groups(
-                            objects, self._options['object_dd_threads']):
-                        self._bulk_delete(container, obj_slice, options,
-                                          obj_dels)
+                bulk_page_size = self._bulk_delete_page_size(objects)
+                if bulk_page_size > 1:
+                    page_at_a_time = n_at_a_time(objects, bulk_page_size)
+                    for page_slice in page_at_a_time:
+                        for obj_slice in n_groups(
+                                page_slice,
+                                self._options['object_dd_threads']):
+                            self._bulk_delete(container, obj_slice, options,
+                                              obj_dels)
                 else:
                     self._per_item_delete(container, objects, options,
                                           obj_dels, rq)
@@ -2208,23 +2213,36 @@ class SwiftService(object):
                                     and not res['success']):
                                 cancelled = True
 
-    def _should_bulk_delete(self, objects):
-        if len(objects) < 2 * self._options['object_dd_threads']:
+    def _bulk_delete_page_size(self, objects):
+        '''
+        Given the iterable 'objects', will return how many items should be
+        deleted at a time.
+
+        :param objects: An iterable that supports 'len()'
+        :returns: The bulk delete page size (i.e. the max number of
+                  objects that can be bulk deleted at once, as reported by
+                  the cluster). If bulk delete is disabled, return 1
+        '''
+        if len(objects) <= 2 * self._options['object_dd_threads']:
             # Not many objects; may as well delete one-by-one
-            return False
+            return 1
 
         try:
             cap_result = self.capabilities()
             if not cap_result['success']:
                 # This shouldn't actually happen, but just in case we start
                 # being more nuanced about our capabilities result...
-                return False
+                return 1
         except ClientException:
             # Old swift, presumably; assume no bulk middleware
-            return False
+            return 1
 
         swift_info = cap_result['capabilities']
-        return 'bulk_delete' in swift_info
+        if 'bulk_delete' in swift_info:
+            return swift_info['bulk_delete'].get(
+                'max_deletes_per_request', 10000)
+        else:
+            return 1
 
     def _per_item_delete(self, container, objects, options, rdict, rq):
         for obj in objects: