From fab04b5751527c6a6e3bf3848c2af698088fb249 Mon Sep 17 00:00:00 2001 From: Kiran Pawar Date: Thu, 2 May 2024 13:03:45 +0000 Subject: [PATCH] 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 --- manila/share/manager.py | 11 ++++++-- manila/tests/share/test_manager.py | 28 ++++++++++++++----- ...share-instance-query-b6366b7c3b0a64db.yaml | 8 ++++++ 3 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/optimize-deferred-deletion-get-share-instance-query-b6366b7c3b0a64db.yaml diff --git a/manila/share/manager.py b/manila/share/manager.py index b4a89cfa15..d02ba027cd 100644 --- a/manila/share/manager.py +++ b/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: diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index c201232836..cb0a033a45 100644 --- a/manila/tests/share/test_manager.py +++ b/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)) diff --git a/releasenotes/notes/optimize-deferred-deletion-get-share-instance-query-b6366b7c3b0a64db.yaml b/releasenotes/notes/optimize-deferred-deletion-get-share-instance-query-b6366b7c3b0a64db.yaml new file mode 100644 index 0000000000..bedf09352b --- /dev/null +++ b/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.