From 397a6fe39c72ac091d53da610682ec4f95ade2ed Mon Sep 17 00:00:00 2001 From: Jan Zerebecki Date: Mon, 14 May 2018 13:23:06 +0200 Subject: [PATCH] Improve speed of listing from DB This removes from_sqlalchemy_model and the related object intermediary representation from get_loabalancers, get_pools, get_listeners, get_healthmonitors, get_l7policies, get_l7policy_rules, get_pool_members. Instead it more directly transforms from the SQLAlchemy model to the required api dictionary format. Thus this entierly skips loading a few relations that walking the intermediary object model triggered without the target output needing them. Use eager loading specified at query time where the api dictionary representation is needed, as the relations needed for it are always the same. Removes eager loading from the model of the relations L7Policy[rules]->L7Rule[policy] and Listener[l7_policies]->L7Policy[listener] and Listener[default_pool]->PoolV2[listeners]. These relations are sometimes unncessarily loaded and then never used. E.g. the API dictionary format for loadbalancers does include policies but not rules; does include listeners but not any L7Policy. In a test with a bit more than 100 LBs this improved the speed of a cli call of lbaas-loadbalancer-list from 50.932s to 2.335s. It similarly improved lbaas-pool-list and lbaas-listener-list, lbaas-healthmonitor-list, lbaas-l7policy-list, lbaas-l7rule-list, lbaas-member-list. This continues what I9d67f0966561baaefb50ae97b943ff6593e194eb started. Story: 2001962 Change-Id: I32328c5206b9cd6fb8d8764c079f22b6ea8bfa9e --- .../db/loadbalancer/loadbalancer_dbv2.py | 88 +++++++- neutron_lbaas/db/loadbalancer/models.py | 188 +++++++++++++++++- neutron_lbaas/services/loadbalancer/plugin.py | 27 ++- .../db/loadbalancer/test_db_loadbalancerv2.py | 15 +- 4 files changed, 293 insertions(+), 25 deletions(-) diff --git a/neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py b/neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py index 5d926c61b..d32cf4887 100644 --- a/neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py +++ b/neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py @@ -33,6 +33,7 @@ from oslo_utils import uuidutils from sqlalchemy import orm from sqlalchemy.orm import exc from sqlalchemy.orm import lazyload +from sqlalchemy.orm import subqueryload from neutron_lbaas._i18n import _ from neutron_lbaas import agent_scheduler @@ -93,9 +94,11 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin, return False return True - def _get_resources(self, context, model, filters=None): + def _get_resources(self, context, model, filters=None, options=None): query = self._get_collection_query(context, model, filters=filters) + if options: + query = query.options(options) return [model_instance for model_instance in query] def _create_port_choose_fixed_ip(self, fixed_ips): @@ -381,7 +384,7 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin, return if port_db['device_owner'] == n_const.DEVICE_OWNER_LOADBALANCERV2: filters = {'vip_port_id': [port_id]} - if len(self.get_loadbalancers(context, filters=filters)) > 0: + if len(self.get_loadbalancer_ids(context, filters=filters)) > 0: reason = _('has device owner %s') % port_db['device_owner'] raise n_exc.ServicePortInUse(port_id=port_db['id'], reason=reason) @@ -391,12 +394,29 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin, _prevent_lbaasv2_port_delete_callback, resources.PORT, events.BEFORE_DELETE) + def get_loadbalancer_ids(self, context, filters=None): + lb_dbs = self._get_resources(context, models.LoadBalancer, + filters=filters) + return [lb_db.id + for lb_db in lb_dbs] + def get_loadbalancers(self, context, filters=None): lb_dbs = self._get_resources(context, models.LoadBalancer, filters=filters) return [data_models.LoadBalancer.from_sqlalchemy_model(lb_db) for lb_db in lb_dbs] + def get_loadbalancers_as_api_dict(self, context, filters=None): + options = ( + subqueryload(models.LoadBalancer.listeners), + subqueryload(models.LoadBalancer.pools), + subqueryload(models.LoadBalancer.provider) + ) + lb_dbs = self._get_resources(context, models.LoadBalancer, + filters=filters, options=options) + return [lb_db.to_api_dict + for lb_db in lb_dbs] + def get_provider_names_used_in_loadbalancers(self, context): lb_dbs = self._get_resources(context, models.LoadBalancer) return [lb_db.provider.provider_name for lb_db in lb_dbs] @@ -578,6 +598,17 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin, return [data_models.Listener.from_sqlalchemy_model(listener_db) for listener_db in listener_dbs] + def get_listeners_as_api_dict(self, context, filters=None): + options = ( + subqueryload(models.Listener.sni_containers), + subqueryload(models.Listener.loadbalancer), + subqueryload(models.Listener.l7_policies) + ) + listener_dbs = self._get_resources(context, models.Listener, + filters=filters, options=options) + return [listener_db.to_api_dict + for listener_db in listener_dbs] + def get_listener(self, context, id): listener_db = self._get_resource(context, models.Listener, id) return data_models.Listener.from_sqlalchemy_model(listener_db) @@ -695,6 +726,19 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin, return [data_models.Pool.from_sqlalchemy_model(pool_db) for pool_db in pool_dbs] + def get_pools_as_api_dict(self, context, filters=None): + options = ( + subqueryload(models.PoolV2.members), + subqueryload(models.PoolV2.listeners), + subqueryload(models.PoolV2.l7_policies), + subqueryload(models.PoolV2.loadbalancer), + subqueryload(models.PoolV2.session_persistence) + ) + pool_dbs = self._get_resources(context, models.PoolV2, + filters=filters, options=options) + return [pool_db.to_api_dict + for pool_db in pool_dbs] + def get_pool(self, context, id): pool_db = self._get_resource(context, models.PoolV2, id) return data_models.Pool.from_sqlalchemy_model(pool_db) @@ -734,6 +778,13 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin, return [data_models.Member.from_sqlalchemy_model(member_db) for member_db in member_dbs] + def get_pool_members_as_api_dict(self, context, filters=None): + filters = filters or {} + member_dbs = self._get_resources(context, models.MemberV2, + filters=filters) + return [member_db.to_api_dict + for member_db in member_dbs] + def get_pool_member(self, context, id): member_db = self._get_resource(context, models.MemberV2, id) return data_models.Member.from_sqlalchemy_model(member_db) @@ -792,6 +843,16 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin, return [data_models.HealthMonitor.from_sqlalchemy_model(hm_db) for hm_db in hm_dbs] + def get_healthmonitors_as_api_dict(self, context, filters=None): + options = ( + subqueryload(models.HealthMonitorV2.pool) + ) + filters = filters or {} + hm_dbs = self._get_resources(context, models.HealthMonitorV2, + filters=filters, options=options) + return [hm_db.to_api_dict + for hm_db in hm_dbs] + def update_loadbalancer_stats(self, context, loadbalancer_id, stats_data): stats_data = stats_data or {} with context.session.begin(subtransactions=True): @@ -889,6 +950,15 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin, return [data_models.L7Policy.from_sqlalchemy_model(l7policy_db) for l7policy_db in l7policy_dbs] + def get_l7policies_as_api_dict(self, context, filters=None): + options = ( + subqueryload(models.L7Policy.rules) + ) + l7policy_dbs = self._get_resources(context, models.L7Policy, + filters=filters, options=options) + return [l7policy_db.to_api_dict + for l7policy_db in l7policy_dbs] + def create_l7policy_rule(self, context, rule, l7policy_id): with context.session.begin(subtransactions=True): if not self._resource_exists(context, models.L7Policy, @@ -948,6 +1018,20 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin, return [data_models.L7Rule.from_sqlalchemy_model(rule_db) for rule_db in rule_dbs] + def get_l7policy_rules_as_api_dict( + self, context, l7policy_id, filters=None): + options = ( + subqueryload(models.L7Rule.policy) + ) + if filters: + filters.update(filters) + else: + filters = {'l7policy_id': [l7policy_id]} + rule_dbs = self._get_resources(context, models.L7Rule, + filters=filters, options=options) + return [rule_db.to_api_dict + for rule_db in rule_dbs] + def _prevent_lbaasv2_port_delete_callback(resource, event, trigger, **kwargs): context = kwargs['context'] diff --git a/neutron_lbaas/db/loadbalancer/models.py b/neutron_lbaas/db/loadbalancer/models.py index 5ded176e2..6de66562d 100644 --- a/neutron_lbaas/db/loadbalancer/models.py +++ b/neutron_lbaas/db/loadbalancer/models.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import six + from neutron.db.models import servicetype as st_db from neutron.db import models_v2 from neutron_lib.db import constants as db_const @@ -94,6 +96,24 @@ class MemberV2(model_base.BASEV2, model_base.HasId, model_base.HasProject): def root_loadbalancer(self): return self.pool.loadbalancer + @property + def to_api_dict(self): + def to_dict(sa_model, attributes): + ret = {} + for attr in attributes: + value = getattr(sa_model, attr) + if six.PY2 and isinstance(value, six.text_type): + ret[attr.encode('utf8')] = value.encode('utf8') + else: + ret[attr] = value + return ret + + ret_dict = to_dict(self, [ + 'id', 'tenant_id', 'pool_id', 'address', 'protocol_port', 'weight', + 'admin_state_up', 'subnet_id', 'name']) + + return ret_dict + class HealthMonitorV2(model_base.BASEV2, model_base.HasId, model_base.HasProject): @@ -121,6 +141,34 @@ class HealthMonitorV2(model_base.BASEV2, model_base.HasId, def root_loadbalancer(self): return self.pool.loadbalancer + @property + def to_api_dict(self): + def to_dict(sa_model, attributes): + ret = {} + for attr in attributes: + value = getattr(sa_model, attr) + if six.PY2 and isinstance(value, six.text_type): + ret[attr.encode('utf8')] = value.encode('utf8') + else: + ret[attr] = value + return ret + + ret_dict = to_dict(self, [ + 'id', 'tenant_id', 'type', 'delay', 'timeout', 'max_retries', + 'http_method', 'url_path', 'expected_codes', 'admin_state_up', + 'name', 'max_retries_down']) + + ret_dict['pools'] = [] + if self.pool: + ret_dict['pools'].append({'id': self.pool.id}) + if self.type in [lb_const.HEALTH_MONITOR_TCP, + lb_const.HEALTH_MONITOR_PING]: + ret_dict.pop('http_method') + ret_dict.pop('url_path') + ret_dict.pop('expected_codes') + + return ret_dict + class LoadBalancer(model_base.BASEV2, model_base.HasId, model_base.HasProject): """Represents a v2 neutron load balancer.""" @@ -168,6 +216,34 @@ class LoadBalancer(model_base.BASEV2, model_base.HasId, model_base.HasProject): def root_loadbalancer(self): return self + @property + def to_api_dict(self): + def to_dict(sa_model, attributes): + ret = {} + for attr in attributes: + value = getattr(sa_model, attr) + if six.PY2 and isinstance(value, six.text_type): + ret[attr.encode('utf8')] = value.encode('utf8') + else: + ret[attr] = value + return ret + + ret_dict = to_dict(self, [ + 'id', 'tenant_id', 'name', 'description', + 'vip_subnet_id', 'vip_port_id', 'vip_address', 'operating_status', + 'provisioning_status', 'admin_state_up', 'flavor_id']) + ret_dict['listeners'] = [{'id': listener.id} + for listener in self.listeners] + ret_dict['pools'] = [{'id': pool.id} for pool in self.pools] + + if self.provider: + ret_dict['provider'] = self.provider.provider_name + + if not self.flavor_id: + del ret_dict['flavor_id'] + + return ret_dict + class PoolV2(model_base.BASEV2, model_base.HasId, model_base.HasProject): """Represents a v2 neutron load balancer pool.""" @@ -221,6 +297,41 @@ class PoolV2(model_base.BASEV2, model_base.HasId, model_base.HasProject): else: return None + @property + def to_api_dict(self): + def to_dict(sa_model, attributes): + ret = {} + for attr in attributes: + value = getattr(sa_model, attr) + if six.PY2 and isinstance(value, six.text_type): + ret[attr.encode('utf8')] = value.encode('utf8') + else: + ret[attr] = value + return ret + + ret_dict = to_dict(self, [ + 'id', 'tenant_id', 'name', 'description', + 'healthmonitor_id', 'protocol', 'lb_algorithm', 'admin_state_up']) + + ret_dict['loadbalancers'] = [] + if self.loadbalancer: + ret_dict['loadbalancers'].append({'id': self.loadbalancer.id}) + ret_dict['session_persistence'] = None + if self.session_persistence: + ret_dict['session_persistence'] = ( + to_dict(self.session_persistence, [ + 'type', 'cookie_name'])) + ret_dict['members'] = [{'id': member.id} for member in self.members] + ret_dict['listeners'] = [{'id': listener.id} + for listener in self.listeners] + if self.listener: + ret_dict['listener_id'] = self.listener.id + else: + ret_dict['listener_id'] = None + ret_dict['l7_policies'] = [{'id': l7_policy.id} + for l7_policy in self.l7_policies] + return ret_dict + class SNI(model_base.BASEV2): @@ -272,6 +383,27 @@ class L7Rule(model_base.BASEV2, model_base.HasId, model_base.HasProject): def root_loadbalancer(self): return self.policy.listener.loadbalancer + @property + def to_api_dict(self): + def to_dict(sa_model, attributes): + ret = {} + for attr in attributes: + value = getattr(sa_model, attr) + if six.PY2 and isinstance(value, six.text_type): + ret[attr.encode('utf8')] = value.encode('utf8') + else: + ret[attr] = value + return ret + + ret_dict = to_dict(self, [ + 'id', 'tenant_id', 'type', 'compare_type', 'invert', 'key', + 'value', 'admin_state_up']) + + ret_dict['policies'] = [] + if self.policy: + ret_dict['policies'].append({'id': self.policy.id}) + return ret_dict + class L7Policy(model_base.BASEV2, model_base.HasId, model_base.HasProject): """Represents L7 Policy.""" @@ -299,7 +431,6 @@ class L7Policy(model_base.BASEV2, model_base.HasId, model_base.HasProject): rules = orm.relationship( L7Rule, uselist=True, - lazy="joined", primaryjoin="L7Policy.id==L7Rule.l7policy_id", foreign_keys=[L7Rule.l7policy_id], cascade="all, delete-orphan", @@ -312,6 +443,29 @@ class L7Policy(model_base.BASEV2, model_base.HasId, model_base.HasProject): def root_loadbalancer(self): return self.listener.loadbalancer + @property + def to_api_dict(self): + def to_dict(sa_model, attributes): + ret = {} + for attr in attributes: + value = getattr(sa_model, attr) + if six.PY2 and isinstance(value, six.text_type): + ret[attr.encode('utf8')] = value.encode('utf8') + else: + ret[attr] = value + return ret + + ret_dict = to_dict(self, [ + 'id', 'tenant_id', 'name', 'description', 'listener_id', 'action', + 'redirect_pool_id', 'redirect_url', 'position', 'admin_state_up']) + + ret_dict['listeners'] = [{'id': self.listener_id}] + ret_dict['rules'] = [{'id': rule.id} for rule in self.rules] + if (ret_dict.get('action') == + lb_const.L7_POLICY_ACTION_REDIRECT_TO_POOL): + del ret_dict['redirect_url'] + return ret_dict + class Listener(model_base.BASEV2, model_base.HasId, model_base.HasProject): """Represents a v2 neutron listener.""" @@ -353,14 +507,13 @@ class Listener(model_base.BASEV2, model_base.HasId, model_base.HasProject): provisioning_status = sa.Column(sa.String(16), nullable=False) operating_status = sa.Column(sa.String(16), nullable=False) default_pool = orm.relationship( - PoolV2, backref=orm.backref("listeners"), lazy='joined') + PoolV2, backref=orm.backref("listeners")) loadbalancer = orm.relationship( LoadBalancer, backref=orm.backref("listeners", uselist=True)) l7_policies = orm.relationship( L7Policy, uselist=True, - lazy="joined", primaryjoin="Listener.id==L7Policy.listener_id", order_by="L7Policy.position", collection_class=orderinglist.ordering_list('position', count_from=1), @@ -371,3 +524,32 @@ class Listener(model_base.BASEV2, model_base.HasId, model_base.HasProject): @property def root_loadbalancer(self): return self.loadbalancer + + @property + def to_api_dict(self): + def to_dict(sa_model, attributes): + ret = {} + for attr in attributes: + value = getattr(sa_model, attr) + if six.PY2 and isinstance(value, six.text_type): + ret[attr.encode('utf8')] = value.encode('utf8') + else: + ret[attr] = value + return ret + + ret_dict = to_dict(self, [ + 'id', 'tenant_id', 'name', 'description', 'default_pool_id', + 'protocol', 'default_tls_container_id', 'protocol_port', + 'connection_limit', 'admin_state_up']) + + # NOTE(blogan): Returning a list to future proof for M:N objects + # that are not yet implemented. + ret_dict['loadbalancers'] = [] + if self.loadbalancer: + ret_dict['loadbalancers'].append({'id': self.loadbalancer.id}) + ret_dict['sni_container_refs'] = [container.tls_container_id + for container in self.sni_containers] + ret_dict['default_tls_container_ref'] = self.default_tls_container_id + ret_dict['l7policies'] = [{'id': l7_policy.id} + for l7_policy in self.l7_policies] + return ret_dict diff --git a/neutron_lbaas/services/loadbalancer/plugin.py b/neutron_lbaas/services/loadbalancer/plugin.py index 4df89126f..6c84ec47e 100644 --- a/neutron_lbaas/services/loadbalancer/plugin.py +++ b/neutron_lbaas/services/loadbalancer/plugin.py @@ -462,8 +462,7 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2, return self.db.get_loadbalancer(context, id).to_api_dict() def get_loadbalancers(self, context, filters=None, fields=None): - return [loadbalancer.to_api_dict() for loadbalancer in - self.db.get_loadbalancers(context, filters=filters)] + return self.db.get_loadbalancers_as_api_dict(context, filters=filters) def _validate_tls(self, listener, curr_listener=None): def validate_tls_container(container_ref): @@ -662,8 +661,8 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2, return self.db.get_listener(context, id).to_api_dict() def get_listeners(self, context, filters=None, fields=None): - return [listener.to_api_dict() for listener in self.db.get_listeners( - context, filters=filters)] + return self.db.get_listeners_as_api_dict( + context, filters=filters) def create_pool(self, context, pool): pool = pool.get('pool') @@ -758,8 +757,8 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2, self._call_driver_operation(context, driver.pool.delete, db_pool) def get_pools(self, context, filters=None, fields=None): - return [pool.to_api_dict() for pool in self.db.get_pools( - context, filters=filters)] + return self.db.get_pools_as_api_dict( + context, filters=filters) def get_pool(self, context, id, fields=None): return self.db.get_pool(context, id).to_api_dict() @@ -830,8 +829,8 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2, if not filters: filters = {} filters['pool_id'] = [pool_id] - return [mem.to_api_dict() for mem in self.db.get_pool_members( - context, filters=filters)] + return self.db.get_pool_members_as_api_dict( + context, filters=filters) def get_pool_member(self, context, id, pool_id, fields=None): self._check_pool_exists(context, pool_id) @@ -902,8 +901,8 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2, return self.db.get_healthmonitor(context, id).to_api_dict() def get_healthmonitors(self, context, filters=None, fields=None): - return [hm.to_api_dict() for hm in self.db.get_healthmonitors( - context, filters=filters)] + return self.db.get_healthmonitors_as_api_dict( + context, filters=filters) def stats(self, context, loadbalancer_id): lb = self.db.get_loadbalancer(context, loadbalancer_id) @@ -973,8 +972,8 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2, self.db.delete_l7policy(context, id) def get_l7policies(self, context, filters=None, fields=None): - return [policy.to_api_dict() for policy in self.db.get_l7policies( - context, filters=filters)] + return self.db.get_l7policies_as_api_dict( + context, filters=filters) def get_l7policy(self, context, id, fields=None): return self.db.get_l7policy(context, id).to_api_dict() @@ -1047,8 +1046,8 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2, def get_l7policy_rules(self, context, l7policy_id, filters=None, fields=None): self._check_l7policy_exists(context, l7policy_id) - return [rule.to_api_dict() for rule in self.db.get_l7policy_rules( - context, l7policy_id, filters=filters)] + return self.db.get_l7policy_rules_as_api_dict( + context, l7policy_id, filters=filters) def get_l7policy_rule(self, context, id, l7policy_id, fields=None): self._check_l7policy_exists(context, l7policy_id) diff --git a/neutron_lbaas/tests/unit/db/loadbalancer/test_db_loadbalancerv2.py b/neutron_lbaas/tests/unit/db/loadbalancer/test_db_loadbalancerv2.py index 201c25410..76c7f9efb 100644 --- a/neutron_lbaas/tests/unit/db/loadbalancer/test_db_loadbalancerv2.py +++ b/neutron_lbaas/tests/unit/db/loadbalancer/test_db_loadbalancerv2.py @@ -509,11 +509,10 @@ class LbaasLoadBalancerTests(LbaasPluginDbTestCase): } ctx = context.get_admin_context() port['device_owner'] = n_constants.DEVICE_OWNER_LOADBALANCERV2 - myloadbalancers = [{'name': 'lb1'}] plugin = mock.Mock() directory.add_plugin(constants.CORE, plugin) - self.plugin.db.get_loadbalancers = ( - mock.Mock(return_value=myloadbalancers)) + self.plugin.db.get_loadbalancer_ids = ( + mock.Mock(return_value=['1'])) plugin._get_port.return_value = port self.assertRaises(n_exc.ServicePortInUse, self.plugin.db.prevent_lbaasv2_port_deletion, @@ -1722,8 +1721,9 @@ class LbaasL7Tests(ListenerTestBase): self.l7policy(listener_id, position=8, name="8"), \ self.l7policy(listener_id, position=1, name="9"), \ self.l7policy(listener_id, position=1, name="10"): + c = context.get_admin_context() listener_db = self.plugin.db._get_resource( - context.get_admin_context(), + c, models.Listener, listener['listener']['id']) names = ['10', '9', '4', '5', '1', '6', '2', '3', '7', '8'] for pos in range(0, 10): @@ -1835,8 +1835,10 @@ class LbaasL7Tests(ListenerTestBase): lb_const.OFFLINE) self.plugin.update_l7policy(c, p10['l7policy']['id'], {'l7policy': expected}) + + c2 = context.get_admin_context() listener_db = self.plugin.db._get_resource( - context.get_admin_context(), + c2, models.Listener, listener['listener']['id']) names = ['1', '3', '10', '6', '4', '7', '8', '9', '5', '2'] for pos in range(0, 10): @@ -1877,8 +1879,9 @@ class LbaasL7Tests(ListenerTestBase): lb_const.OFFLINE) self.plugin.delete_l7policy(c, p5['l7policy']['id']) + c2 = context.get_admin_context() listener_db = self.plugin.db._get_resource( - context.get_admin_context(), + c2, models.Listener, listener['listener']['id']) names = ['0', '1', '2', '4', '6'] for pos in range(0, 4):