Skip to content

Commit

Permalink
Fix select n+1 issue in keystone catalog
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Roman Verchikov committed Aug 7, 2013
1 parent ed1f967 commit b920d15
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 40 deletions.
71 changes: 31 additions & 40 deletions keystone/catalog/backends/sql.py
Expand Up @@ -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):
Expand Down Expand Up @@ -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

Expand All @@ -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
4 changes: 4 additions & 0 deletions keystone/common/sql/core.py
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit b920d15

Please sign in to comment.