Merge "Optimize deferred deletion share instance get query"

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.