openstack/nova
Code Issues Proposed changes

trivial: Fetch 'Service' objects once when building AZs

Browse Source 
As noted in a TODO by mriedem, we were making two trips to the DB when
retrieving 'Service' objects in order to build a list of availability
zones. Optimize things so that this is no longer the case.

Change-Id: Id0d8c52cba8b548a0e6436d94765b84f040efee5
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
This commit is contained in:
Stephen Finucane 2020-02-04 12:28:32 +00:00
parent 5cea9ed9ad
commit 14fed95f61
5 changed files with 104 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
nova/api/openstack/compute/availability_zone.py
View File

@ -44,9 +44,9 @@ class AvailabilityZoneController(wsgi.Controller):
def _describe_availability_zones(self, context, **kwargs):
ctxt = context.elevated()
available_zones, not_available_zones = \
available_zones, not_available_zones = (
availability_zones.get_availability_zones(
ctxt, self.host_api)
ctxt, self.host_api))
filtered_available_zones = \
self._get_filtered_availability_zones(available_zones, True)
@ -58,18 +58,16 @@ class AvailabilityZoneController(wsgi.Controller):
def _describe_availability_zones_verbose(self, context, **kwargs):
ctxt = context.elevated()
# Available services
enabled_services = self.host_api.service_get_all(
context, {'disabled': False}, set_zones=True, all_cells=True)
services = self.host_api.service_get_all(
context, set_zones=True, all_cells=True)
available_zones, not_available_zones = (
availability_zones.get_availability_zones(
ctxt, self.host_api, enabled_services=enabled_services))
ctxt, self.host_api, services=services))
zone_hosts = {}
host_services = {}
api_services = ('nova-osapi_compute', 'nova-metadata')
for service in enabled_services:
for service in filter(lambda x: not x.disabled, services):
if service.binary in api_services:
# Skip API services in the listing since they are not
# maintained in the same way as other services

97
nova/availability_zones.py
View File

@ -110,7 +110,7 @@ def update_host_availability_zone_cache(context, host, availability_zone=None):
def get_availability_zones(context, hostapi, get_only_available=False,
with_hosts=False, enabled_services=None):
with_hosts=False, services=None):
"""Return available and unavailable zones on demand.
:param context: nova auth RequestContext
@ -121,50 +121,65 @@ def get_availability_zones(context, hostapi, get_only_available=False,
available zones only
:param with_hosts: whether to return hosts part of the AZs
:type with_hosts: bool
:param enabled_services: list of enabled services to use; if None
enabled services will be retrieved from all cells with zones set
:param services: list of services to use; if None, enabled services will be
retrieved from all cells with zones set
"""
if enabled_services is None:
enabled_services = hostapi.service_get_all(
context, {'disabled': False}, set_zones=True, all_cells=True)
if services is None:
services = hostapi.service_get_all(
context, set_zones=True, all_cells=True)
available_zones = []
for (zone, host) in [(service['availability_zone'], service['host'])
for service in enabled_services]:
if not with_hosts and zone not in available_zones:
available_zones.append(zone)
elif with_hosts:
_available_zones = dict(available_zones)
zone_hosts = _available_zones.setdefault(zone, set())
zone_hosts.add(host)
# .items() returns a view in Py3, casting it to list for Py2 compat
available_zones = list(_available_zones.items())
enabled_services = []
disabled_services = []
for service in services:
if not service.disabled:
enabled_services.append(service)
else:
disabled_services.append(service)
if not get_only_available:
# TODO(mriedem): We could probably optimize if we know that we're going
# to get both enabled and disabled services and just pull them all from
# the cell DBs at the same time and then filter into enabled/disabled
# lists in python.
disabled_services = hostapi.service_get_all(
context, {'disabled': True}, set_zones=True, all_cells=True)
not_available_zones = []
azs = available_zones if not with_hosts else dict(available_zones)
zones = [(service['availability_zone'], service['host'])
for service in disabled_services
if service['availability_zone'] not in azs]
for (zone, host) in zones:
if not with_hosts and zone not in not_available_zones:
not_available_zones.append(zone)
elif with_hosts:
_not_available_zones = dict(not_available_zones)
zone_hosts = _not_available_zones.setdefault(zone, set())
zone_hosts.add(host)
# .items() returns a view in Py3, casting it to list for Py2
# compat
not_available_zones = list(_not_available_zones.items())
return (available_zones, not_available_zones)
if with_hosts:
return _get_availability_zones_with_hosts(
enabled_services, disabled_services, get_only_available)
else:
return available_zones
return _get_availability_zones(
enabled_services, disabled_services, get_only_available)
def _get_availability_zones(
enabled_services, disabled_services, get_only_available=False):
available_zones = {
service['availability_zone'] for service in enabled_services
}
if get_only_available:
return sorted(available_zones)
not_available_zones = {
service['availability_zone'] for service in disabled_services
if service['availability_zone'] not in available_zones
}
return sorted(available_zones), sorted(not_available_zones)
def _get_availability_zones_with_hosts(
enabled_services, disabled_services, get_only_available=False):
available_zones = collections.defaultdict(set)
for service in enabled_services:
available_zones[service['availability_zone']].add(service['host'])
if get_only_available:
return sorted(available_zones.items())
not_available_zones = collections.defaultdict(set)
for service in disabled_services:
if service['availability_zone'] in available_zones:
continue
not_available_zones[service['availability_zone']].add(service['host'])
return sorted(available_zones.items()), sorted(not_available_zones.items())
def get_instance_availability_zone(context, instance):

2
nova/tests/fixtures.py
View File

@ -2401,7 +2401,7 @@ class AvailabilityZoneFixture(fixtures.Fixture):
def fake_get_availability_zones(
ctxt, hostapi, get_only_available=False,
with_hosts=False, enabled_services=None):
with_hosts=False, services=None):
# A 2-item tuple is returned if get_only_available=False.
if not get_only_available:
return self.zones, []

81
nova/tests/unit/api/openstack/compute/test_availability_zone.py
View File

@ -38,7 +38,6 @@ FAKE_UUID = fakes.FAKE_UUID
def fake_service_get_all(context, filters=None, **kwargs):
disabled = filters.get('disabled') if filters else None
def __fake_service(binary, availability_zone,
created_at, updated_at, host, disabled):
@ -54,30 +53,32 @@ def fake_service_get_all(context, filters=None, **kwargs):
db_s.pop('version', None)
return objects.Service(context, **db_s)
if disabled:
svcs = [__fake_service("nova-compute", "zone-2",
datetime.datetime(2012, 11, 14, 9, 53, 25, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", True),
__fake_service("nova-scheduler", "internal",
datetime.datetime(2012, 11, 14, 9, 57, 3, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", True)]
else:
svcs = [__fake_service("nova-compute", "zone-1",
datetime.datetime(2012, 11, 14, 9, 53, 25, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", False),
__fake_service("nova-sched", "internal",
datetime.datetime(2012, 11, 14, 9, 57, 3, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", False),
# nova-conductor is in the same zone and host as nova-sched
# and is here to make sure /detail filters out duplicates.
__fake_service("nova-conductor", "internal",
datetime.datetime(2012, 11, 14, 9, 57, 3, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", False)]
svcs = [__fake_service("nova-compute", "zone-2",
datetime.datetime(2012, 11, 14, 9, 53, 25, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", True),
__fake_service("nova-scheduler", "internal",
datetime.datetime(2012, 11, 14, 9, 57, 3, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", True),
__fake_service("nova-compute", "zone-1",
datetime.datetime(2012, 11, 14, 9, 53, 25, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", False),
__fake_service("nova-sched", "internal",
datetime.datetime(2012, 11, 14, 9, 57, 3, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", False),
# nova-conductor is in the same zone and host as nova-sched
# and is here to make sure /detail filters out duplicates.
__fake_service("nova-conductor", "internal",
datetime.datetime(2012, 11, 14, 9, 57, 3, 0),
datetime.datetime(2012, 12, 26, 14, 45, 25, 0),
"fake_host-1", False)]
if filters and 'disabled' in filters:
svcs = [svc for svc in svcs if svc.disabled == filters['disabled']]
return objects.ServiceList(objects=svcs)
@ -136,19 +137,6 @@ class AvailabilityZoneApiTestV21(test.NoDBTestCase):
self.assertEqual(len(zones), 3)
timestamp = iso8601.parse_date("2012-12-26T14:45:25Z")
expected = [
{
'zoneName': 'zone-1',
'zoneState': {'available': True},
'hosts': {
'fake_host-1': {
'nova-compute': {
'active': True,
'available': True,
'updated_at': timestamp
}
}
}
},
{
'zoneName': 'internal',
'zoneState': {'available': True},
@ -167,6 +155,19 @@ class AvailabilityZoneApiTestV21(test.NoDBTestCase):
}
}
},
{
'zoneName': 'zone-1',
'zoneState': {'available': True},
'hosts': {
'fake_host-1': {
'nova-compute': {
'active': True,
'available': True,
'updated_at': timestamp
}
}
}
},
{
'zoneName': 'zone-2',
'zoneState': {'available': False},
@ -174,9 +175,7 @@ class AvailabilityZoneApiTestV21(test.NoDBTestCase):
}
]
self.assertEqual(expected, zones)
# We get both enabled and disabled services per cell (just one in this
# test case) so we'll query the services table twice.
self.assertEqual(2, self.mock_service_get_all.call_count,
self.assertEqual(1, self.mock_service_get_all.call_count,
self.mock_service_get_all.call_args_list)
@mock.patch.object(availability_zones, 'get_availability_zones',

2
nova/tests/unit/test_availability_zones.py
View File

@ -215,7 +215,7 @@ class AvailabilityZoneTestCases(test.TestCase):
zones, not_zones = az.get_availability_zones(self.context, host_api)
self.assertEqual(['nova-test', 'nova-test2'], zones)
self.assertEqual(['nova-test3', 'nova'], not_zones)
self.assertEqual(['nova', 'nova-test3'], not_zones)
zones = az.get_availability_zones(self.context, host_api,
get_only_available=True)