Improve performance when list instances with IP filter

When list instances with IP filter, there is no need
to try to get instances from BuildRequest, as building
instances will not have IP address. Skip this db call
can slightly improve performance.

Closes-Bug: #1764685

Change-Id: Ic02206e887e3fea7752d615bbed680510c482097
(cherry picked from commit a3a94cf4ec)

nova/compute/api.py

@@ -2393,8 +2393,10 @@ class API(base.Base):
         # IP address filtering cannot be applied at the DB layer, remove any DB
         # limit so that it can be applied after the IP filter.
         filter_ip = 'ip6' in filters or 'ip' in filters
+        skip_build_request = False
         orig_limit = limit
         if filter_ip:
+            skip_build_request = True
             if self.network_api.has_substr_port_filtering_extension(context):
                 # We're going to filter by IP using Neutron so set filter_ip
                 # to False so we don't attempt post-DB query filtering in
@@ -2425,21 +2427,27 @@ class API(base.Base):
                 LOG.debug('Removing limit for DB query due to IP filter')
                 limit = None
 
-        # The ordering of instances will be
-        # [sorted instances with no host] + [sorted instances with host].
-        # This means BuildRequest and cell0 instances first, then cell
-        # instances
-        try:
-            build_requests = objects.BuildRequestList.get_by_filters(
-                context, filters, limit=limit, marker=marker,
-                sort_keys=sort_keys, sort_dirs=sort_dirs)
-            # If we found the marker in we need to set it to None
-            # so we don't expect to find it in the cells below.
-            marker = None
-        except exception.MarkerNotFound:
-            # If we didn't find the marker in the build requests then keep
-            # looking for it in the cells.
+        # Skip get BuildRequest if filtering by IP address, as building
+        # instances will not have IP addresses.
+        if skip_build_request:
             build_requests = objects.BuildRequestList()
+        else:
+            # The ordering of instances will be
+            # [sorted instances with no host] + [sorted instances with host].
+            # This means BuildRequest and cell0 instances first, then cell
+            # instances
+            try:
+                build_requests = objects.BuildRequestList.get_by_filters(
+                    context, filters, limit=limit, marker=marker,
+                    sort_keys=sort_keys, sort_dirs=sort_dirs)
+                # If we found the marker in we need to set it to None
+                # so we don't expect to find it in the cells below.
+                marker = None
+            except exception.MarkerNotFound:
+                # If we didn't find the marker in the build requests then keep
+                # looking for it in the cells.
+                build_requests = objects.BuildRequestList()
+
         build_req_instances = objects.InstanceList(
             objects=[build_req.instance for build_req in build_requests])
         # Only subtract from limit if it is not None

nova/tests/unit/compute/test_compute_api.py

@@ -5843,7 +5843,8 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
 
     @mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
     @mock.patch.object(neutron_api.API, 'list_ports')
-    @mock.patch.object(objects.BuildRequestList, 'get_by_filters')
+    @mock.patch.object(objects.BuildRequestList, 'get_by_filters',
+                       new_callable=mock.NonCallableMock)
     def test_get_all_ip_filter_use_neutron(self, mock_buildreq_get,
                                            mock_list_port, mock_check_ext):
         mock_check_ext.return_value = True
@@ -5863,18 +5864,15 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
             mock_list_port.assert_called_once_with(
                 self.context, fixed_ips='ip_address_substr=fake',
                 fields=['device_id'])
-            mock_buildreq_get.assert_called_once_with(
-                self.context, {'ip': 'fake', 'uuid': ['fake_device_id']},
-                limit=None, marker='fake-marker',
-                sort_keys=['baz'], sort_dirs=['desc'])
             fields = ['metadata', 'info_cache', 'security_groups']
             mock_inst_get.assert_called_once_with(
                 self.context, {'ip': 'fake', 'uuid': ['fake_device_id']},
-                None, None, fields, ['baz'], ['desc'])
+                None, 'fake-marker', fields, ['baz'], ['desc'])
 
     @mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
     @mock.patch.object(neutron_api.API, 'list_ports')
-    @mock.patch.object(objects.BuildRequestList, 'get_by_filters')
+    @mock.patch.object(objects.BuildRequestList, 'get_by_filters',
+                       new_callable=mock.NonCallableMock)
     def test_get_all_ip6_filter_use_neutron(self, mock_buildreq_get,
                                             mock_list_port, mock_check_ext):
         mock_check_ext.return_value = True
@@ -5894,18 +5892,15 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
             mock_list_port.assert_called_once_with(
                 self.context, fixed_ips='ip_address_substr=fake',
                 fields=['device_id'])
-            mock_buildreq_get.assert_called_once_with(
-                self.context, {'ip6': 'fake', 'uuid': ['fake_device_id']},
-                limit=None, marker='fake-marker',
-                sort_keys=['baz'], sort_dirs=['desc'])
             fields = ['metadata', 'info_cache', 'security_groups']
             mock_inst_get.assert_called_once_with(
                 self.context, {'ip6': 'fake', 'uuid': ['fake_device_id']},
-                None, None, fields, ['baz'], ['desc'])
+                None, 'fake-marker', fields, ['baz'], ['desc'])
 
     @mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
     @mock.patch.object(neutron_api.API, 'list_ports')
-    @mock.patch.object(objects.BuildRequestList, 'get_by_filters')
+    @mock.patch.object(objects.BuildRequestList, 'get_by_filters',
+                       new_callable=mock.NonCallableMock)
     def test_get_all_ip_and_ip6_filter_use_neutron(self, mock_buildreq_get,
                                                    mock_list_port,
                                                    mock_check_ext):
@@ -5931,16 +5926,11 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
                     self.context, fixed_ips='ip_address_substr=fake2',
                     fields=['device_id'])
             ])
-            mock_buildreq_get.assert_called_once_with(
-                self.context, {'ip': 'fake1', 'ip6': 'fake2',
-                               'uuid': ['fake_device_id', 'fake_device_id']},
-                limit=None, marker='fake-marker',
-                sort_keys=['baz'], sort_dirs=['desc'])
             fields = ['metadata', 'info_cache', 'security_groups']
             mock_inst_get.assert_called_once_with(
                 self.context, {'ip': 'fake1', 'ip6': 'fake2',
                                'uuid': ['fake_device_id', 'fake_device_id']},
-                None, None, fields, ['baz'], ['desc'])
+                None, 'fake-marker', fields, ['baz'], ['desc'])
 
     @mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
     @mock.patch.object(neutron_api.API, 'list_ports')