From efabef60d003c67587036f504da3444c7df5294c Mon Sep 17 00:00:00 2001 From: Crag Wolfe Date: Tue, 10 Jan 2017 16:12:54 -0800 Subject: [PATCH] Efficient passing of attrs during traversals Two improvements: Do not iterate through stack outputs when determining what attributes to send as input_data to the next resources in a traversal; it is at best extra processing and at worse results in extra attributes being included in input_data. Do not re-resolve attributes / re-calculate input_data when we already have it. I.e., when a resource constructs input_data to send to a requirer resource, that same input_data may be used for all other requirer resources. Change-Id: I64089fb0774c10f172d986c3f87090e91cb3f263 Closes-Bug: #1656125 --- heat/engine/check_resource.py | 17 +++++++----- heat/engine/stack.py | 11 +++----- heat/tests/test_stack_collect_attributes.py | 30 +++++++-------------- 3 files changed, 25 insertions(+), 33 deletions(-) diff --git a/heat/engine/check_resource.py b/heat/engine/check_resource.py index 4164e73a93..afcd10d9fe 100644 --- a/heat/engine/check_resource.py +++ b/heat/engine/check_resource.py @@ -203,9 +203,13 @@ class CheckResource(object): # for the next traversal. graph_key = (rsrc.replaces, is_update) - def _get_input_data(req, fwd): + def _get_input_data(req, fwd, input_forward_data=None): if fwd: - return construct_input_data(rsrc, stack) + if input_forward_data is None: + return construct_input_data(rsrc, stack) + else: + # do not re-resolve attrs + return input_forward_data else: # Don't send data if initiating clean-up for self i.e. # initiating delete of a replaced resource @@ -217,8 +221,11 @@ class CheckResource(object): return None try: + input_forward_data = None for req, fwd in deps.required_by(graph_key): - input_data = _get_input_data(req, fwd) + input_data = _get_input_data(req, fwd, input_forward_data) + if fwd: + input_forward_data = input_data propagate_check_resource( cnxt, self._rpc_client, req, current_traversal, set(graph[(req, fwd)]), graph_key, input_data, fwd, @@ -311,9 +318,7 @@ def _resolve_attributes(dep_attrs, rsrc): def construct_input_data(rsrc, curr_stack): dep_attrs = curr_stack.get_dep_attrs( six.itervalues(curr_stack.resources), - curr_stack.outputs, - rsrc.name, - curr_stack.t.OUTPUT_VALUE) + rsrc.name) input_data = {'id': rsrc.id, 'name': rsrc.name, 'reference_id': rsrc.get_reference_id(), diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 4561fb66c8..e6bcea5d4d 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -462,17 +462,14 @@ class Stack(collections.Mapping): LOG.warning(_LW("Unable to set parameters StackId identifier")) @staticmethod - def get_dep_attrs(resources, outputs, resource_name, value_sec): + def get_dep_attrs(resources, resource_name): """Return the attributes of the specified resource that are referenced. Return an iterator over any attributes of the specified resource that - are referenced. + are referenced in resources. """ - attr_lists = itertools.chain((res.dep_attrs(resource_name) - for res in resources), - (out.dep_attrs(resource_name) - for out in six.itervalues(outputs))) - return set(itertools.chain.from_iterable(attr_lists)) + return set(itertools.chain.from_iterable( + res.dep_attrs(resource_name) for res in resources)) def _get_dependencies(self, ignore_errors=True): """Return the dependency graph for a list of resources.""" diff --git a/heat/tests/test_stack_collect_attributes.py b/heat/tests/test_stack_collect_attributes.py index af0e348612..71c3c93f15 100644 --- a/heat/tests/test_stack_collect_attributes.py +++ b/heat/tests/test_stack_collect_attributes.py @@ -192,8 +192,7 @@ class DepAttrsTest(common.HeatTestCase): ('one_res_several_attrs', dict(tmpl=tmpl3, expected={'AResource': {'attr_A1', 'attr_A2', 'attr_A3', - 'meta_A1', 'meta_A2', 'out_A1', - 'out_A2'}, + 'meta_A1', 'meta_A2'}, 'BResource': set()})), ('several_res_one_attr', dict(tmpl=tmpl4, @@ -203,25 +202,19 @@ class DepAttrsTest(common.HeatTestCase): 'DResource': set()})), ('several_res_several_attrs', dict(tmpl=tmpl5, - expected={'AResource': {'attr_A1', 'attr_A2', 'meta_A1', - 'attr_A3', 'attr_A4'}, - 'BResource': {'attr_B1', 'attr_B2', 'meta_B2', - 'attr_B3'}, + expected={'AResource': {'attr_A1', 'attr_A2', 'meta_A1'}, + 'BResource': {'attr_B1', 'attr_B2', 'meta_B2'}, 'CResource': set()})), ('nested_attr', dict(tmpl=tmpl6, - expected={'AResource': set([(u'flat_dict', u'key2'), - (u'list', 1), - (u'nested_dict', u'dict', u'b'), - (u'nested_dict', u'string')]), - 'BResource': set(['attr_B3'])})), + expected={'AResource': set([(u'list', 1), + (u'nested_dict', u'dict', u'b')]), + 'BResource': set([])})), ('several_res_several_attrs_and_all_attrs', dict(tmpl=tmpl7, - expected={'AResource': {'attr_A1', 'attr_A2', 'meta_A1', - 'attr_A3', 'attr_A4'}, - 'BResource': {'attr_B1', 'attr_B2', 'meta_B2', - 'attr_B3'}, - 'CResource': {'foo', 'Foo', 'show'}})) + expected={'AResource': {'attr_A1', 'attr_A2', 'meta_A1'}, + 'BResource': {'attr_B1', 'attr_B2', 'meta_B2'}, + 'CResource': set()})) ] def setUp(self): @@ -234,11 +227,8 @@ class DepAttrsTest(common.HeatTestCase): template.Template(parsed_tmpl)) for res in six.itervalues(self.stack): - outputs = self.stack.outputs resources = six.itervalues(self.stack.resources) self.assertEqual(self.expected[res.name], self.stack.get_dep_attrs( resources, - outputs, - res.name, - self.stack.t.OUTPUT_VALUE)) + res.name))