Skip to content

Commit 3dec91e

Browse files
committed
Change SG rules backref load method to "joined"
Since [1], we introduced a way to skip the load of the OVO synthetic fields depending on the resource fields retrieved. In the case of the security groups (SG), the SG rules are child objects to the SGs. The SG rules are retrieved when a SG OVO is created. The improvement done in [1] is to make the SG rules load dynamically, that means using the load mode "lazy='dynamic'". That will issue a SQL query only if the SG rules are read; if not, the query is never issued. However since [2] (and this is previous to the [1] optimization), we always add the field "shared" to the filters and thus to the fields to retrieve, because it is a policy required field. Because "shared" is a synthetic field [3], that will force the SG "synthetic_fields" load and the retrieval of the SG rules always. This is undoing any performance improvement. This patch is changing the loading method to "joined"; that will request the SG rules in the same SQL query, instead of issuing separate queries for the SG rules. Until a method to load the SG "shared" field, independently of the synthetic OVO fields is implemented, this change will improve the SG retrieval performance. In a testing environment with around 5500K SG and 4 rules (default ones) per SG: * lazy='dynamic': 38 seconds * lazy='select': 20 seconds * lazy='joined': 12 seconds [1]https://review.opendev.org/q/topic:%22bug/1810563%22 [2]https://review.opendev.org/c/openstack/neutron/+/328313 [3]https://github.com/openstack/neutron/blob/b85b19e3846fa74975ba5d703336b6e7cd8af433/neutron/objects/rbac_db.py#L349 Related-Bug: #2052419 Change-Id: I300464472f2348d148f2a3ddac384c883d9d815b (cherry picked from commit 52662ca)
1 parent f26a625 commit 3dec91e

File tree

2 files changed

+9
-1
lines changed

2 files changed

+9
-1
lines changed

neutron/db/models/securitygroup.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,13 @@ class SecurityGroupRule(standard_attr.HasStandardAttributes, model_base.BASEV2,
101101
port_range_min = sa.Column(sa.Integer)
102102
port_range_max = sa.Column(sa.Integer)
103103
remote_ip_prefix = sa.Column(sa.String(255))
104+
# NOTE(ralonsoh): loading method is temporarily changed to "joined" until
105+
# a proper way to only load the security groups "shared" field, without
106+
# loading the rest of the synthetic fields, is implemented. LP#2052419
107+
# description for more information and context.
104108
security_group = orm.relationship(
105109
SecurityGroup, load_on_pending=True,
106-
backref=orm.backref('rules', cascade='all,delete', lazy='dynamic'),
110+
backref=orm.backref('rules', cascade='all,delete', lazy='joined'),
107111
primaryjoin="SecurityGroup.id==SecurityGroupRule.security_group_id")
108112
source_group = orm.relationship(
109113
SecurityGroup,

neutron/tests/unit/db/test_securitygroups_db.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,10 @@ def _test_security_group_precommit_create_event(self,
347347
# Especially we want to check the revision number here.
348348
sg_dict_got = self.mixin.get_security_group(
349349
self.ctx, sg_dict['id'])
350+
# Order the SG rules to avoid issues in the assertion.
351+
for _sg_dict in (sg_dict, sg_dict_got):
352+
_sg_dict['security_group_rules'] = sorted(
353+
_sg_dict['security_group_rules'], key=lambda d: d['id'])
350354
self.assertEqual(sg_dict, sg_dict_got)
351355

352356
def test_security_group_precommit_create_event_with_revisions(self):

0 commit comments

Comments
 (0)