perf: improve share creation by using SQL aggregates in server limit checks

manila/db/api.py

@@ -396,6 +396,17 @@ def share_instance_sizes_sum_by_host(context, host):
     return IMPL.share_instance_sizes_sum_by_host(context, host)
 
 
+def share_instance_count_and_size_sum_by_server(context, share_server_id):
+    """Returns (count, size_sum) of share instances on the given server."""
+    return IMPL.share_instance_count_and_size_sum_by_server(
+        context, share_server_id)
+
+
+def share_snapshot_size_sum_by_server(context, share_server_id):
+    """Returns total snapshot size for all snapshots on the given server."""
+    return IMPL.share_snapshot_size_sum_by_server(context, share_server_id)
+
+
 def share_instance_purge(context, instance_id):
     """Removes share instance from database."""
     return IMPL.share_instance_purge(context, instance_id)

manila/db/sqlalchemy/api.py

@@ -2181,6 +2181,57 @@ def share_instance_sizes_sum_by_host(context, host):
     return int(result[0] or 0)
 
 
+@require_context
+@context_manager.reader
+def share_instance_count_and_size_sum_by_server(context, share_server_id):
+    """Return (count, size_sum) of share instances on the share server."""
+    result = model_query(
+        context, models.ShareInstance,
+        func.count(models.ShareInstance.id),
+        func.sum(models.Share.size),
+    ).join(
+        models.ShareInstance.share,
+    ).filter(
+        models.ShareInstance.share_server_id == share_server_id,
+    ).first()
+    return int(result[0] or 0), int(result[1] or 0)
+
+
+@require_context
+@context_manager.reader
+def share_snapshot_size_sum_by_server(context, share_server_id):
+    """Return total snapshot size for all snapshots on the given share server.
+
+    Uses instance_size when set, otherwise falls back to ShareSnapshot.size,
+    mirroring the ShareSnapshotInstance.size property.
+
+    Share-group snapshot members (snapshot_id=NULL) are excluded: their
+    snapshot relationship is NULL so their size cannot be resolved anyway.
+    """
+    result = model_query(
+        context, models.ShareSnapshotInstance,
+        func.sum(
+            func.coalesce(
+                models.ShareSnapshotInstance.instance_size,
+                models.ShareSnapshot.size,
+            )
+        ),
+    ).join(
+        models.ShareSnapshot,
+        models.ShareSnapshot.id == models.ShareSnapshotInstance.snapshot_id,
+    ).join(
+        models.ShareInstance,
+        models.ShareInstance.id ==
+        models.ShareSnapshotInstance.share_instance_id,
+    ).filter(
+        models.ShareInstance.share_server_id == share_server_id,
+        models.ShareInstance.deleted == 'False',
+        models.ShareSnapshotInstance.deleted == 'False',
+        models.ShareSnapshot.deleted == 'False',
+    ).first()
+    return int(result[0] or 0)
+
+
 @require_context
 @context_manager.reader
 def share_instance_get_all_by_share_network(context, share_network_id):

manila/share/manager.py

@@ -881,24 +881,15 @@ def _check_share_server_backend_limits(
             return available_share_servers
 
         for ss in available_share_servers[:]:
-            share_instances = self.db.share_instance_get_all_by_share_server(
-                context, ss['id'], with_share_data=True)
-            if not share_instances:
+            num_instances, instances_size_sum = (
+                self.db.share_instance_count_and_size_sum_by_server(
+                    context, ss['id']))
+            if num_instances == 0:
                 continue
-            share_instance_ids = [si['id'] for si in share_instances]
-            share_snapshot_instances = (
-                self.db.share_snapshot_instance_get_all_with_filters(
-                    context, {"share_instance_ids": share_instance_ids},
-                    with_share_data=True))
-
-            server_instances_size_sum = 0
-            num_instances = 0
+            snapshot_size_sum = self.db.share_snapshot_size_sum_by_server(
+                context, ss['id'])
 
-            server_instances_size_sum += sum(
-                instance['size'] for instance in share_instances)
-            server_instances_size_sum += sum(
-                instance['size'] for instance in share_snapshot_instances)
-            num_instances += len(share_instances)
+            server_instances_size_sum = instances_size_sum + snapshot_size_sum
 
             # NOTE(carloss): If a share instance was not provided, means that
             # a share group is being requested and there aren't shares to

manila/tests/share/test_manager.py

@@ -3212,7 +3212,7 @@ def test__check_share_server_backend_limits(
 
         # Creates some snapshot instances to make sure they are being
         # accounted
-        snapshot_instances = [
+        snapshots = [
             db_utils.create_snapshot(
                 size=resource_size, share_id=share['id'])['instance']
             for i in range(3)]
@@ -3226,12 +3226,15 @@ def test__check_share_server_backend_limits(
         driver_mock.max_shares_per_share_server = max_shares
         driver_mock.max_share_server_size = max_gigabytes
         self.share_manager.driver = driver_mock
+        num_instances = len(share_instances)
+        instances_size_sum = sum(si['size'] for si in share_instances)
+        snapshot_size_sum = sum(si['size'] for si in snapshots)
         self.mock_object(
-            db, 'share_instance_get_all_by_share_server',
-            mock.Mock(return_value=share_instances))
+            db, 'share_instance_count_and_size_sum_by_server',
+            mock.Mock(return_value=(num_instances, instances_size_sum)))
         self.mock_object(
-            db, 'share_snapshot_instance_get_all_with_filters',
-            mock.Mock(return_value=snapshot_instances))
+            db, 'share_snapshot_size_sum_by_server',
+            mock.Mock(return_value=snapshot_size_sum))
 
         # NOTE(carloss): If with_share_instance, simulates the behavior where
         # the provide_share_server method call was not related to a request to
@@ -3276,11 +3279,12 @@ def test__check_share_server_backend_limits_migrating_share(self):
 
         self.share_manager.driver = driver_mock
         self.mock_object(
-            db, 'share_instance_get_all_by_share_server',
-            mock.Mock(return_value=share_instances))
+            db, 'share_instance_count_and_size_sum_by_server',
+            mock.Mock(return_value=(len(share_instances),
+                                    sum(s['size'] for s in share_instances))))
         self.mock_object(
-            db, 'share_snapshot_instance_get_all_with_filters',
-            mock.Mock(return_value=[]))
+            db, 'share_snapshot_size_sum_by_server',
+            mock.Mock(return_value=0))
         self.mock_object(db, 'share_get', mock.Mock(return_value=share))
         self.mock_object(api.API, 'get_migrating_instances',
                          mock.Mock(return_value=share_instance_ids))
@@ -3302,12 +3306,10 @@ def test__check_share_server_backend_limits_migrating_share(self):
 
         self.assertEqual(
             1, len(available_share_servers))
-        db.share_instance_get_all_by_share_server.assert_called_once_with(
-            self.context, share_servers[0]['id'], with_share_data=True)
-        (db.share_snapshot_instance_get_all_with_filters.
-            assert_called_once_with(
-                self.context, {"share_instance_ids": share_instance_ids},
-                with_share_data=True))
+        db.share_instance_count_and_size_sum_by_server.assert_called_once_with(
+            self.context, share_servers[0]['id'])
+        db.share_snapshot_size_sum_by_server.assert_called_once_with(
+            self.context, share_servers[0]['id'])
         db.share_get.assert_called_once_with(self.context, share['id'])
         api.API.get_migrating_instances.assert_called_once_with(share)
         db.share_instance_get.assert_called_once_with(