From b920d15b01b21831999030b0becbc1da988ee638 Mon Sep 17 00:00:00 2001
From: Roman Verchikov <rverchikov@mirantis.com>
Date: Wed, 7 Aug 2013 14:56:24 -0700
Subject: [PATCH] Fix select n+1 issue in keystone catalog

keystone.catalog.backends.sql.get_catalog() and get_v3_catalog() methods
generate N+1 select statements for each endpoint. Use sqlalchemy's
eager load to generate single select statement instead of generating N+1
select statents for each endpoint.

Given change does not modify DB schema and is runtime-only, since it's a
one-to-many relationship.

Change-Id: Ia72b8603fc13f01696771f6116b320364bd50f51
Fixes: bug #1206725
---
 keystone/catalog/backends/sql.py | 71 ++++++++++++++------------------
 keystone/common/sql/core.py      |  4 ++
 2 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/keystone/catalog/backends/sql.py b/keystone/catalog/backends/sql.py
index 1dad5a8017..718d7c53ac 100644
--- a/keystone/catalog/backends/sql.py
+++ b/keystone/catalog/backends/sql.py
@@ -32,6 +32,7 @@ class Service(sql.ModelBase, sql.DictBase):
     id = sql.Column(sql.String(64), primary_key=True)
     type = sql.Column(sql.String(255))
     extra = sql.Column(sql.JsonBlob())
+    endpoints = sql.relationship("Endpoint", backref="service")
 
 
 class Endpoint(sql.ModelBase, sql.DictBase):
@@ -150,28 +151,26 @@ def get_catalog(self, user_id, tenant_id, metadata=None):
         d.update({'tenant_id': tenant_id,
                   'user_id': user_id})
 
+        session = self.get_session()
+        endpoints = (session.query(Endpoint).
+                     options(sql.joinedload(Endpoint.service)).
+                     all())
+
         catalog = {}
-        services = {}
-        for endpoint in self.list_endpoints():
-            # look up the service
-            services.setdefault(
-                endpoint['service_id'],
-                self.get_service(endpoint['service_id']))
-            service = services[endpoint['service_id']]
-
-            # add the endpoint to the catalog if it's not already there
-            catalog.setdefault(endpoint['region'], {})
-            catalog[endpoint['region']].setdefault(
-                service['type'], {
-                    'id': endpoint['id'],
-                    'name': service['name'],
-                    'publicURL': '',  # this may be overridden, but must exist
-                })
-
-            # add the interface's url
-            url = core.format_url(endpoint.get('url'), d)
+
+        for endpoint in endpoints:
+            region = endpoint['region']
+            service_type = endpoint.service['type']
+            default_service = {
+                'id': endpoint['id'],
+                'name': endpoint.service['name'],
+                'publicURL': ''
+            }
+            catalog.setdefault(region, {})
+            catalog[region].setdefault(service_type, default_service)
+            url = core.format_url(endpoint['url'], d)
             interface_url = '%sURL' % endpoint['interface']
-            catalog[endpoint['region']][service['type']][interface_url] = url
+            catalog[region][service_type][interface_url] = url
 
         return catalog
 
@@ -180,27 +179,19 @@ def get_v3_catalog(self, user_id, tenant_id, metadata=None):
         d.update({'tenant_id': tenant_id,
                   'user_id': user_id})
 
-        services = {}
-        for endpoint in self.list_endpoints():
-            # look up the service
-            service_id = endpoint['service_id']
-            services.setdefault(
-                service_id,
-                self.get_service(service_id))
-            service = services[service_id]
+        session = self.get_session()
+        services = (session.query(Service).
+                    options(sql.joinedload(Service.endpoints)).
+                    all())
+
+        def make_v3_endpoint(endpoint):
             del endpoint['service_id']
             endpoint['url'] = core.format_url(endpoint['url'], d)
-            if 'endpoints' in services[service_id]:
-                services[service_id]['endpoints'].append(endpoint)
-            else:
-                services[service_id]['endpoints'] = [endpoint]
-
-        catalog = []
-        for service_id, service in services.iteritems():
-            formatted_service = {}
-            formatted_service['id'] = service['id']
-            formatted_service['type'] = service['type']
-            formatted_service['endpoints'] = service['endpoints']
-            catalog.append(formatted_service)
+            return endpoint
+
+        catalog = [{'endpoints': [make_v3_endpoint(ep.to_dict())
+                                  for ep in svc.endpoints],
+                    'id': svc.id,
+                    'type': svc.type} for svc in services]
 
         return catalog
diff --git a/keystone/common/sql/core.py b/keystone/common/sql/core.py
index 2d3114f26a..a2deb58b7e 100644
--- a/keystone/common/sql/core.py
+++ b/keystone/common/sql/core.py
@@ -54,6 +54,8 @@
 Boolean = sql.Boolean
 Text = sql.Text
 UniqueConstraint = sql.UniqueConstraint
+relationship = sql.orm.relationship
+joinedload = sql.orm.joinedload
 
 
 def initialize_decorator(init):
@@ -179,6 +181,8 @@ def __setitem__(self, key, value):
         setattr(self, key, value)
 
     def __getitem__(self, key):
+        if key in self.extra:
+            return self.extra[key]
         return getattr(self, key)
 
     def get(self, key, default=None):