From 7c91e2fafcadf17513ea5c8808d928e2a001a324 Mon Sep 17 00:00:00 2001 From: chuan137 Date: Tue, 9 May 2023 11:35:35 +0200 Subject: [PATCH] Improve scheduler performance when thin provisioning The scheduler need to estimate the allocated capacity for thin provisioning hosts. To do this more efficiently, use a sum fucntion in query, instead of loading all share instances for a specific host. --- manila/db/api.py | 5 +++++ manila/db/sqlalchemy/api.py | 13 +++++++++++++ manila/scheduler/host_manager.py | 10 +--------- manila/tests/scheduler/test_host_manager.py | 13 +++++++------ 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/manila/db/api.py b/manila/db/api.py index 06b82967b9..d8752e03c4 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -376,6 +376,11 @@ def share_instances_get_all_by_share_group_id(context, share_group_id): context, share_group_id) +def share_instance_sizes_sum_by_host(context, host): + """Returns sum of sizes of all share instances on given host.""" + return IMPL.share_instance_sizes_sum_by_host(context, host) + + def share_instance_purge(context, instance_id): """Removes share instance from database.""" return IMPL.share_instance_purge(context, instance_id) diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 5bc9d17ef6..b9da77d399 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -1784,6 +1784,19 @@ def share_instances_get_all_by_host(context, host, with_share_data=False, return instances +@require_context +def share_instance_sizes_sum_by_host(context, host): + result = model_query( + context, models.Share, func.sum(models.Share.size), + ).join( + models.ShareInstance.share, + ).filter(or_( + models.ShareInstance.host == host, + models.ShareInstance.host.like("{0}#%".format(host)), + )).first() + return int(result[0] or 0) + + @require_context def share_instances_get_all_by_share_network(context, share_network_id): """Returns list of share instances that belong to given share network.""" diff --git a/manila/scheduler/host_manager.py b/manila/scheduler/host_manager.py index be00e67636..ea63f3f98d 100644 --- a/manila/scheduler/host_manager.py +++ b/manila/scheduler/host_manager.py @@ -418,15 +418,7 @@ def __init__(self, host, capabilities, pool_name): def _estimate_provisioned_capacity(self, host_name, context=None): """Estimate provisioned capacity from share sizes on backend.""" - provisioned_capacity = 0 - - instances = db.share_instances_get_all_by_host( - context, host_name, with_share_data=True) - - for instance in instances: - # Size of share instance that's still being created, will be None. - provisioned_capacity += instance['size'] or 0 - return provisioned_capacity + return db.share_instance_sizes_sum_by_host(context, host_name) def update_from_share_capability( self, capability, service=None, context=None): diff --git a/manila/tests/scheduler/test_host_manager.py b/manila/tests/scheduler/test_host_manager.py index 8c85c52774..6e1b187cec 100644 --- a/manila/tests/scheduler/test_host_manager.py +++ b/manila/tests/scheduler/test_host_manager.py @@ -1196,9 +1196,10 @@ class PoolStateTestCase(test.TestCase): @ddt.unpack def test_update_from_share_capability(self, share_capability, instances): fake_context = context.RequestContext('user', 'project', is_admin=True) + sizes = [instance['size'] or 0 for instance in instances] self.mock_object( - db, 'share_instances_get_all_by_host', - mock.Mock(return_value=instances)) + db, 'share_instance_sizes_sum_by_host', + mock.Mock(return_value=sum(sizes))) fake_pool = host_manager.PoolState('host1', None, 'pool0') self.assertIsNone(fake_pool.free_capacity_gb) @@ -1221,19 +1222,19 @@ def test_update_from_share_capability(self, share_capability, instances): self.assertEqual(thin_provisioned, fake_pool.thin_provisioning) if 'provisioned_capacity_gb' not in share_capability or ( share_capability['provisioned_capacity_gb'] is None): - db.share_instances_get_all_by_host.assert_called_once_with( - fake_context, fake_pool.host, with_share_data=True) + db.share_instance_sizes_sum_by_host.assert_called_once_with( + fake_context, fake_pool.host) if len(instances) > 0: self.assertEqual(4, fake_pool.provisioned_capacity_gb) else: self.assertEqual(0, fake_pool.provisioned_capacity_gb) else: - self.assertFalse(db.share_instances_get_all_by_host.called) + self.assertFalse(db.share_instance_sizes_sum_by_host.called) self.assertEqual(share_capability['provisioned_capacity_gb'], fake_pool.provisioned_capacity_gb) else: self.assertFalse(fake_pool.thin_provisioning) - self.assertFalse(db.share_instances_get_all_by_host.called) + self.assertFalse(db.share_instance_sizes_sum_by_host.called) if 'provisioned_capacity_gb' not in share_capability or ( share_capability['provisioned_capacity_gb'] is None): self.assertIsNone(fake_pool.provisioned_capacity_gb)