openstack/neutron
Code Issues Proposed changes

Fix slow SG api calls when limiting fields

Browse Source 
Relating to the issue with creating rules, when reading security
groups, it is very slow. Even when limiting to id/name, it pulls
in all rules before returning just id/name.

This change looks at the fields requested, and if no "synthetic"
fields are in the list, skips initializing those.

Co-Authored-By: Hongbin Lu<hongbin.lu@huawei.com>
Closes-Bug: #1810563
Change-Id: Id6870633e3943666e9b7fb900ad2d0894ee2715d
This commit is contained in:
Doug Wiegley 2019-01-11 15:14:52 -07:00
parent 4408da7fb6
commit 9aafd5f131
No known key found for this signature in database
GPG Key ID: 4D3C112B76BBDB5F
4 changed files with 86 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
neutron/db/securitygroups_db.py
View File

@ -161,7 +161,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
sorts=sorts, limit=limit, marker=marker, page_reverse=page_reverse)
sg_objs = sg_obj.SecurityGroup.get_objects(
context, _pager=pager, validate_filters=False, **filters)
context, _pager=pager, validate_filters=False,
fields=fields, **filters)
return [self._make_security_group_dict(obj, fields) for obj in sg_objs]
@ -176,7 +177,6 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
"""Tenant id is given to handle the case when creating a security
group rule on behalf of another use.
"""
if tenant_id:
tmp_context_tenant_id = context.tenant_id
context.tenant_id = tenant_id
@ -184,16 +184,22 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
try:
with db_api.context_manager.reader.using(context):
ret = self._make_security_group_dict(self._get_security_group(
context, id), fields)
ret['security_group_rules'] = self.get_security_group_rules(
context, {'security_group_id': [id]})
context, id,
fields=fields),
fields)
if (fields is None or len(fields) == 0 or
'security_group_rules' in fields):
rules = self.get_security_group_rules(context,
{'security_group_id': [id]})
ret['security_group_rules'] = rules
finally:
if tenant_id:
context.tenant_id = tmp_context_tenant_id
return ret
def _get_security_group(self, context, id):
sg = sg_obj.SecurityGroup.get_object(context, id=id)
def _get_security_group(self, context, id, fields=None):
sg = sg_obj.SecurityGroup.get_object(context, fields=fields, id=id)
if sg is None:
raise ext_sg.SecurityGroupNotFound(id=id)
return sg
@ -218,7 +224,7 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
if ports:
raise ext_sg.SecurityGroupInUse(id=id)
# confirm security group exists
sg = self._get_security_group(context, id)
sg = self._get_security_group(context, id, fields=['id', 'name'])
if sg['name'] == 'default' and not context.is_admin:
raise ext_sg.SecurityGroupCannotRemoveDefault()
@ -291,6 +297,7 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
'name': security_group['name'],
'tenant_id': security_group['tenant_id'],
'description': security_group['description']}
if security_group.rules:
res['security_group_rules'] = [
self._make_security_group_rule_dict(r.db_obj)
for r in security_group.rules

39
neutron/objects/base.py
View File

@ -137,6 +137,7 @@ class NeutronObject(obj_base.VersionedObject,
def __init__(self, context=None, **kwargs):
super(NeutronObject, self).__init__(context, **kwargs)
self._load_synthetic_fields = True
self.obj_set_defaults()
def _synthetic_fields_items(self):
@ -223,7 +224,7 @@ class NeutronObject(obj_base.VersionedObject,
return obj
@classmethod
def get_object(cls, context, **kwargs):
def get_object(cls, context, fields=None, **kwargs):
raise NotImplementedError()
@classmethod
@ -252,7 +253,7 @@ class NeutronObject(obj_base.VersionedObject,
@classmethod
@abc.abstractmethod
def get_objects(cls, context, _pager=None, validate_filters=True,
**kwargs):
fields=None, **kwargs):
raise NotImplementedError()
@classmethod
@ -443,6 +444,7 @@ class NeutronDbObject(NeutronObject):
for field in self.fields:
if field in fields and not self.is_synthetic(field):
setattr(self, field, fields[field])
if self._load_synthetic_fields:
self.load_synthetic_db_fields(db_obj)
self._captured_db_model = db_obj
self.obj_reset_changes()
@ -497,8 +499,13 @@ class NeutronDbObject(NeutronObject):
return result
@classmethod
def _load_object(cls, context, db_obj):
def _load_object(cls, context, db_obj, fields=None):
obj = cls(context)
if fields is not None and len(fields) != 0:
if len(set(fields).intersection(set(cls.synthetic_fields))) == 0:
obj._load_synthetic_fields = False
obj.from_db_object(db_obj)
# detach the model so that consequent fetches don't reuse it
context.session.expunge(obj.db_obj)
@ -533,12 +540,18 @@ class NeutronDbObject(NeutronObject):
return db_api.autonested_transaction(context.session)
@classmethod
def get_object(cls, context, **kwargs):
"""
def get_object(cls, context, fields=None, **kwargs):
"""Fetch a single object
Return the first result of given context or None if the result doesn't
contain any row. Next, convert it to a versioned object.
:param context:
:param fields: indicate which fields the caller is interested in
using. Note that currently this is limited to
avoid loading synthetic fields when possible, and
does not affect db queries. Default is None, which
is the same as []. Example: ['id', 'name']
:param kwargs: multiple keys defined by key=value pairs
:return: single object of NeutronDbObject class or None
"""
@ -553,12 +566,13 @@ class NeutronDbObject(NeutronObject):
db_obj = obj_db_api.get_object(
cls, context, **cls.modify_fields_to_db(kwargs))
if db_obj:
return cls._load_object(context, db_obj)
return cls._load_object(context, db_obj, fields=fields)
@classmethod
def get_objects(cls, context, _pager=None, validate_filters=True,
**kwargs):
"""
fields=None, **kwargs):
"""Fetch a list of objects
Fetch all results from DB and convert them to versioned objects.
:param context:
@ -566,6 +580,11 @@ class NeutronDbObject(NeutronObject):
criteria
:param validate_filters: Raises an error in case of passing an unknown
filter
:param fields: indicate which fields the caller is interested in
using. Note that currently this is limited to
avoid loading synthetic fields when possible, and
does not affect db queries. Default is None, which
is the same as []. Example: ['id', 'name']
:param kwargs: multiple keys defined by key=value pairs
:return: list of objects of NeutronDbObject class or empty list
"""
@ -574,7 +593,9 @@ class NeutronDbObject(NeutronObject):
with cls.db_context_reader(context):
db_objs = obj_db_api.get_objects(
cls, context, _pager=_pager, **cls.modify_fields_to_db(kwargs))
return [cls._load_object(context, db_obj) for db_obj in db_objs]
return [cls._load_object(context, db_obj, fields=fields)
for db_obj in db_objs]
@classmethod
def update_object(cls, context, values, validate_filters=True, **kwargs):

4
neutron/objects/securitygroup.py
View File

@ -59,7 +59,9 @@ class SecurityGroup(base.NeutronDbObject):
def from_db_object(self, db_obj):
super(SecurityGroup, self).from_db_object(db_obj)
setattr(self, 'is_default', bool(db_obj.get('default_security_group')))
if self._load_synthetic_fields:
setattr(self, 'is_default',
bool(db_obj.get('default_security_group')))
self.obj_reset_changes(['is_default'])

32
neutron/tests/unit/objects/test_securitygroup.py
View File

@ -97,6 +97,38 @@ class SecurityGroupDbObjTestCase(test_base.BaseDbObjectTestCase,
# follow-up patch.
pass
def test_get_object_no_synth(self):
fields = self.obj_fields[0].copy()
sg_obj = self._make_object(fields)
sg_obj.is_default = True
sg_obj.create()
listed_obj = securitygroup.SecurityGroup.get_object(
self.context,
fields=['id', 'name'],
id=sg_obj.id,
project_id=sg_obj.project_id
)
self.assertIsNotNone(listed_obj)
self.assertEqual(len(sg_obj.rules), 0)
self.assertIsNone(listed_obj.rules)
def test_get_objects_no_synth(self):
fields = self.obj_fields[0].copy()
sg_obj = self._make_object(fields)
sg_obj.is_default = True
sg_obj.create()
listed_objs = securitygroup.SecurityGroup.get_objects(
self.context,
fields=['id', 'name'],
id=sg_obj.id,
project_id=sg_obj.project_id
)
self.assertEqual(len(listed_objs), 1)
self.assertEqual(len(sg_obj.rules), 0)
self.assertIsNone(listed_objs[0].rules)
class DefaultSecurityGroupIfaceObjTestCase(test_base.BaseObjectIfaceTestCase):