From 9ea340eb0d3bdb103bd64ca40b999bd2b10b80aa Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Thu, 9 Aug 2018 10:46:20 -0400 Subject: [PATCH] placement: use simple code paths when possible Somewhere in the past release, we started using extremely complex code paths involving sharing providers, anchor providers, and nested resource provider calculations when we absolutely don't need to do so. There was a _has_provider_trees() function in the nova/api/openstack/placement/objects/resource_provider.py file that used to be used for top-level switching between a faster, simpler approach to finding allocation candidates for a simple search of resources and traits when no sharing providers and no nesting was used. That was removed at some point and all code paths -- even for simple "get me these amounts of these resources" when no trees or sharing providers are present (which is the vast majority of OpenStack deployments) -- were going through the complex tree-search-and-match queries and algorithms. This patch changes that so that when there's a request for some resources and there's no trees or sharing providers, we do the simple code path. Hopefully this gets our performance for the simple, common cases back to where we were pre-Rocky. This change is a prerequisite for the following change which adds debugging output to help diagnose which resource classes are running out of inventory when GET /allocation_candidates returns 0 results. That code is not possible without the changes here as they only work if we can identify when a "simpler approach" is possible and call that simpler code. Related-Bug: #1786055 Partial-Bug: #1786519 Change-Id: I1fdbcdb7a1dd51e738924c8a30238237d7ac74e1 --- .../placement/objects/resource_provider.py | 42 +++++++++++-------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/nova/api/openstack/placement/objects/resource_provider.py b/nova/api/openstack/placement/objects/resource_provider.py index 3fb1fed54b41..35a5faae9735 100644 --- a/nova/api/openstack/placement/objects/resource_provider.py +++ b/nova/api/openstack/placement/objects/resource_provider.py @@ -3976,7 +3976,7 @@ class AllocationCandidates(base.VersionedObject): ) @staticmethod - def _get_by_one_request(context, request): + def _get_by_one_request(context, request, sharing_providers, has_trees): """Get allocation candidates for one RequestGroup. Must be called from within an placement_context_manager.reader @@ -3984,6 +3984,12 @@ class AllocationCandidates(base.VersionedObject): :param context: Nova RequestContext. :param request: One nova.api.openstack.placement.util.RequestGroup + :param sharing_providers: dict, keyed by resource class internal ID, of + the set of provider IDs containing shared + inventory of that resource class + :param has_trees: bool indicating there is some level of nesting in the + environment (if there isn't, we take faster, simpler + code paths) :return: A tuple of (allocation_requests, provider_summaries) satisfying `request`. """ @@ -4006,26 +4012,14 @@ class AllocationCandidates(base.VersionedObject): missing = traits - set(trait_map) raise exception.TraitNotFound(names=', '.join(missing)) - # Microversions prior to 1.21 will not have 'member_of' in the groups. - # This allows earlier microversions to continue to work. - member_of = getattr(request, "member_of", None) + member_of = request.member_of - if not request.use_same_provider: + any_sharing = any(sharing_providers.values()) + if not request.use_same_provider and (has_trees or any_sharing): # TODO(jaypipes): The check/callout to handle trees goes here. # Build a dict, keyed by resource class internal ID, of lists of # internal IDs of resource providers that share some inventory for # each resource class requested. - # TODO(jaypipes): Consider caching this for some amount of time - # since sharing providers generally don't change often and here we - # aren't concerned with how *much* inventory/capacity the sharing - # provider has, only that it is sharing *some* inventory of a - # particular resource class. - sharing_providers = { - rc_id: _get_providers_with_shared_capacity(context, rc_id, - amount, member_of) - for rc_id, amount in resources.items() - } - # If there aren't any providers that have any of the # required traits, just exit early... if required_trait_map: @@ -4064,9 +4058,23 @@ class AllocationCandidates(base.VersionedObject): @db_api.placement_context_manager.writer def _get_by_requests(cls, context, requests, limit=None, group_policy=None): + # TODO(jaypipes): Make a RequestGroupContext object and put these + # pieces of information in there, passing the context to the various + # internal functions handling that part of the request. + sharing = {} + for request in requests.values(): + member_of = request.member_of + for rc_name, amount in request.resources.items(): + rc_id = _RC_CACHE.id_from_string(rc_name) + if rc_id not in sharing: + sharing[rc_id] = _get_providers_with_shared_capacity( + context, rc_id, amount, member_of) + has_trees = _has_provider_trees(context) + candidates = {} for suffix, request in requests.items(): - alloc_reqs, summaries = cls._get_by_one_request(context, request) + alloc_reqs, summaries = cls._get_by_one_request( + context, request, sharing, has_trees) LOG.debug("%s (suffix '%s') returned %d matches", str(request), str(suffix), len(alloc_reqs)) if not alloc_reqs: