openstack
/
neutron
Code Issues Proposed changes

Use dynamic lazy mode for fetching security group rules 

In conjunction with the prior fix to only get a subset of fields
when needed, this makes the querying of non-rules SG objects
very very fast.

Before the two fixes, if you have about ten security groups with 2000 rules each:

list all: 14s
list all, just 'id' field: 14s
list one: 0.6s
list one, just 'id' field: 0.6s

With just the previous partial fix:

list all: 14s
list all, just 'id' field: 6s
list one: 0.6s
list one, just 'id' field: 0.2s

Now with this change:

list all: 14s
list all, just 'id' field: 0.04s
list one: 0.6s
list one, just 'id' field: 0.03s

Closes-Bug: #1810563
Change-Id: I15df276ba7dbcb3763ab20b63b26cddf2d594954
(cherry picked from commit 1e9086f6e2)
This commit is contained in:
Doug Wiegley 2019-03-03 10:42:16 -07:00
parent 8915febaaf
commit d075028146
No known key found for this signature in database
GPG Key ID: 4D3C112B76BBDB5F
5 changed files with 94 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
neutron/db/models/securitygroup.py
View File

@ -88,7 +88,7 @@ class SecurityGroupRule(standard_attr.HasStandardAttributes, model_base.BASEV2,
remote_ip_prefix = sa.Column(sa.String(255))
security_group = orm.relationship(
SecurityGroup, load_on_pending=True,
backref=orm.backref('rules', cascade='all,delete', lazy='subquery'),
backref=orm.backref('rules', cascade='all,delete', lazy='dynamic'),
primaryjoin="SecurityGroup.id==SecurityGroupRule.security_group_id")
source_group = orm.relationship(
SecurityGroup,

61
neutron/objects/base.py
View File

@ -32,6 +32,7 @@ from sqlalchemy import orm
from neutron._i18n import _
from neutron.db import api as db_api
from neutron.db import standard_attr
from neutron.objects import common_types
from neutron.objects.db import api as obj_db_api
from neutron.objects.extensions import standardattributes
@ -62,6 +63,25 @@ def register_filter_hook_on_model(model, filter_name):
obj_class.add_extra_filter_name(filter_name)
class LazyQueryIterator(six.Iterator):
def __init__(self, obj_class, lazy_query):
self.obj_class = obj_class
self.context = None
self.query = lazy_query
def __iter__(self):
self.results = self.query.all()
self.i = 0
return self
def __next__(self):
if self.i >= len(self.results):
raise StopIteration()
item = self.obj_class._load_object(self.context, self.results[self.i])
self.i += 1
return item
class Pager(object):
'''
This class represents a pager object. It is consumed by get_objects to
@ -136,6 +156,11 @@ class NeutronObject(obj_base.VersionedObject,
synthetic_fields = []
extra_filter_names = set()
# To use lazy queries for child objects, you must set the ORM
# relationship in the db model to 'dynamic'. By default, all
# children are eager loaded.
lazy_fields = set()
def __init__(self, context=None, **kwargs):
super(NeutronObject, self).__init__(context, **kwargs)
self._load_synthetic_fields = True
@ -161,7 +186,8 @@ class NeutronObject(obj_base.VersionedObject,
dict_[name] = value
for field_name, value in self._synthetic_fields_items():
field = self.fields[field_name]
if isinstance(field, obj_fields.ListOfObjectsField):
if (isinstance(field, obj_fields.ListOfObjectsField) or
isinstance(field, common_types.ListOfObjectsField)):
dict_[field_name] = [obj.to_dict() for obj in value]
elif isinstance(field, obj_fields.ObjectField):
dict_[field_name] = (
@ -177,7 +203,8 @@ class NeutronObject(obj_base.VersionedObject,
@classmethod
def is_object_field(cls, field):
return (isinstance(cls.fields[field], obj_fields.ListOfObjectsField) or
isinstance(cls.fields[field], obj_fields.ObjectField))
isinstance(cls.fields[field], common_types.ListOfObjectsField) or
isinstance(cls.fields[field], obj_fields.ObjectField))
@classmethod
def obj_class_from_name(cls, objname, objver):
@ -440,8 +467,15 @@ class NeutronDbObject(NeutronObject):
'''Return a database model that persists object data.'''
return self._captured_db_model
def _set_lazy_contexts(self, fields, context):
for field in self.lazy_fields.intersection(fields):
if isinstance(fields[field], LazyQueryIterator):
fields[field].context = context
def from_db_object(self, db_obj):
fields = self.modify_fields_from_db(db_obj)
if self.lazy_fields:
self._set_lazy_contexts(fields, self.obj_context)
for field in self.fields:
if field in fields and not self.is_synthetic(field):
setattr(self, field, fields[field])
@ -470,12 +504,23 @@ class NeutronDbObject(NeutronObject):
:param fields: dict of fields from NeutronDbObject
:return: modified dict of fields
"""
for k, v in fields.items():
if isinstance(v, LazyQueryIterator):
fields[k] = list(v)
result = copy.deepcopy(dict(fields))
for field, field_db in cls.fields_need_translation.items():
if field in result:
result[field_db] = result.pop(field)
return result
@classmethod
def _get_lazy_iterator(cls, field, appender_query):
if field not in cls.lazy_fields:
raise KeyError(_('Field %s is not a lazy query field') % field)
n_obj_classes = NeutronObjectRegistry.obj_classes()
n_obj = n_obj_classes.get(cls.fields[field].objname)
return LazyQueryIterator(n_obj[0], appender_query)
@classmethod
def modify_fields_from_db(cls, db_obj):
"""Modify the fields after data were fetched from DB.
@ -501,6 +546,8 @@ class NeutronDbObject(NeutronObject):
# don't allow sqlalchemy lists to propagate outside
if isinstance(v, orm.collections.InstrumentedList):
result[k] = list(v)
if isinstance(v, orm.dynamic.AppenderQuery):
result[k] = cls._get_lazy_iterator(k, v)
return result
@classmethod
@ -513,7 +560,8 @@ class NeutronDbObject(NeutronObject):
obj.from_db_object(db_obj)
# detach the model so that consequent fetches don't reuse it
context.session.expunge(obj.db_obj)
if context is not None:
context.session.expunge(obj.db_obj)
return obj
def obj_load_attr(self, attrname):
@ -744,8 +792,9 @@ class NeutronDbObject(NeutronObject):
# subclasses=True
for field in self.synthetic_fields:
try:
field_def = self.fields[field]
objclasses = NeutronObjectRegistry.obj_classes(
).get(self.fields[field].objname)
).get(field_def.objname)
except AttributeError:
# NOTE(rossella_s) this is probably because this field is not
# an ObjectField
@ -772,7 +821,9 @@ class NeutronDbObject(NeutronObject):
# synth_db_objs can be list, empty list or None, that is why
# we need 'is not None', because [] is valid case for 'True'
if synth_db_objs is not None:
if not isinstance(synth_db_objs, list):
if isinstance(synth_db_objs, orm.dynamic.AppenderQuery):
synth_db_objs = list(synth_db_objs)
elif not isinstance(synth_db_objs, list):
synth_db_objs = [synth_db_objs]
synth_objs = [objclass._load_object(self.obj_context, obj)
for obj in synth_db_objs]

23
neutron/objects/common_types.py
View File

@ -11,6 +11,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import collections
import itertools
import uuid
@ -313,3 +314,25 @@ class FloatingIPStatusEnumField(obj_fields.AutoTypedField):
class RouterStatusEnumField(obj_fields.AutoTypedField):
AUTO_TYPE = obj_fields.Enum(valid_values=constants.VALID_ROUTER_STATUS)
# Duplicate some fixes in later oslo.versionedobjects, so we can backport
# fixes without modifying requirements.
class List(obj_fields.List):
def coerce(self, obj, attr, value):
if (not isinstance(value, collections.Iterable) or
isinstance(value, six.string_types + (collections.Mapping,))):
raise ValueError(_('A list is required in field %(attr)s, '
'not a %(type)s') %
{'attr': attr, 'type': type(value).__name__})
coerced_list = obj_fields.CoercedList()
coerced_list.enable_coercing(self._element_type, obj, attr)
coerced_list.extend(value)
return coerced_list
class ListOfObjectsField(obj_fields.AutoTypedField):
def __init__(self, objtype, subclasses=False, **kwargs):
self.AUTO_TYPE = List(obj_fields.Object(objtype, subclasses))
self.objname = objtype
super(ListOfObjectsField, self).__init__(**kwargs)

4
neutron/objects/securitygroup.py
View File

@ -30,7 +30,7 @@ class SecurityGroup(base.NeutronDbObject):
'name': obj_fields.StringField(nullable=True),
'project_id': obj_fields.StringField(nullable=True),
'is_default': obj_fields.BooleanField(default=False),
'rules': obj_fields.ListOfObjectsField(
'rules': common_types.ListOfObjectsField(
'SecurityGroupRule', nullable=True
),
# NOTE(ihrachys): we don't include source_rules that is present in the
@ -43,6 +43,8 @@ class SecurityGroup(base.NeutronDbObject):
extra_filter_names = {'is_default'}
lazy_fields = set(['rules'])
def create(self):
# save is_default before super() resets it to False
is_default = self.is_default

13
neutron/tests/unit/objects/test_base.py
View File

@ -29,6 +29,7 @@ from oslo_utils import uuidutils
from oslo_versionedobjects import base as obj_base
from oslo_versionedobjects import exception
from oslo_versionedobjects import fields as obj_fields
from sqlalchemy import orm
import testtools
from neutron.db import _model_query as model_query
@ -484,6 +485,7 @@ FIELD_TYPE_VALUE_GENERATOR_MAP = {
common_types.IPVersionEnumField: tools.get_random_ip_version,
common_types.IpProtocolEnumField: tools.get_random_ip_protocol,
common_types.ListOfIPNetworksField: get_list_of_random_networks,
common_types.ListOfObjectsField: lambda: [],
common_types.MACAddressField: tools.get_random_EUI,
common_types.PortBindingStatusEnumField:
tools.get_random_port_binding_statuses,
@ -1217,7 +1219,10 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
self.get_random_db_fields(obj_cls=objclass))
)
child_dict = child.to_dict()
if isinstance(cls_.fields[field], obj_fields.ListOfObjectsField):
if (isinstance(cls_.fields[field],
obj_fields.ListOfObjectsField) or
isinstance(cls_.fields[field],
common_types.ListOfObjectsField)):
setattr(obj, field, [child])
dict_ = obj.to_dict()
self.assertEqual([child_dict], dict_[field])
@ -1986,7 +1991,11 @@ class BaseDbObjectTestCase(_BaseObjectTestCase,
obj.update()
self.assertIsNotNone(obj.db_obj)
for k, v in obj.modify_fields_to_db(fields_to_update).items():
self.assertEqual(v, obj.db_obj[k], '%s attribute differs' % k)
if isinstance(obj.db_obj[k], orm.dynamic.AppenderQuery):
self.assertIsInstance(v, list)
else:
self.assertEqual(v, obj.db_obj[k],
'%s attribute differs' % k)
obj.delete()
self.assertIsNone(obj.db_obj)