From 1e9086f6e269f32d1cb19a0f55eb1f5dd73ca651 Mon Sep 17 00:00:00 2001 From: Doug Wiegley Date: Sun, 3 Mar 2019 10:42:16 -0700 Subject: [PATCH] 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 --- lower-constraints.txt | 2 +- neutron/db/models/securitygroup.py | 2 +- neutron/objects/base.py | 44 +++++++++++++++++++++++++ neutron/objects/securitygroup.py | 2 ++ neutron/tests/unit/objects/test_base.py | 7 +++- requirements.txt | 2 +- 6 files changed, 55 insertions(+), 4 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index 43dad768fb1..2179068eba8 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -81,7 +81,7 @@ oslo.serialization==2.18.0 oslo.service==1.24.0 oslo.upgradecheck==0.1.0 oslo.utils==3.33.0 -oslo.versionedobjects==1.31.2 +oslo.versionedobjects==1.35.1 oslosphinx==4.7.0 oslotest==3.2.0 osprofiler==1.4.0 diff --git a/neutron/db/models/securitygroup.py b/neutron/db/models/securitygroup.py index b7b0ce1a744..239103d8c3f 100644 --- a/neutron/db/models/securitygroup.py +++ b/neutron/db/models/securitygroup.py @@ -93,7 +93,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, diff --git a/neutron/objects/base.py b/neutron/objects/base.py index 97af381ff1d..4e98c0affbc 100644 --- a/neutron/objects/base.py +++ b/neutron/objects/base.py @@ -55,6 +55,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): '''Pager class @@ -131,6 +150,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 @@ -428,8 +452,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]) @@ -459,12 +490,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. @@ -490,6 +532,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 diff --git a/neutron/objects/securitygroup.py b/neutron/objects/securitygroup.py index 85c5a3a635b..702f406b596 100644 --- a/neutron/objects/securitygroup.py +++ b/neutron/objects/securitygroup.py @@ -60,6 +60,8 @@ class SecurityGroup(rbac_db.NeutronRbacObject): 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 diff --git a/neutron/tests/unit/objects/test_base.py b/neutron/tests/unit/objects/test_base.py index c657f26a495..387cec5a5c7 100644 --- a/neutron/tests/unit/objects/test_base.py +++ b/neutron/tests/unit/objects/test_base.py @@ -32,6 +32,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 import objects @@ -2126,7 +2127,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) diff --git a/requirements.txt b/requirements.txt index c4f90c0c893..5b425e37118 100644 --- a/requirements.txt +++ b/requirements.txt @@ -41,7 +41,7 @@ oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.service!=1.28.1,>=1.24.0 # Apache-2.0 oslo.upgradecheck>=0.1.0 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0 -oslo.versionedobjects>=1.31.2 # Apache-2.0 +oslo.versionedobjects>=1.35.1 # Apache-2.0 osprofiler>=1.4.0 # Apache-2.0 os-ken >= 0.3.0 # Apache-2.0 ovs>=2.8.0 # Apache-2.0