From 48b30783c0d550b4fb23c5886090c4f57b88ad53 Mon Sep 17 00:00:00 2001 From: Oleg Bondarev Date: Fri, 9 Apr 2021 11:52:29 +0300 Subject: [PATCH] DHCP notification optimization DHCP notification is done after each create/update/delete for network, subnet and port. This notification currently has to retrieve network from DB almost every time, which is a quite heavy DB request and hence affects performance of port and subnet CRUD. This patch suggests 2 optimizations: - do not fetch network if not needed (only fetch when schedule needed) - for port and subnet AFTER_CREATE event pass network dict from plugin According to Rally tests these changes improve performance: - port create ~20% - port update ~20% - subnet create ~15% - port delete and subnet update/delete - not tested Conflicts: neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py Closes-Bug: #1923161 Change-Id: I0ab836ac09225f4f3ad435e9ceaf315018855d52 (cherry picked from commit 130655cdb98f493d0ceda72e2c3a0649c3d9024d) --- .../rpc/agentnotifiers/dhcp_rpc_agent_api.py | 31 +++++++++++++------ neutron/plugins/ml2/plugin.py | 4 +++ .../agentnotifiers/test_dhcp_rpc_agent_api.py | 19 ++++++------ .../tests/unit/db/test_agentschedulers_db.py | 6 ++++ 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py index ced6a023bf8..29b61b2c9ba 100644 --- a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py +++ b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py @@ -143,11 +143,11 @@ class DhcpAgentNotifyAPI(object): network['id']) return new_agents + existing_agents - def _get_enabled_agents(self, context, network, agents, method, payload): + def _get_enabled_agents( + self, context, network_id, network, agents, method, payload): """Get the list of agents who can provide services.""" if not agents: return [] - network_id = network['id'] enabled_agents = agents if not cfg.CONF.enable_services_on_agents_with_admin_state_down: enabled_agents = [x for x in agents if x.admin_state_up] @@ -165,6 +165,10 @@ class DhcpAgentNotifyAPI(object): if not enabled_agents: num_ports = self.plugin.get_ports_count( context, {'network_id': [network_id]}) + if not network: + admin_ctx = (context if context.is_admin else + context.elevated()) + network = self.plugin.get_network(admin_ctx, network_id) notification_required = ( num_ports > 0 and len(network['subnets']) >= 1) if notification_required: @@ -179,7 +183,8 @@ class DhcpAgentNotifyAPI(object): def _is_reserved_dhcp_port(self, port): return port.get('device_id') == constants.DEVICE_ID_RESERVED_DHCP_PORT - def _notify_agents(self, context, method, payload, network_id): + def _notify_agents( + self, context, method, payload, network_id, network=None): """Notify all the agents that are hosting the network.""" payload['priority'] = METHOD_PRIORITY_MAP.get(method) # fanout is required as we do not know who is "listening" @@ -194,31 +199,35 @@ class DhcpAgentNotifyAPI(object): if fanout_required: self._fanout_message(context, method, payload) elif cast_required: - admin_ctx = (context if context.is_admin else context.elevated()) - network = self.plugin.get_network(admin_ctx, network_id) + candidate_hosts = None if 'subnet' in payload and payload['subnet'].get('segment_id'): # if segment_id exists then the segment service plugin # must be loaded segment_plugin = directory.get_plugin('segments') segment = segment_plugin.get_segment( context, payload['subnet']['segment_id']) - network['candidate_hosts'] = segment['hosts'] + candidate_hosts = segment['hosts'] agents = self.plugin.get_dhcp_agents_hosting_networks( - context, [network_id], hosts=network.get('candidate_hosts')) + context, [network_id], hosts=candidate_hosts) # schedule the network first, if needed schedule_required = ( method == 'subnet_create_end' or method == 'port_create_end' and not self._is_reserved_dhcp_port(payload['port'])) if schedule_required: + admin_ctx = context if context.is_admin else context.elevated() + network = network or self.plugin.get_network( + admin_ctx, network_id) + if candidate_hosts: + network['candidate_hosts'] = candidate_hosts agents = self._schedule_network(admin_ctx, network, agents) if not agents: LOG.debug("Network %s is not hosted by any dhcp agent", network_id) return enabled_agents = self._get_enabled_agents( - context, network, agents, method, payload) + context, network_id, network, agents, method, payload) if method == 'port_create_end': high_agent = enabled_agents.pop( @@ -346,6 +355,8 @@ class DhcpAgentNotifyAPI(object): payload['network_id'] = network_id if obj_type == 'port': payload['fixed_ips'] = obj_value['fixed_ips'] - self._notify_agents(context, method_name, payload, network_id) + self._notify_agents(context, method_name, payload, network_id, + obj_value.get('network')) else: - self._notify_agents(context, method_name, data, network_id) + self._notify_agents(context, method_name, data, network_id, + obj_value.get('network')) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 7084c4621be..ab1be4acbcd 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -1247,6 +1247,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, # db base plugin post commit ops self._create_subnet_postcommit(context, result) + # add network to subnet dict to save a DB call on dhcp notification + result['network'] = mech_context.network.current kwargs = {'context': context, 'subnet': result} registry.notify(resources.SUBNET, events.AFTER_CREATE, self, **kwargs) try: @@ -1410,6 +1412,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, return self._after_create_port(context, result, mech_context) def _after_create_port(self, context, result, mech_context): + # add network to port dict to save a DB call on dhcp notification + result['network'] = mech_context.network.current # notify any plugin that is interested in port create events kwargs = {'context': context, 'port': result} registry.notify(resources.PORT, events.AFTER_CREATE, self, **kwargs) diff --git a/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py b/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py index de3bd75beac..1b4a63c65f8 100644 --- a/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py +++ b/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py @@ -85,12 +85,12 @@ class TestDhcpAgentNotifyAPI(base.BaseTestCase): new_agents=None, existing_agents=[], expected_casts=0, expected_warnings=1) - def _test__get_enabled_agents(self, network, + def _test__get_enabled_agents(self, network_id, agents=None, port_count=0, expected_warnings=0, expected_errors=0): self.notifier.plugin.get_ports_count.return_value = port_count enabled_agents = self.notifier._get_enabled_agents( - mock.ANY, network, agents, mock.ANY, mock.ANY) + mock.Mock(), network_id, None, agents, mock.ANY, mock.ANY) if not cfg.CONF.enable_services_on_agents_with_admin_state_down: agents = [x for x in agents if x.admin_state_up] self.assertEqual(agents, enabled_agents) @@ -104,8 +104,8 @@ class TestDhcpAgentNotifyAPI(base.BaseTestCase): agent2 = agent_obj.Agent(mock.ANY, id=uuidutils.generate_uuid()) agent2.admin_state_up = False agent2.heartbeat_timestamp = timeutils.utcnow() - network = {'id': 'foo_network_id'} - self._test__get_enabled_agents(network, agents=[agent1]) + self._test__get_enabled_agents(network_id='foo_network_id', + agents=[agent1]) def test__get_enabled_agents_with_inactive_ones(self): agent1 = agent_obj.Agent(mock.ANY, id=uuidutils.generate_uuid()) @@ -115,17 +115,18 @@ class TestDhcpAgentNotifyAPI(base.BaseTestCase): agent2.admin_state_up = True # This is effectively an inactive agent agent2.heartbeat_timestamp = datetime.datetime(2000, 1, 1, 0, 0) - network = {'id': 'foo_network_id'} - self._test__get_enabled_agents(network, + self._test__get_enabled_agents(network_id='foo_network_id', agents=[agent1, agent2], expected_warnings=1, expected_errors=0) def test__get_enabled_agents_with_notification_required(self): network = {'id': 'foo_network_id', 'subnets': ['foo_subnet_id']} + self.notifier.plugin.get_network.return_value = network agent = agent_obj.Agent(mock.ANY, id=uuidutils.generate_uuid()) agent.admin_state_up = False agent.heartbeat_timestamp = timeutils.utcnow() - self._test__get_enabled_agents(network, [agent], port_count=20, + self._test__get_enabled_agents('foo_network_id', + [agent], port_count=20, expected_warnings=0, expected_errors=1) def test__get_enabled_agents_with_admin_state_down(self): @@ -137,8 +138,8 @@ class TestDhcpAgentNotifyAPI(base.BaseTestCase): agent2 = agent_obj.Agent(mock.ANY, id=uuidutils.generate_uuid()) agent2.admin_state_up = False agent2.heartbeat_timestamp = timeutils.utcnow() - network = {'id': 'foo_network_id'} - self._test__get_enabled_agents(network, agents=[agent1, agent2]) + self._test__get_enabled_agents(network_id='foo_network_id', + agents=[agent1, agent2]) def test__notify_agents_allocate_priority(self): mock_context = mock.MagicMock() diff --git a/neutron/tests/unit/db/test_agentschedulers_db.py b/neutron/tests/unit/db/test_agentschedulers_db.py index 3c57b415116..c81e2f59139 100644 --- a/neutron/tests/unit/db/test_agentschedulers_db.py +++ b/neutron/tests/unit/db/test_agentschedulers_db.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy import datetime import mock @@ -1523,6 +1524,11 @@ class OvsDhcpAgentNotifierTestCase(test_agent.AgentDBTestMixIn, return net, sub, port def _notification_mocks(self, hosts, net, subnet, port, port_priority): + subnet['subnet']['network'] = copy.deepcopy(net['network']) + # 'availability_zones' is empty at the time subnet_create_end + # notification is sent + subnet['subnet']['network']['availability_zones'] = [] + port['port']['network'] = net['network'] host_calls = {} for host in hosts: expected_calls = [