Skip to content

Commit

Permalink
Improve scheduler performance when thin provisioning
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
chuan137 authored and Carthaca committed May 9, 2023
1 parent 5b27408 commit 7c91e2f
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 15 deletions.
5 changes: 5 additions & 0 deletions manila/db/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions manila/db/sqlalchemy/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
10 changes: 1 addition & 9 deletions manila/scheduler/host_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
13 changes: 7 additions & 6 deletions manila/tests/scheduler/test_host_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down

0 comments on commit 7c91e2f

Please sign in to comment.