From 327bd26e2f07ebc16ee1b025f2c029ea7d95baae Mon Sep 17 00:00:00 2001
From: You Yamagata <bi.yamagata@gmail.com>
Date: Tue, 4 Jun 2013 10:26:11 +0900
Subject: [PATCH] sqlalchemy: fix performance issue on get_meters()

Change SQL query to fix performance issue on Connection.get_meters().
It affects response time of 'ceilometer meter-list'.

Fixes: Bug #1180438

Change-Id: I8ff6e03b9487705fea277066fbc51f554c54bba5
---
 ceilometer/storage/impl_sqlalchemy.py         | 53 ++++++++++++-------
 .../versions/010_add_index_to_meter.py        | 34 ++++++++++++
 2 files changed, 69 insertions(+), 18 deletions(-)
 create mode 100644 ceilometer/storage/sqlalchemy/migrate_repo/versions/010_add_index_to_meter.py

diff --git a/ceilometer/storage/impl_sqlalchemy.py b/ceilometer/storage/impl_sqlalchemy.py
index 340c09deb..e816f2250 100644
--- a/ceilometer/storage/impl_sqlalchemy.py
+++ b/ceilometer/storage/impl_sqlalchemy.py
@@ -24,6 +24,7 @@ import os
 import uuid
 from sqlalchemy import func
 from sqlalchemy import desc
+from sqlalchemy.orm import aliased
 
 from ceilometer.openstack.common import log
 from ceilometer.openstack.common import timeutils
@@ -298,7 +299,31 @@ class Connection(base.Connection):
         :param metaquery: Optional dict with metadata to match on.
         """
         session = sqlalchemy_session.get_session()
-        query = session.query(Resource)
+
+        # Meter table will store large records and join with resource
+        # will be very slow.
+        # subquery_meter is used to reduce meter records
+        # by selecting a record for each (resource_id, counter_name).
+        # max() is used to choice a meter record, so the latest record
+        # is selected for each (resource_id, counter_name).
+        #
+        subquery_meter = session.query(func.max(Meter.id).label('id')).\
+            group_by(Meter.resource_id, Meter.counter_name).subquery()
+
+        # The SQL of query_meter is essentially:
+        #
+        # SELECT meter.* FROM meter INNER JOIN
+        #  (SELECT max(meter.id) AS id FROM meter
+        #   GROUP BY meter.resource_id, meter.counter_name) AS anon_2
+        # ON meter.id = anon_2.id
+        #
+        query_meter = session.query(Meter).\
+            join(subquery_meter, Meter.id == subquery_meter.c.id)
+
+        alias_meter = aliased(Meter, query_meter.subquery())
+        query = session.query(Resource, alias_meter).join(
+            alias_meter, Resource.id == alias_meter.resource_id)
+
         if user is not None:
             query = query.filter(Resource.user_id == user)
         if source is not None:
@@ -307,26 +332,18 @@ class Connection(base.Connection):
             query = query.filter(Resource.id == resource)
         if project is not None:
             query = query.filter(Resource.project_id == project)
-        query = query.options(
-            sqlalchemy_session.sqlalchemy.orm.joinedload('meters'))
         if metaquery:
             raise NotImplementedError('metaquery not implemented')
 
-        for resource in query.all():
-            meter_names = set()
-            for meter in resource.meters:
-                if meter.counter_name in meter_names:
-                    continue
-                meter_names.add(meter.counter_name)
-                yield api_models.Meter(
-                    name=meter.counter_name,
-                    type=meter.counter_type,
-                    unit=meter.counter_unit,
-                    resource_id=resource.id,
-                    project_id=resource.project_id,
-                    source=resource.sources[0].id,
-                    user_id=resource.user_id,
-                )
+        for resource, meter in query.all():
+            yield api_models.Meter(
+                name=meter.counter_name,
+                type=meter.counter_type,
+                unit=meter.counter_unit,
+                resource_id=resource.id,
+                project_id=resource.project_id,
+                source=resource.sources[0].id,
+                user_id=resource.user_id)
 
     @staticmethod
     def get_samples(sample_filter, limit=None):
diff --git a/ceilometer/storage/sqlalchemy/migrate_repo/versions/010_add_index_to_meter.py b/ceilometer/storage/sqlalchemy/migrate_repo/versions/010_add_index_to_meter.py
new file mode 100644
index 000000000..548bba7bd
--- /dev/null
+++ b/ceilometer/storage/sqlalchemy/migrate_repo/versions/010_add_index_to_meter.py
@@ -0,0 +1,34 @@
+# -*- encoding: utf-8 -*-
+#
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from sqlalchemy import MetaData
+from sqlalchemy import Index
+from ceilometer.storage.sqlalchemy.models import Meter
+
+meta = MetaData()
+
+
+def upgrade(migrate_engine):
+    meta.bind = migrate_engine
+    index = Index('idx_meter_rid_cname', Meter.resource_id,
+                  Meter.counter_name)
+    index.create(bind=migrate_engine)
+
+
+def downgrade(migrate_engine):
+    meta.bind = migrate_engine
+    index = Index('idx_meter_rid_cname', Meter.resource_id,
+                  Meter.counter_name)
+    index.drop(bind=migrate_engine)