From 346509f8fa2f0410aba5db07f75577454502dd28 Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Wed, 22 May 2019 12:42:21 +0100 Subject: [PATCH] Avoid traversing summaries in _check_traits_for_alloc_request While doing other work, I noticed that _check_traits_for_alloc_request traverses a list of summaries multiple time to find the "right one". This is a classic case of a list being the wrong data structure for the job. Instead use the request resource resource provider's id to key into the summaries dict directly to get the relevant summary. Then, since we know the summary has the trait info we need, we don't need to pass prov_traits to the method. Change-Id: Ic2f8f7bdc2984db2011cec329fb6f5b9efec5a0f --- placement/objects/allocation_candidate.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/placement/objects/allocation_candidate.py b/placement/objects/allocation_candidate.py index 541cca7c6..364c8c381 100644 --- a/placement/objects/allocation_candidate.py +++ b/placement/objects/allocation_candidate.py @@ -422,7 +422,7 @@ def _alloc_candidates_multiple_providers( # (ARR(rc1, ss2), ARR(rc2, ss2), ARR(rc3, ss1))] for res_requests in itertools.product(*request_groups): if not _check_traits_for_alloc_request( - res_requests, summaries, prov_traits, required_traits, + res_requests, summaries, required_traits, forbidden_traits): # This combination doesn't satisfy trait constraints continue @@ -598,8 +598,8 @@ def _build_provider_summaries(context, usages, prov_traits): return summaries -def _check_traits_for_alloc_request(res_requests, summaries, prov_traits, - required_traits, forbidden_traits): +def _check_traits_for_alloc_request(res_requests, summaries, required_traits, + forbidden_traits): """Given a list of AllocationRequestResource objects, check if that combination can provide trait constraints. If it can, returns all resource provider internal IDs in play, else return an empty list. @@ -611,11 +611,9 @@ def _check_traits_for_alloc_request(res_requests, summaries, prov_traits, resource providers to be checked if they collectively satisfy trait constraints in the required_traits and forbidden_traits parameters. - :param summaries: dict, keyed by resource provider ID, of ProviderSummary + :param summaries: dict, keyed by resource provider id, of ProviderSummary objects containing usage and trait information for resource providers involved in the overall request - :param prov_traits: A dict, keyed by internal resource provider ID, of - string trait names associated with that provider :param required_traits: A map, keyed by trait string name, of required trait internal IDs that each *allocation request's set of providers* must *collectively* have @@ -627,11 +625,9 @@ def _check_traits_for_alloc_request(res_requests, summaries, prov_traits, all_prov_ids = [] all_traits = set() for res_req in res_requests: - rp_uuid = res_req.resource_provider.uuid - for rp_id, summary in summaries.items(): - if summary.resource_provider.uuid == rp_uuid: - break - rp_traits = set(prov_traits.get(rp_id, [])) + rp_id = res_req.resource_provider.id + rp_summary = summaries[rp_id] + rp_traits = set([trait.name for trait in rp_summary.traits]) # Check if there are forbidden_traits conflict_traits = set(forbidden_traits) & set(rp_traits)