Optimize deferred deletion share instance get query

The periodic database queries made by the share manager service to
process deferred deletion of shares has been fixed to consider the
host in addition to the share's state. This both improves performance
of the periodic task, as well as fixes incorrect behavior where
incorrect shares are retrieved by the query.

Partially-implements: bp/deferred-deletion
Change-Id: I813a3130ae015a6b8778bb2a288075b949313c73
(cherry picked from commit fab04b5751)

manila/share/manager.py

@@ -3644,12 +3644,19 @@ class ShareManager(manager.SchedulerDependentManager):
 
     def _get_share_instances_with_deferred_deletion(self, ctxt):
         share_instances = self.db.share_instance_get_all(
-            ctxt, filters={'status': constants.STATUS_DEFERRED_DELETING})
+            ctxt,
+            filters={
+                'status': constants.STATUS_DEFERRED_DELETING,
+                'host': self.host,
+            })
 
         share_instances_error_deferred_deleting = (
             self.db.share_instance_get_all(
                 ctxt,
-                filters={'status': constants.STATUS_ERROR_DEFERRED_DELETING}))
+                filters={
+                    'status': constants.STATUS_ERROR_DEFERRED_DELETING,
+                    'host': self.host,
+                }))
         updated_del = timeutils.utcnow() - datetime.timedelta(minutes=30)
         for share_instance in share_instances_error_deferred_deleting:
             if share_instance.get('updated_at') < updated_del:

manila/tests/share/test_manager.py

@@ -104,6 +104,7 @@ class ShareManagerTestCase(test.TestCase):
         self.mock_object(self.share_manager.message_api, 'create')
         self.context = context.get_admin_context()
         self.share_manager.driver.initialized = True
+        self.host = 'host'
         self.share_manager.host = 'fake_host'
         mock.patch.object(
             lockutils, 'lock', fake_utils.get_fake_lock_context())
@@ -4065,17 +4066,19 @@ class ShareManagerTestCase(test.TestCase):
             'share_id': share['id'],
             'share_server_id': share_server['id'],
             'status': 'deferred_deleting',
-            'updated_at': timeutils.utcnow()
+            'updated_at': timeutils.utcnow(),
+            'host': self.host,
         }
-        db_utils.create_share_instance(**kwargs)
+        si_1 = db_utils.create_share_instance(**kwargs)
         kwargs = {
             'id': 2,
             'share_id': share['id'],
             'share_server_id': share_server['id'],
             'status': 'deferred_deleting',
-            'updated_at': timeutils.utcnow()
+            'updated_at': timeutils.utcnow(),
+            'host': self.host,
         }
-        db_utils.create_share_instance(**kwargs)
+        si_2 = db_utils.create_share_instance(**kwargs)
         mins = 20
         if consider_error_deleting:
             mins = 40
@@ -4084,15 +4087,22 @@ class ShareManagerTestCase(test.TestCase):
             'share_id': share['id'],
             'share_server_id': share_server['id'],
             'status': 'error_deferred_deleting',
-            'updated_at': timeutils.utcnow() - datetime.timedelta(minutes=mins)
+            'updated_at': (
+                timeutils.utcnow() - datetime.timedelta(minutes=mins)),
+            'host': self.host,
         }
-        db_utils.create_share_instance(**kwargs)
+        si_3 = db_utils.create_share_instance(**kwargs)
 
         self.mock_object(self.share_manager.db, 'share_server_get',
                          mock.Mock(return_value=share_server))
         self.mock_object(self.share_manager.db, 'share_get',
                          mock.Mock(return_value=share))
         self.mock_object(self.share_manager.db, 'share_instance_delete')
+        self.mock_object(
+            self.share_manager.db, 'share_instance_get_all',
+            mock.Mock(side_effect=[
+                [si_1, si_2],
+                [si_3] if consider_error_deleting else {}]))
         self.mock_object(self.share_manager, '_check_delete_share_server')
         self.mock_object(self.share_manager, '_notify_about_share_usage')
         mock_delete_share = self.mock_object(
@@ -4114,7 +4124,8 @@ class ShareManagerTestCase(test.TestCase):
             'share_id': share['id'],
             'share_server_id': share_server['id'],
             'status': 'deferred_deleting',
-            'updated_at': timeutils.utcnow()
+            'updated_at': timeutils.utcnow(),
+            'host': self.host,
         }
         si = db_utils.create_share_instance(**kwargs)
 
@@ -4125,6 +4136,9 @@ class ShareManagerTestCase(test.TestCase):
         self.mock_object(self.share_manager.db, 'share_instance_update')
         mock_delete = self.mock_object(self.share_manager.db,
                                        'share_instance_delete')
+        self.mock_object(
+            self.share_manager.db, 'share_instance_get_all',
+            mock.Mock(return_value=[si]))
         self.mock_object(
             self.share_manager.driver, 'delete_share',
             mock.Mock(side_effect=exception.ManilaException))

releasenotes/notes/optimize-deferred-deletion-get-share-instance-query-b6366b7c3b0a64db.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    The periodic database queries made by the share manager service to
+    process deferred deletion of shares has been fixed to consider the
+    host in addition to the share's state. This both improves performance
+    of the periodic task, as well as fixes incorrect behavior where
+    incorrect shares are retrieved by the query.