openstack/placement
Code Issues Proposed changes

Cache provider ids in requested aggregates

Browse Source 
Getting allocation candidates for one request group, placement
internally calls provider_ids_matching_aggregates() several times with
the same arguments. This is redundant and can degrade performance.

This patch changes to call it on initialization of
`RequestGroupSearchContext` object and cache the results so that we can
refer the results later on demand.

Change-Id: I93e1fa81a7345b651ea2d58dfa0d5508e726f43f
Story: 2005712
Task: 31038
This commit is contained in:
Tetsuro Nakamura 2019-05-16 05:37:14 +00:00 committed by Eric Fried
parent fb71a6ab71
commit 7f4b79b7e1
3 changed files with 64 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
placement/objects/research_context.py
View File

@ -72,6 +72,14 @@ class RequestGroupSearchContext(object):
# that request group must not be members of
self.forbidden_aggs = request.forbidden_aggs
# A set of provider ids that matches the requested positive aggregates
self.rps_in_aggs = set()
if self.member_of:
self.rps_in_aggs = provider_ids_matching_aggregates(
context, self.member_of)
if not self.rps_in_aggs:
raise exception.ResourceProviderNotFound()
# If True, this RequestGroup represents requests which must be
# satisfied by a single resource provider. If False, represents a
# request for resources in any resource provider in the same tree,
@ -314,10 +322,9 @@ def get_provider_ids_matching(rg_ctx):
:param rg_ctx: RequestGroupSearchContext
"""
# TODO(tetsuro): refactor this to have only the rg_ctx argument
filtered_rps, forbidden_rp_ids = get_provider_ids_for_traits_and_aggs(
rg_ctx.context, rg_ctx.required_trait_map, rg_ctx.forbidden_trait_map,
rg_ctx.member_of, rg_ctx.forbidden_aggs)
rg_ctx)
if filtered_rps is None:
# If no providers match the traits/aggs, we can short out
return []
@ -413,17 +420,6 @@ def get_trees_matching_all(rg_ctx):
:param rg_ctx: RequestGroupSearchContext
"""
# If 'member_of' has values, do a separate lookup to identify the
# resource providers that meet the member_of constraints.
if rg_ctx.member_of:
rps_in_aggs = provider_ids_matching_aggregates(
rg_ctx.context, rg_ctx.member_of)
if not rps_in_aggs:
# Short-circuit. The user either asked for a non-existing
# aggregate or there were no resource providers that matched
# the requirements...
return rp_candidates.RPCandidateList()
if rg_ctx.forbidden_aggs:
rps_bad_aggs = provider_ids_matching_aggregates(
rg_ctx.context, [rg_ctx.forbidden_aggs])
@ -470,7 +466,7 @@ def get_trees_matching_all(rg_ctx):
if rg_ctx.member_of:
# Aggregate on root spans the whole tree, so the rp itself
# *or its root* should be in the aggregate
provs_with_inv_rc.filter_by_rp_or_tree(rps_in_aggs)
provs_with_inv_rc.filter_by_rp_or_tree(rg_ctx.rps_in_aggs)
LOG.debug("found %d providers under %d trees after applying "
"aggregate filter %s",
len(provs_with_inv_rc.rps), len(provs_with_inv_rc.trees),
@ -751,9 +747,7 @@ def _get_provider_ids_having_all_traits(ctx, required_traits):
return set(r[0] for r in ctx.session.execute(sel))
def get_provider_ids_for_traits_and_aggs(ctx, required_traits,
forbidden_traits, member_of,
forbidden_aggs):
def get_provider_ids_for_traits_and_aggs(rg_ctx):
"""Get internal IDs for all providers matching the specified traits/aggs.
:return: A tuple of:
@ -765,48 +759,53 @@ def get_provider_ids_for_traits_and_aggs(ctx, required_traits,
specified forbidden_traits.
"""
filtered_rps = set()
if required_traits:
trait_map = _normalize_trait_map(ctx, required_traits)
trait_rps = _get_provider_ids_having_all_traits(ctx, trait_map)
if rg_ctx.required_trait_map:
trait_map = _normalize_trait_map(rg_ctx.context,
rg_ctx.required_trait_map)
trait_rps = _get_provider_ids_having_all_traits(
rg_ctx.context, trait_map)
filtered_rps = trait_rps
LOG.debug("found %d providers after applying required traits filter "
"(%s)",
len(filtered_rps), list(required_traits))
len(filtered_rps), list(rg_ctx.required_trait_map))
if not filtered_rps:
return None, []
# If 'member_of' has values, do a separate lookup to identify the
# resource providers that meet the member_of constraints.
if member_of:
rps_in_aggs = provider_ids_matching_aggregates(ctx, member_of)
if rg_ctx.member_of:
if filtered_rps:
filtered_rps &= rps_in_aggs
filtered_rps &= rg_ctx.rps_in_aggs
else:
filtered_rps = rps_in_aggs
filtered_rps = rg_ctx.rps_in_aggs
LOG.debug("found %d providers after applying required aggregates "
"filter (%s)", len(filtered_rps), member_of)
"filter (%s)", len(filtered_rps), rg_ctx.member_of)
if not filtered_rps:
return None, []
forbidden_rp_ids = set()
if forbidden_aggs:
rps_bad_aggs = provider_ids_matching_aggregates(ctx, [forbidden_aggs])
if rg_ctx.forbidden_aggs:
rps_bad_aggs = provider_ids_matching_aggregates(
rg_ctx.context, [rg_ctx.forbidden_aggs])
forbidden_rp_ids |= rps_bad_aggs
if filtered_rps:
filtered_rps -= rps_bad_aggs
LOG.debug("found %d providers after applying forbidden aggregates "
"filter (%s)", len(filtered_rps), forbidden_aggs)
"filter (%s)", len(filtered_rps), rg_ctx.forbidden_aggs)
if not filtered_rps:
return None, []
if forbidden_traits:
trait_map = _normalize_trait_map(ctx, forbidden_traits)
rps_bad_traits = get_provider_ids_having_any_trait(ctx, trait_map)
if rg_ctx.forbidden_trait_map:
trait_map = _normalize_trait_map(
rg_ctx.context, rg_ctx.forbidden_trait_map)
rps_bad_traits = get_provider_ids_having_any_trait(
rg_ctx.context, trait_map)
forbidden_rp_ids |= rps_bad_traits
if filtered_rps:
filtered_rps -= rps_bad_traits
LOG.debug("found %d providers after applying forbidden traits "
"filter (%s)", len(filtered_rps), list(forbidden_traits))
"filter (%s)", len(filtered_rps),
list(rg_ctx.forbidden_trait_map))
if not filtered_rps:
return None, []

43
placement/objects/resource_provider.py
View File

@ -896,6 +896,7 @@ def _get_all_by_filters_from_db(context, filters):
required = required - forbidden
forbidden = set([trait.lstrip('!') for trait in forbidden])
resources = filters.pop('resources', {})
in_tree = filters.pop('in_tree', None)
rp = sa.alias(_RP_TBL, name="rp")
root_rp = sa.alias(_RP_TBL, name="root_rp")
@ -925,36 +926,44 @@ def _get_all_by_filters_from_db(context, filters):
query = query.where(rp.c.name == name)
if uuid:
query = query.where(rp.c.uuid == uuid)
if 'in_tree' in filters:
if in_tree:
# The 'in_tree' parameter is the UUID of a resource provider that
# the caller wants to limit the returned providers to only those
# within its "provider tree". So, we look up the resource provider
# having the UUID specified by the 'in_tree' parameter and grab the
# root_provider_id value of that record. We can then ask for only
# those resource providers having a root_provider_id of that value.
tree_uuid = filters.pop('in_tree')
tree_ids = res_ctx.provider_ids_from_uuid(context, tree_uuid)
tree_ids = res_ctx.provider_ids_from_uuid(context, in_tree)
if tree_ids is None:
# List operations should simply return an empty list when a
# non-existing resource provider UUID is given.
return []
root_id = tree_ids.root_id
query = query.where(rp.c.root_provider_id == root_id)
# Get the provider IDs matching any specified traits and/or aggregates
rp_ids, forbidden_rp_ids = res_ctx.get_provider_ids_for_traits_and_aggs(
context, required, forbidden, member_of, forbidden_aggs)
if rp_ids is None:
# If no providers match the traits/aggs, we can short out
if required:
trait_map = trait_obj.ids_from_names(context, required)
trait_rps = res_ctx._get_provider_ids_having_all_traits(
context, trait_map)
if not trait_rps:
return []
if rp_ids:
query = query.where(rp.c.id.in_(rp_ids))
# forbidden providers, if found, are mutually exclusive with matching
# providers above, so we only need to include this clause if we didn't
# use the positive filter above.
elif forbidden_rp_ids:
query = query.where(~rp.c.id.in_(forbidden_rp_ids))
query = query.where(rp.c.id.in_(trait_rps))
if forbidden:
trait_map = trait_obj.ids_from_names(context, forbidden)
trait_rps = res_ctx.get_provider_ids_having_any_trait(
context, trait_map)
if trait_rps:
query = query.where(~rp.c.id.in_(trait_rps))
if member_of:
rps_in_aggs = res_ctx.provider_ids_matching_aggregates(
context, member_of)
if not rps_in_aggs:
return []
query = query.where(rp.c.id.in_(rps_in_aggs))
if forbidden_aggs:
rps_bad_aggs = res_ctx.provider_ids_matching_aggregates(
context, [forbidden_aggs])
if rps_bad_aggs:
query = query.where(~rp.c.id.in_(rps_bad_aggs))
for rc_name, amount in resources.items():
rc_id = rc_cache.RC_CACHE.id_from_string(rc_name)
rps_with_resource = res_ctx.get_providers_with_resource(

8
tox.ini
View File

@ -146,10 +146,10 @@ enable-extensions = H106,H203,H904
# a natural summary line. Rejecting code for this reason is wrong.
ignore = H405
exclude = .venv,.git,.tox,dist,*lib/python*,*egg,build,releasenotes
# To get a list of functions that have a complexity of 15 or more, set
# max-complexity to 15 and run 'tox -epep8'.
# 14 is currently the most complex thing we have
max-complexity=15
# To get a list of functions that have a complexity of 17 or more, set
# max-complexity to 17 and run 'tox -epep8'.
# 16 is currently the most complex thing we have
max-complexity=17
[testenv:bindep]
# Do not install any requirements. We want this to be fast and work even if