Change to use selectin for DB load strategy

During a mailing list discussion on some OOM issues neutron has been seeing [0], Mike Bayer recommended we should change from using subquery to selectin DB load strategy. A full description of this strategy can be found here [1], but in short: - “subquery” loading incurs additional performance / complexity issues when used on a many-levels-deep eager load, as subqueries will be nested repeatedly. - "The subqueryload() eager loader is mostly legacy at this point, superseded by selectinload() - "The only scenario in which selectin eager loading is not feasible is when the model is using composite primary keys, and the backend database does not support tuples with IN, which currently includes SQL Server." So that does not apply to us. The plan agreed to at the neutron drivers meeting [2] was to make this change early in the cycle so we would be able to see if there were any issues through the D cycle. Added hacking checks so new code using subquery loads is not added back. [0] https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.org/thread/EHLQQXNG3NLLZYPDGG2ES3DINIJ7YT3N/ [1] https://docs.sqlalchemy.org/en/20/orm/queryguide/relationships.html#selectin-eager-loading [2] https://meetings.opendev.org/meetings/neutron_drivers/2024/neutron_drivers.2024-05-31-14.00.log.html#l-67 Closes-bug: #2067770 Depends-on: https://review.opendev.org/c/openstack/neutron-lib/+/920936 Change-Id: I6e40a15284da392a3d48d45205a7a5770c14c297
2024-05-31 16:14:32 -04:00 · 2024-05-31 16:14:32 -04:00 · 05fcfef6ce
commit 05fcfef6ce
parent abe8110f53
20 changed files with 59 additions and 30 deletions
--- a/neutron/db/extra_dhcp_opt/models.py
+++ b/neutron/db/extra_dhcp_opt/models.py
@ -42,5 +42,5 @@ class ExtraDhcpOpt(model_base.BASEV2, model_base.HasId):
    # eagerly load extra_dhcp_opts bindings
    ports = orm.relationship(
        models_v2.Port, load_on_pending=True,
-        backref=orm.backref("dhcp_opts", lazy='subquery', cascade='delete'))
+        backref=orm.backref("dhcp_opts", lazy='selectin', cascade='delete'))
    revises_on_change = ('ports', )
--- a/neutron/db/models/address_group.py
+++ b/neutron/db/models/address_group.py
@ -42,7 +42,7 @@ class AddressGroup(standard_attr.HasStandardAttributes,
    addresses = orm.relationship(AddressAssociation,
                                 backref=orm.backref('address_groups',
                                                     load_on_pending=True),
-                                 lazy='subquery',
+                                 lazy='selectin',
                                 cascade='all, delete-orphan')
    rbac_entries = sa.orm.relationship(rbac_db_models.AddressGroupRBAC,
                                       backref='address_groups',
--- a/neutron/db/models/allowed_address_pair.py
+++ b/neutron/db/models/allowed_address_pair.py
@ -27,5 +27,5 @@ class AllowedAddressPair(model_base.BASEV2):
    port = orm.relationship(
        models_v2.Port, load_on_pending=True,
        backref=orm.backref("allowed_address_pairs",
-                            lazy="subquery", cascade="delete"))
+                            lazy="selectin", cascade="delete"))
    revises_on_change = ('port', )
--- a/neutron/db/models/conntrack_helper.py
+++ b/neutron/db/models/conntrack_helper.py
@ -39,7 +39,7 @@ class ConntrackHelper(model_base.BASEV2, model_base.HasId):

    router = orm.relationship(l3.Router, load_on_pending=True,
                              backref=orm.backref("conntrack_helpers",
-                                                  lazy='subquery',
+                                                  lazy='selectin',
                                                  uselist=True,
                                                  cascade='delete'))
    revises_on_change = ('router', )
--- a/neutron/db/models/flavor.py
+++ b/neutron/db/models/flavor.py
@ -41,7 +41,7 @@ class FlavorServiceProfileBinding(model_base.BASEV2):
    flavor = orm.relationship(Flavor,
                              backref=orm.backref(
                                  "service_profiles",
-                                  lazy='subquery',
+                                  lazy='selectin',
                                  cascade="all, delete-orphan"))
    service_profile_id = sa.Column(sa.String(36),
                                   sa.ForeignKey("serviceprofiles.id",
--- a/neutron/db/models/l3.py
+++ b/neutron/db/models/l3.py
@ -58,9 +58,9 @@ class Router(standard_attr.HasStandardAttributes, model_base.BASEV2,
    attached_ports = orm.relationship(
        RouterPort,
        backref=orm.backref('router', load_on_pending=True),
-        lazy='subquery')
+        lazy='selectin')
    l3_agents = orm.relationship(
-        'Agent', lazy='subquery', viewonly=True,
+        'Agent', lazy='selectin', viewonly=True,
        secondary=rb_model.RouterL3AgentBinding.__table__)
    api_collections = [l3_apidef.ROUTERS]
    collection_resource_map = {l3_apidef.ROUTERS: l3_apidef.ROUTER}
@ -120,6 +120,6 @@ class RouterRoute(model_base.BASEV2, models_v2.Route):

    router = orm.relationship(Router, load_on_pending=True,
                              backref=orm.backref("route_list",
-                                                  lazy='subquery',
+                                                  lazy='selectin',
                                                  cascade='delete'))
    revises_on_change = ('router', )
--- a/neutron/db/models/metering.py
+++ b/neutron/db/models/metering.py
@ -38,11 +38,11 @@ class MeteringLabel(model_base.BASEV2,
    name = sa.Column(sa.String(db_const.NAME_FIELD_SIZE))
    description = sa.Column(sa.String(db_const.LONG_DESCRIPTION_FIELD_SIZE))
    rules = orm.relationship(MeteringLabelRule, backref="label",
-                             cascade="delete", lazy="subquery")
+                             cascade="delete", lazy="selectin")
    routers = orm.relationship(
        l3_models.Router,
        primaryjoin="MeteringLabel.project_id==Router.project_id",
        foreign_keys='MeteringLabel.project_id',
-        lazy='subquery',
+        lazy='selectin',
        uselist=True)
    shared = sa.Column(sa.Boolean, default=False, server_default=sql.false())
--- a/neutron/db/models/ndp_proxy.py
+++ b/neutron/db/models/ndp_proxy.py
@ -63,6 +63,6 @@ class RouterNDPProxyState(model_base.BASEV2):
    router = orm.relationship(
        l3.Router, load_on_pending=True,
        backref=orm.backref("ndp_proxy_state",
-                            lazy='subquery', uselist=False,
+                            lazy='selectin', uselist=False,
                            cascade='delete')
    )
--- a/neutron/db/models/port_forwarding.py
+++ b/neutron/db/models/port_forwarding.py
@ -58,13 +58,13 @@ class PortForwarding(standard_attr.HasStandardAttributes,
        models_v2.Port, load_on_pending=True,
        foreign_keys=internal_neutron_port_id,
        backref=orm.backref("port_forwardings",
-                            lazy='subquery', uselist=True,
+                            lazy='selectin', uselist=True,
                            cascade='delete')
    )
    floating_ip = orm.relationship(
        l3.FloatingIP, load_on_pending=True,
        backref=orm.backref("port_forwardings",
-                            lazy='subquery', uselist=True,
+                            lazy='selectin', uselist=True,
                            cascade='delete')
    )
    revises_on_change = ('floating_ip', 'port',)
--- a/neutron/db/models/segment.py
+++ b/neutron/db/models/segment.py
@ -49,7 +49,7 @@ class NetworkSegment(standard_attr.HasStandardAttributes,
                     nullable=True)
    network = orm.relationship(models_v2.Network,
                               backref=orm.backref("segments",
-                                                   lazy='subquery',
+                                                   lazy='selectin',
                                                   cascade='delete'))
    api_collections = [segment.COLLECTION_NAME]

@ -81,6 +81,6 @@ class SegmentHostMapping(model_base.BASEV2):
    network_segment = orm.relationship(
        NetworkSegment, load_on_pending=True,
        backref=orm.backref("segment_host_mapping",
-                            lazy='subquery',
+                            lazy='selectin',
                            cascade='delete'))
    revises_on_change = ('network_segment', )
--- a/neutron/db/models/subnet_service_type.py
+++ b/neutron/db/models/subnet_service_type.py
@ -33,7 +33,7 @@ class SubnetServiceType(model_base.BASEV2):
        length=db_const.DEVICE_OWNER_FIELD_SIZE))
    subnet = orm.relationship(models_v2.Subnet, load_on_pending=True,
                              backref=orm.backref('service_types',
-                                                  lazy='subquery',
+                                                  lazy='selectin',
                                                  cascade='all, delete-orphan',
                                                  uselist=True))
    __table_args__ = (
--- a/neutron/db/models/tag.py
+++ b/neutron/db/models/tag.py
@ -26,6 +26,6 @@ class Tag(model_base.BASEV2):
    tag = sa.Column(sa.String(255), nullable=False, primary_key=True)
    standard_attr = orm.relationship(
        'StandardAttribute', load_on_pending=True,
-        backref=orm.backref('tags', lazy='subquery', viewonly=True),
+        backref=orm.backref('tags', lazy='selectin', viewonly=True),
        sync_backref=False)
    revises_on_change = ('standard_attr', )
--- a/neutron/db/models_v2.py
+++ b/neutron/db/models_v2.py
@ -132,7 +132,7 @@ class Port(standard_attr.HasStandardAttributes, model_base.BASEV2,
    fixed_ips = orm.relationship(IPAllocation,
                                 backref=orm.backref('port',
                                                     load_on_pending=True),
-                                 lazy='subquery',
+                                 lazy='selectin',
                                 cascade='all, delete-orphan',
                                 order_by=(IPAllocation.ip_address,
                                           IPAllocation.subnet_id))
@ -217,24 +217,24 @@ class Subnet(standard_attr.HasStandardAttributes, model_base.BASEV2,
    cidr = sa.Column(sa.String(64), nullable=False)
    gateway_ip = sa.Column(sa.String(64))
    network_standard_attr = orm.relationship(
-        'StandardAttribute', lazy='subquery', viewonly=True,
+        'StandardAttribute', lazy='selectin', viewonly=True,
        secondary='networks', uselist=False,
        load_on_pending=True)
    revises_on_change = ('network_standard_attr', )
    allocation_pools = orm.relationship(IPAllocationPool,
                                        backref='subnet',
-                                        lazy="subquery",
+                                        lazy="selectin",
                                        cascade='delete')
    enable_dhcp = sa.Column(sa.Boolean())
    dns_nameservers = orm.relationship(DNSNameServer,
                                       backref='subnet',
                                       cascade='all, delete, delete-orphan',
                                       order_by=DNSNameServer.order,
-                                       lazy='subquery')
+                                       lazy='selectin')
    routes = orm.relationship(SubnetRoute,
                              backref='subnet',
                              cascade='all, delete, delete-orphan',
-                              lazy='subquery')
+                              lazy='selectin')
    ipv6_ra_mode = sa.Column(sa.Enum(constants.IPV6_SLAAC,
                                     constants.DHCPV6_STATEFUL,
                                     constants.DHCPV6_STATELESS,
@ -299,7 +299,7 @@ class SubnetPool(standard_attr.HasStandardAttributes, model_base.BASEV2,
    prefixes = orm.relationship(SubnetPoolPrefix,
                                backref='subnetpools',
                                cascade='all, delete, delete-orphan',
-                                lazy='subquery')
+                                lazy='selectin')
    rbac_entries = sa.orm.relationship(rbac_db_models.SubnetPoolRBAC,
                                       backref='subnetpools',
                                       lazy='joined',
@ -317,7 +317,7 @@ class Network(standard_attr.HasStandardAttributes, model_base.BASEV2,
    name = sa.Column(sa.String(db_const.NAME_FIELD_SIZE))
    subnets = orm.relationship(
        Subnet,
-        lazy="subquery")
+        lazy="selectin")
    status = sa.Column(sa.String(16))
    admin_state_up = sa.Column(sa.Boolean)
    vlan_transparent = sa.Column(sa.Boolean, nullable=True)
@ -331,7 +331,7 @@ class Network(standard_attr.HasStandardAttributes, model_base.BASEV2,
                    default=constants.DEFAULT_NETWORK_MTU,
                    server_default=str(constants.DEFAULT_NETWORK_MTU))
    dhcp_agents = orm.relationship(
-        'Agent', lazy='subquery', viewonly=True,
+        'Agent', lazy='selectin', viewonly=True,
        secondary=ndab_model.NetworkDhcpAgentBinding.__table__)
    api_collections = [net_def.COLLECTION_NAME]
    collection_resource_map = {net_def.COLLECTION_NAME: net_def.RESOURCE_NAME}
--- a/neutron/db/network_dhcp_agent_binding/models.py
+++ b/neutron/db/network_dhcp_agent_binding/models.py
@ -31,7 +31,7 @@ class NetworkDhcpAgentBinding(model_base.BASEV2):
    network_id = sa.Column(sa.String(36),
                           sa.ForeignKey("networks.id", ondelete='CASCADE'),
                           primary_key=True)
-    dhcp_agent = orm.relationship(agent_model.Agent, lazy='subquery')
+    dhcp_agent = orm.relationship(agent_model.Agent, lazy='selectin')
    dhcp_agent_id = sa.Column(sa.String(36),
                              sa.ForeignKey("agents.id",
                                            ondelete='CASCADE'),
--- a/neutron/hacking/checks.py
+++ b/neutron/hacking/checks.py
@ -44,6 +44,9 @@ import_packaging = re.compile(r"\bimport[\s]+packaging\b")
 import_version_from_packaging = (
    re.compile(r"\bfrom[\s]+packaging[\s]+import[\s]version\b"))

+filter_lazy_subquery = re.compile(r".*lazy=.+subquery")
+filter_subquery_load = re.compile(r".*subqueryload\(")
+

@core.flake8ext
 def check_assert_called_once_with(logical_line, filename):
@ -263,3 +266,17 @@ def check_no_import_packaging(logical_line, filename, noqa):
    for regex in import_packaging, import_version_from_packaging:
        if re.match(regex, logical_line):
            yield (0, msg)
+
+
+@core.flake8ext
+def check_no_sqlalchemy_lazy_subquery(logical_line):
+    """N350 - Use selectin DB load strategy instead of subquery."""
+
+    msg = ("N350: Use selectin DB load strategy instead of "
+           "subquery with sqlalchemy.")
+
+    if filter_lazy_subquery.match(logical_line):
+        yield (0, msg)
+
+    if filter_subquery_load.match(logical_line):
+        yield (0, msg)
--- a/neutron/plugins/ml2/drivers/l2pop/db.py
+++ b/neutron/plugins/ml2/drivers/l2pop/db.py
@ -79,7 +79,7 @@ def _get_active_network_ports(context, network_id):
        agent_model.Agent,
        agent_model.Agent.host == ml2_models.PortBinding.host)
    query = query.join(models_v2.Port)
-    query = query.options(orm.subqueryload(ml2_models.PortBinding.port))
+    query = query.options(orm.selectinload(ml2_models.PortBinding.port))
    query = query.filter(models_v2.Port.network_id == network_id,
                         models_v2.Port.status == const.PORT_STATUS_ACTIVE)
    return query
@ -126,7 +126,7 @@ def _get_dvr_active_network_ports(context, network_id):
                       ml2_models.DistributedPortBinding.host)
    query = query.join(models_v2.Port)
    query = query.options(
-        orm.subqueryload(ml2_models.DistributedPortBinding.port))
+        orm.selectinload(ml2_models.DistributedPortBinding.port))
    query = query.filter(models_v2.Port.network_id == network_id,
                         models_v2.Port.status == const.PORT_STATUS_ACTIVE,
                         models_v2.Port.device_owner ==
--- a/neutron/plugins/ml2/models.py
+++ b/neutron/plugins/ml2/models.py
@ -89,7 +89,7 @@ class PortBindingLevel(model_base.BASEV2):
    port = orm.relationship(
        models_v2.Port,
        load_on_pending=True,
-        backref=orm.backref("binding_levels", lazy='subquery',
+        backref=orm.backref("binding_levels", lazy='selectin',
                            cascade='delete'))
    segment = orm.relationship(
        segment_models.NetworkSegment,
--- a/neutron/services/trunk/models.py
+++ b/neutron/services/trunk/models.py
@ -46,7 +46,7 @@ class Trunk(standard_attr.HasStandardAttributes, model_base.BASEV2,
                               cascade='delete'))

    sub_ports = sa.orm.relationship(
-        'SubPort', lazy='subquery', uselist=True, cascade="all, delete-orphan")
+        'SubPort', lazy='selectin', uselist=True, cascade="all, delete-orphan")
    api_collections = ['trunks']
    collection_resource_map = {'trunks': 'trunk'}
    tag_support = True
--- a/neutron/tests/unit/hacking/test_checks.py
+++ b/neutron/tests/unit/hacking/test_checks.py
@ -279,3 +279,14 @@ class HackingTestCase(base.BaseTestCase):
        _pass(["_('foo')"], "neutron/_i18n.py")
        _pass(["_('foo')"], "neutron/i18n.py")
        _pass(["_('foo')"], "neutron/foo.py", noqa=True)
+
+    def test_check_no_sqlalchemy_lazy_subquery(self):
+        f = checks.check_no_sqlalchemy_lazy_subquery
+        self.assertLineFails('N350', f,
+            "backref=orm.backref('tags', lazy='subquery', viewonly=True),")
+        self.assertLineFails('N350', f,
+            "query.options(orm.subqueryload(ml2_models.PortBinding.port))")
+        self.assertLinePasses(f,
+            "backref=orm.backref('tags', lazy='selectin', viewonly=True),")
+        self.assertLinePasses(f,
+            "query.options(orm.selectinload(ml2_models.PortBinding.port))")
--- a/tox.ini
+++ b/tox.ini
@ -239,6 +239,7 @@ extension =
  N346 = neutron.hacking.checks:check_no_sqlalchemy_event_import
  N348 = neutron.hacking.checks:check_no_import_six
  N349 = neutron.hacking.checks:check_no_import_packaging
+  N350 = neutron.hacking.checks:check_no_sqlalchemy_lazy_subquery
  # Checks from neutron-lib
  N521 = neutron_lib.hacking.checks:use_jsonutils
  N524 = neutron_lib.hacking.checks:check_no_contextlib_nested