From a57d47bcb2df76df81eed3ad44c8f49e7dbda0e8 Mon Sep 17 00:00:00 2001 From: Fernando Royo Date: Tue, 7 Jun 2022 13:51:51 +0200 Subject: [PATCH] Optimize queries for port operations Port create/update are most time-consuming operations on subnet creation. As example, in those cases where several subnets are created over the same network the response time for those port operations is linearly increased as the total subnets increases. This patch improves the number of queries required on port operations in order to reduce the response time. Closes-Bug: #1977831 Change-Id: I0fccf36a2035e8f6c2fa8dab0307358da600c8f7 (cherry picked from commit c25097b0b0da2af9021699036a69cd10e66533b1) --- neutron/objects/subnet.py | 11 ++- .../ovn/mech_driver/ovsdb/ovn_client.py | 68 +++++++++++++------ neutron/tests/unit/objects/test_subnet.py | 20 ++++++ .../ovn/mech_driver/test_mech_driver.py | 59 ++++++++++++++++ 4 files changed, 133 insertions(+), 25 deletions(-) diff --git a/neutron/objects/subnet.py b/neutron/objects/subnet.py index 267cfbf0293..3a17687bca2 100644 --- a/neutron/objects/subnet.py +++ b/neutron/objects/subnet.py @@ -364,12 +364,17 @@ class Subnet(base.NeutronDbObject): :raises: FixedIpsSubnetsNotOnSameSegment """ segment_ids = [] + subnets = query.all() + for fixed_ip in fixed_ips: subnet = None if 'subnet_id' in fixed_ip: try: - subnet = query.filter( - cls.db_model.id == fixed_ip['subnet_id']).all()[0] + subnet = [ + sub + for sub in subnets + if sub['id'] == fixed_ip['subnet_id'] + ][0] except IndexError: # NOTE(hjensas): The subnet is invalid for the network, # return all subnets. This will be detected in following @@ -378,7 +383,7 @@ class Subnet(base.NeutronDbObject): elif 'ip_address' in fixed_ip: ip = netaddr.IPNetwork(fixed_ip['ip_address']) - for s in query.all(): + for s in subnets: if ip in netaddr.IPNetwork(s.cidr): subnet = s break diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 77940fba942..b0c6a9a7a8f 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -281,27 +281,46 @@ class OVNClient(object): parent_name = binding_prof.get('parent_name', []) tag = binding_prof.get('tag', []) address = port['mac_address'] - for ip in port.get('fixed_ips', []): - try: - subnet = self._plugin.get_subnet(context, ip['subnet_id']) - except n_exc.SubnetNotFound: - continue - ip_addr = ip['ip_address'] - address += ' ' + ip_addr - cidrs += ' {}/{}'.format(ip['ip_address'], - subnet['cidr'].split('/')[1]) - # Check if the port being created is a virtual port - parents = utils.get_virtual_port_parents( - self._nb_idl, ip_addr, port['network_id'], port['id']) - if not parents: - continue + ip_subnets = port.get('fixed_ips', []) + subnet_ids = [ + ip['subnet_id'] + for ip in ip_subnets + if 'subnet_id' in ip + ] + subnets = self._plugin.get_subnets( + context, filters={'id': subnet_ids}) + if subnets: + for ip in ip_subnets: + ip_addr = ip['ip_address'] + address += ' ' + ip_addr + subnet = None - port_type = ovn_const.LSP_TYPE_VIRTUAL - options[ovn_const.LSP_OPTIONS_VIRTUAL_IP_KEY] = ip_addr - options[ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY] = ( - ','.join(parents)) - break + try: + subnet = [ + sub + for sub in subnets + if sub["id"] == ip["subnet_id"] + ][0] + except IndexError: + LOG.debug('Subnet not found for ip address %s', + ip_addr) + continue + + cidrs += ' {}/{}'.format(ip['ip_address'], + subnet['cidr'].split('/')[1]) + + # Check if the port being created is a virtual port + parents = utils.get_virtual_port_parents( + self._nb_idl, ip_addr, port['network_id'], port['id']) + if not parents: + continue + + port_type = ovn_const.LSP_TYPE_VIRTUAL + options[ovn_const.LSP_OPTIONS_VIRTUAL_IP_KEY] = ip_addr + options[ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY] = ( + ','.join(parents)) + break # Metadata port. if port['device_owner'] == const.DEVICE_OWNER_DISTRIBUTED: @@ -616,9 +635,14 @@ class OVNClient(object): if self.is_metadata_port(port): context = n_context.get_admin_context() network = self._plugin.get_network(context, port['network_id']) - subnet_ids = set(_ip['subnet_id'] for _ip in port['fixed_ips']) - for subnet_id in subnet_ids: - subnet = self._plugin.get_subnet(context, subnet_id) + subnet_ids = [ + _ip['subnet_id'] + for _ip in port['fixed_ips'] + if 'subnet_id' in _ip + ] + + for subnet in self._plugin.get_subnets( + context, filters={'id': subnet_ids}): if not subnet['enable_dhcp']: continue self._update_subnet_dhcp_options(subnet, network, txn) diff --git a/neutron/tests/unit/objects/test_subnet.py b/neutron/tests/unit/objects/test_subnet.py index 7e2eee08c9d..05ddb9a3eb5 100644 --- a/neutron/tests/unit/objects/test_subnet.py +++ b/neutron/tests/unit/objects/test_subnet.py @@ -268,6 +268,26 @@ class SubnetDbObjectTestCase(obj_test_base.BaseDbObjectTestCase, self.assertEqual([service_type_obj.service_type], obj1.service_types) + def test_find_candidate_subnets(self): + network = self._create_test_network() + subnet_data = dict(self.obj_fields[0]) + subnet_data['network_id'] = network['id'] + subnet_net = self._make_object(subnet_data) + subnet2_data = dict(self.obj_fields[0]) + subnet2_data['id'] = uuidutils.generate_uuid() + subnet2_data['network_id'] = network['id'] + subnet2_net = self._make_object(subnet2_data) + subnet_net.create() + subnet2_net.create() + fixed_ips = [ + {'subnet_id': subnet_data['id'], 'ip_address': '10.0.0.2'}, + {'subnet_id': subnet2_data['id'], 'ip_address': '10.0.1.2'}] + candidate_subnet = subnet.Subnet.find_candidate_subnets( + self.context, network['id'], None, None, True, fixed_ips) + self.assertEqual(2, len(candidate_subnet)) + self.assertNotEqual( + candidate_subnet[0]['id'], candidate_subnet[1]['id']) + class NetworkSubnetLockTestCase(obj_test_base.BaseObjectIfaceTestCase): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index d8a64665baf..04808f8d53b 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -1823,6 +1823,65 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): context.current, context.network.current, mock.ANY) umd.assert_called_once_with(mock.ANY, 'id', subnet=subnet) + def test__get_port_options(self): + with mock.patch.object(self.mech_driver._plugin, 'get_subnets') as \ + mock_get_subnets: + port = {'id': 'virt-port', + 'mac_address': '00:00:00:00:00:00', + 'device_owner': 'device_owner', + 'network_id': 'foo', + 'fixed_ips': [{'subnet_id': 'subnet-1', + 'ip_address': '10.0.0.55'}, + {'subnet_id': 'subnet-2', + 'ip_address': '10.0.1.55'}, + ]} + subnet_ids = [ + ip['subnet_id'] + for ip in port.get('fixed_ips') + ] + self.mech_driver._ovn_client._get_port_options(port) + mock_get_subnets.assert_called_once_with( + mock.ANY, + filters={'id': subnet_ids}) + + def test_update_port(self): + with mock.patch.object( + self.mech_driver._ovn_client, 'is_metadata_port') as \ + mock_is_metadata_port, \ + mock.patch.object(self.mech_driver._plugin, 'get_subnets') as \ + mock_get_subnets, \ + mock.patch.object(self.mech_driver._plugin, 'get_network') as \ + mock_get_network: + net_attrs = {az_def.AZ_HINTS: ['az0', 'az1', 'az2']} + fake_net = ( + fakes.FakeNetwork.create_one_network(attrs=net_attrs).info()) + port = {'id': 'virt-port', + 'mac_address': '00:00:00:00:00:00', + 'name': 'port-foo', + 'device_id': 'device_id-foo', + 'project_id': 'project_id-foo', + 'device_owner': 'device_owner', + 'network_id': 'foo', + 'admin_state_up': True, + 'fixed_ips': [{'subnet_id': 'subnet-1', + 'ip_address': '10.0.0.55'}, + {'subnet_id': 'subnet-2', + 'ip_address': '10.0.1.55'}, + ]} + subnet_ids = [ + ip['subnet_id'] + for ip in port.get('fixed_ips') + ] + + mock_is_metadata_port.return_value = [True] + mock_get_network.return_value = fake_net + self.mech_driver._ovn_client.update_port( + self.context, port) + self.assertEqual(mock_get_subnets.call_count, 2) + mock_get_subnets.assert_called_with( + mock.ANY, + filters={'id': subnet_ids}) + def test_update_metadata_port_with_subnet(self): ovn_conf.cfg.CONF.set_override('ovn_metadata_enabled', True, group='ovn')