openstack
/
heat
Code Issues Proposed changes

Stop using needed_by field in resource 

During the original prototype development, I missed the fact that the
`needed_by` field of a resource is no longer needed for the actual
convergence algorithm:

c74aac1f07

Since nothing is using this any more, we can avoid an unnecessary DB
write to every resource at the start of a stack update. For now, just
write an empty list to that field any time we are storing the resource.
In future, we can stop reading/writing the field altogether, and in a
subsequent release we could drop the column from the DB.

Change-Id: I0c9c77f395db1131b16e5bd9d579a092033400b1
This commit is contained in:
Zane Bitter 2018-04-25 20:22:05 -04:00
parent ff4ff1b839
commit 83da046447
4 changed files with 12 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
heat/engine/resource.py
View File

@ -242,7 +242,6 @@ class Resource(status.ResourceStatus):
self.created_time = stack.created_time
self.updated_time = stack.updated_time
self._rpc_client = None
self.needed_by = []
self.requires = set()
self.replaces = None
self.replaced_by = None
@ -291,7 +290,6 @@ class Resource(status.ResourceStatus):
self._rsrc_prop_data_id = resource.rsrc_prop_data_id
self.created_time = resource.created_at
self.updated_time = resource.updated_at
self.needed_by = resource.needed_by
self.requires = set(resource.requires)
self.replaces = resource.replaces
self.replaced_by = resource.replaced_by
@ -392,7 +390,7 @@ class Resource(status.ResourceStatus):
rs = {'stack_id': self.stack.id,
'name': self.name,
'rsrc_prop_data_id': None,
'needed_by': self.needed_by,
'needed_by': [],
'requires': sorted(requires, reverse=True),
'replaces': self.id,
'action': self.INIT,
@ -529,15 +527,6 @@ class Resource(status.ResourceStatus):
"""
pass
@classmethod
def set_needed_by(cls, db_rsrc, needed_by, expected_engine_id=None):
if db_rsrc:
db_rsrc.select_and_update(
{'needed_by': needed_by},
atomic_key=db_rsrc.atomic_key,
expected_engine_id=expected_engine_id
)
@classmethod
def set_requires(cls, db_rsrc, requires):
if db_rsrc:
@ -1887,8 +1876,8 @@ class Resource(status.ResourceStatus):
raise exception.StackValidationFailed(message=msg, path=path)
def _update_replacement_data(self, template_id):
# Update the replacement resource's needed_by and replaces
# fields. Make sure that the replacement belongs to the given
# Update the replacement resource's replaces field.
# Make sure that the replacement belongs to the given
# template and there is no engine working on it.
if self.replaced_by is None:
return
@ -1899,19 +1888,16 @@ class Resource(status.ResourceStatus):
fields=('current_template_id', 'atomic_key'))
except exception.NotFound:
LOG.info("Could not find replacement of resource %(name)s "
"with id %(id)s while updating needed_by.",
"with id %(id)s while updating replaces.",
{'name': self.name, 'id': self.replaced_by})
return
if (db_res.current_template_id == template_id):
# Following update failure is ignorable; another
# update might have locked/updated the resource.
if db_res.select_and_update(
{'needed_by': self.needed_by,
'replaces': None},
atomic_key=db_res.atomic_key,
expected_engine_id=None):
self._incr_atomic_key(self._atomic_key)
# Following update failure is ignorable; another
# update might have locked/updated the resource.
db_res.select_and_update({'replaces': None},
atomic_key=db_res.atomic_key,
expected_engine_id=None)
def delete_convergence(self, template_id, engine_id, timeout,
progress_callback=None):
@ -1923,10 +1909,9 @@ class Resource(status.ResourceStatus):
Also, since this resource is visited as part of clean-up phase,
the needed_by should be updated. If this resource was
replaced by more recent resource, then delete this and update
the replacement resource's needed_by and replaces fields.
the replacement resource's replaces field.
"""
self._calling_engine_id = engine_id
self.needed_by = []
if self.current_template_id != template_id:
# just delete the resources in INIT state
@ -2073,7 +2058,7 @@ class Resource(status.ResourceStatus):
'name': self.name,
'rsrc_prop_data_id':
self._create_or_replace_rsrc_prop_data(),
'needed_by': self.needed_by,
'needed_by': [],
'requires': sorted(self.requires, reverse=True),
'replaces': self.replaces,
'replaced_by': self.replaced_by,

18
heat/engine/stack.py
View File

@ -1444,31 +1444,15 @@ class Stack(collections.Mapping):
def _update_or_store_resources(self):
self.ext_rsrcs_db = self.db_active_resources_get()
curr_name_translated_dep = self.dependencies.translate(lambda res:
res.name)
rsrcs = {}
def update_needed_by(res):
new_requirers = set(
rsrcs[rsrc_name].id for rsrc_name in
curr_name_translated_dep.required_by(res.name)
)
old_requirers = set(res.needed_by) if res.needed_by else set()
needed_by = old_requirers | new_requirers
res.needed_by = list(needed_by)
for rsrc in reversed(self.dependencies):
existing_rsrc_db = self._get_best_existing_rsrc_db(rsrc.name)
if existing_rsrc_db is None:
update_needed_by(rsrc)
rsrc.current_template_id = self.t.id
rsrc.store()
rsrcs[rsrc.name] = rsrc
else:
update_needed_by(existing_rsrc_db)
resource.Resource.set_needed_by(
existing_rsrc_db, existing_rsrc_db.needed_by
)
rsrcs[existing_rsrc_db.name] = existing_rsrc_db
return rsrcs
@ -1477,9 +1461,7 @@ class Stack(collections.Mapping):
res.id)
ext_rsrcs_db = self.db_active_resources_get()
for r in self.dependencies:
r.needed_by = list(curr_name_translated_dep.required_by(r.id))
r.requires = list(curr_name_translated_dep.requires(r.id))
resource.Resource.set_needed_by(ext_rsrcs_db[r.id], r.needed_by)
resource.Resource.set_requires(ext_rsrcs_db[r.id], r.requires)
def _compute_convg_dependencies(self, existing_resources,

42
heat/tests/test_convg_stack.py
View File

@ -99,20 +99,6 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase):
[[2, True], [3, True]]]), # D, C
sorted(stack_db.current_deps['edges']))
# check if needed_by is stored properly
expected_needed_by = {'A': [3], 'B': [3],
'C': [1, 2],
'D': [], 'E': []}
rsrcs_db = resource_objects.Resource.get_all_by_stack(
stack_db._context, stack_db.id
)
self.assertEqual(5, len(rsrcs_db))
for rsrc_name, rsrc_obj in rsrcs_db.items():
self.assertEqual(sorted(expected_needed_by[rsrc_name]),
sorted(rsrc_obj.needed_by))
self.assertEqual(stack_db.raw_template_id,
rsrc_obj.current_template_id)
# check if sync_points were stored
for entity_id in [5, 4, 3, 2, 1, stack_db.id]:
sync_point = sync_point_object.SyncPoint.get_by_key(
@ -226,23 +212,6 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase):
Leaves are at the bottom
'''
# check if needed_by are stored properly
# For A & B:
# needed_by=C, F
expected_needed_by = {'A': [3, 8], 'B': [3, 8],
'C': [1, 2],
'D': [], 'E': [],
'F': [6, 7],
'G': [], 'H': []}
rsrcs_db = resource_objects.Resource.get_all_by_stack(
stack_db._context, stack_db.id
)
self.assertEqual(8, len(rsrcs_db))
for rsrc_name, rsrc_obj in rsrcs_db.items():
self.assertEqual(sorted(expected_needed_by[rsrc_name]),
sorted(rsrc_obj.needed_by))
# check if sync_points are created for forward traversal
# [F, H, G, A, B, Stack]
for entity_id in [8, 7, 6, 5, 4, stack_db.id]:
@ -321,17 +290,6 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase):
[[4, False], [3, False]]]),
sorted(stack_db.current_deps['edges']))
expected_needed_by = {'A': [3], 'B': [3],
'C': [1, 2],
'D': [], 'E': []}
rsrcs_db = resource_objects.Resource.get_all_by_stack(
stack_db._context, stack_db.id
)
self.assertEqual(5, len(rsrcs_db))
for rsrc_name, rsrc_obj in rsrcs_db.items():
self.assertEqual(sorted(expected_needed_by[rsrc_name]),
sorted(rsrc_obj.needed_by))
# check if sync_points are created for cleanup traversal
# [A, B, C, D, E, Stack]
for entity_id in [5, 4, 3, 2, 1, stack_db.id]:

17
heat/tests/test_resource.py
View File

@ -2461,26 +2461,11 @@ class ResourceTest(common.HeatTestCase):
res.name)
self.assertEqual(msg, six.text_type(ex))
def test_delete_convergence_updates_needed_by(self):
tmpl = rsrc_defn.ResourceDefinition('test_res', 'Foo')
res = generic_rsrc.GenericResource('test_res', tmpl, self.stack)
res.current_template_id = 1
res.status = res.COMPLETE
res.action = res.CREATE
res.store()
res.destroy = mock.Mock()
self._assert_resource_lock(res.id, None, None)
scheduler.TaskRunner(res.delete_convergence, 1,
'engine-007', self.dummy_timeout,
self.dummy_event)()
self.assertItemsEqual([], res.needed_by)
@mock.patch.object(resource_objects.Resource, 'get_obj')
def test_update_replacement_data(self, mock_get_obj):
tmpl = rsrc_defn.ResourceDefinition('test_res', 'Foo')
r = generic_rsrc.GenericResource('test_res', tmpl, self.stack)
r.replaced_by = 4
r.needed_by = [4, 5]
r.store()
db_res = mock.MagicMock()
db_res.current_template_id = 'same_tmpl'
@ -2489,7 +2474,7 @@ class ResourceTest(common.HeatTestCase):
self.assertTrue(mock_get_obj.called)
self.assertTrue(db_res.select_and_update.called)
args, kwargs = db_res.select_and_update.call_args
self.assertEqual({'replaces': None, 'needed_by': [4, 5]}, args[0])
self.assertEqual({'replaces': None}, args[0])
self.assertIsNone(kwargs['expected_engine_id'])
@mock.patch.object(resource_objects.Resource, 'get_obj')