openstack/heat
Code Issues Proposed changes

Eager load resource_properties_data in resource

Browse Source 
Eager load resource_properties_data in resources in the typical
resource-loading scenarios where properties data will be
accessed. Thus, we can save an extra db query per resource when
loading all the resources in a stack, for instance. Fall back to lazy
loading properties data in other scenarios.

Also, the resource object doesn't need to store a copy of its
ResourcePropertiesData object in self.rsrc_prop_data, so don't.

Change-Id: Ib7684af3fe06f818628fd21f1216de5047872948
Closes-Bug: #1665503
This commit is contained in:
Crag Wolfe 2017-03-21 01:52:50 -04:00 committed by Zane Bitter
parent 8c7b66a8a3
commit 705625cee7
8 changed files with 102 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
heat/db/sqlalchemy/api.py
View File

@ -174,9 +174,14 @@ def raw_template_files_get(context, files_id):
return result
def resource_get(context, resource_id, refresh=False, refresh_data=False):
result = context.session.query(models.Resource).get(resource_id)
def resource_get(context, resource_id, refresh=False, refresh_data=False,
eager=True):
query = context.session.query(models.Resource)
if eager:
query = query.options(orm.joinedload("data")).options(
orm.joinedload("rsrc_prop_data"))
result = query.get(resource_id)
if not result:
raise exception.NotFound(_("resource with id %s not found") %
resource_id)
@ -443,7 +448,7 @@ def resource_get_all_by_stack(context, stack_id, filters=None):
models.Resource
).filter_by(
stack_id=stack_id
).options(orm.joinedload("data"))
).options(orm.joinedload("data")).options(orm.joinedload("rsrc_prop_data"))
query = db_filters.exact_filter(query, models.Resource, filters)
results = query.all()

21
heat/engine/event.py
View File

@ -13,14 +13,14 @@
from heat.common import identifier
from heat.objects import event as event_object
from heat.objects import resource_properties_data as rpd_objects
class Event(object):
"""Class representing a Resource state change."""
def __init__(self, context, stack, action, status, reason,
physical_resource_id, resource_properties, resource_name,
physical_resource_id, resource_prop_data_id,
resource_properties, resource_name,
resource_type, uuid=None, timestamp=None, id=None):
"""Initialise from a context, stack, and event information.
@ -35,17 +35,10 @@ class Event(object):
self.physical_resource_id = physical_resource_id
self.resource_name = resource_name
self.resource_type = resource_type
self.rsrc_prop_data = None
if isinstance(resource_properties,
rpd_objects.ResourcePropertiesData):
self.rsrc_prop_data = resource_properties
self.resource_properties = self.rsrc_prop_data.data
elif resource_properties is None:
self.rsrc_prop_data_id = resource_prop_data_id
self.resource_properties = resource_properties
if self.resource_properties is None:
self.resource_properties = {}
else:
raise AssertionError(
'resource_properties is unexpected type %s' %
type(resource_properties).__name__)
self.uuid = uuid
self.timestamp = timestamp
self.id = id
@ -68,8 +61,8 @@ class Event(object):
if self.timestamp is not None:
ev['created_at'] = self.timestamp
if self.rsrc_prop_data:
ev['rsrc_prop_data_id'] = self.rsrc_prop_data.id
if self.rsrc_prop_data_id is not None:
ev['rsrc_prop_data_id'] = self.rsrc_prop_data_id
new_ev = event_object.Event.create(self.context, ev)

20
heat/engine/resource.py
View File

@ -240,7 +240,7 @@ class Resource(status.ResourceStatus):
self._data = None
self._attr_data_id = None
self._rsrc_metadata = None
self._rsrc_prop_data = None
self._rsrc_prop_data_id = None
self._stored_properties_data = None
self.created_time = stack.created_time
self.updated_time = stack.updated_time
@ -289,7 +289,7 @@ class Resource(status.ResourceStatus):
self._attr_data_id = resource.attr_data_id
self._rsrc_metadata = resource.rsrc_metadata
self._stored_properties_data = resource.properties_data
self._rsrc_prop_data = resource.rsrc_prop_data
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
@ -944,7 +944,7 @@ class Resource(status.ResourceStatus):
old_props = self._stored_properties_data
self._stored_properties_data = function.resolve(self.properties.data)
if self._stored_properties_data != old_props:
self._rsrc_prop_data = None
self._rsrc_prop_data_id = None
self.attributes.reset_resolved_values()
def referenced_attrs(self, stk_defn=None,
@ -2022,8 +2022,8 @@ class Resource(status.ResourceStatus):
"""Add a state change event to the database."""
physical_res_id = self.resource_id or self.physical_resource_name()
ev = event.Event(self.context, self.stack, action, status, reason,
physical_res_id, self._rsrc_prop_data,
self.name, self.type())
physical_res_id, self._rsrc_prop_data_id,
self._stored_properties_data, self.name, self.type())
ev.store()
self.stack.dispatch_event(ev)
@ -2487,16 +2487,16 @@ class Resource(status.ResourceStatus):
return True
def _create_or_replace_rsrc_prop_data(self):
if self._rsrc_prop_data is not None:
return self._rsrc_prop_data.id
if self._rsrc_prop_data_id is not None:
return self._rsrc_prop_data_id
if not self._stored_properties_data:
return None
self._rsrc_prop_data = \
self._rsrc_prop_data_id = \
rpd_objects.ResourcePropertiesData(self.context).create(
self.context, self._stored_properties_data)
return self._rsrc_prop_data.id
self.context, self._stored_properties_data).id
return self._rsrc_prop_data_id
def is_using_neutron(self):
try:

2
heat/engine/stack.py
View File

@ -922,7 +922,7 @@ class Stack(collections.Mapping):
def _add_event(self, action, status, reason):
"""Add a state change event to the database."""
ev = event.Event(self.context, self, action, status, reason,
self.id, None,
self.id, None, None,
self.name, 'OS::Heat::Stack')
ev.store()

29
heat/objects/resource.py
View File

@ -85,8 +85,6 @@ class Resource(
resource_data.ResourceData,
nullable=True
),
'rsrc_prop_data': fields.ObjectField(
rpd.ResourcePropertiesData, nullable=True),
'rsrc_prop_data_id': fields.ObjectField(
fields.IntegerField(nullable=True)),
'engine_id': fields.StringField(nullable=True),
@ -114,12 +112,21 @@ class Resource(
elif field != 'attr_data':
resource[field] = db_resource[field]
if db_resource['rsrc_prop_data'] is not None:
resource['rsrc_prop_data'] = \
rpd.ResourcePropertiesData._from_db_object(
rpd.ResourcePropertiesData(context), context,
db_resource['rsrc_prop_data'])
resource._properties_data = resource['rsrc_prop_data'].data
if db_resource['rsrc_prop_data_id'] is not None:
if hasattr(db_resource, '__dict__'):
rpd_obj = db_resource.__dict__.get('rsrc_prop_data')
else:
rpd_obj = None
if rpd_obj is not None:
# Object is already eager loaded
rpd_obj = (
rpd.ResourcePropertiesData._from_db_object(
rpd.ResourcePropertiesData(),
context,
rpd_obj))
resource._properties_data = rpd_obj.data
else:
resource._properties_data = {}
if db_resource['properties_data']:
LOG.error(
'Unexpected condition where resource.rsrc_prop_data '
@ -155,6 +162,12 @@ class Resource(
@property
def properties_data(self):
if (not self._properties_data and
self.rsrc_prop_data_id is not None):
LOG.info('rsrp_prop_data lazy load')
rpd_obj = rpd.ResourcePropertiesData.get_by_id(
self._context, self.rsrc_prop_data_id)
self._properties_data = rpd_obj.data or {}
return self._properties_data
@classmethod

7
heat/tests/test_engine_api_utils.py
View File

@ -63,7 +63,9 @@ class FormatTest(common.HeatTestCase):
ev = event.Event(self.context, self.stack, 'CREATE',
'COMPLETE', 'state changed',
'z3455xyc-9f88-404d-a85b-5315293e67de',
res_properties, resource.name, resource.type(),
resource._rsrc_prop_data_id,
resource._stored_properties_data,
resource.name, resource.type(),
uuid=ev_uuid)
ev.store()
return event_object.Event.get_all_by_stack(
@ -322,7 +324,8 @@ class FormatTest(common.HeatTestCase):
resource = self.stack['generic1']
resource._update_stored_properties()
resource.store()
event = self._dummy_event(res_properties=resource._rsrc_prop_data)
event = self._dummy_event(
res_properties=resource._stored_properties_data)
formatted = api.format_event(event, self.stack.identifier(),
include_rsrc_prop_data=True)
self.assertEqual({'k1': 'v1'}, formatted[rpc_api.EVENT_RES_PROPERTIES])

37
heat/tests/test_event.py
View File

@ -85,14 +85,16 @@ class EventTest(EventCommon):
self.resource.resource_id_set('resource_physical_id')
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'alabama', self.resource._rsrc_prop_data,
'alabama', self.resource._rsrc_prop_data_id,
self.resource._stored_properties_data,
self.resource.name, self.resource.type())
e.store()
self.assertEqual(1, len(event_object.Event.get_all_by_stack(
self.ctx,
self.stack.id)))
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'arizona', self.resource._rsrc_prop_data,
'arizona', self.resource._rsrc_prop_data_id,
self.resource._stored_properties_data,
self.resource.name, self.resource.type())
e.store()
events = event_object.Event.get_all_by_stack(self.ctx, self.stack.id)
@ -105,7 +107,7 @@ class EventTest(EventCommon):
self.resource.resource_id_set('resource_physical_id')
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'arkansas', self.resource._rsrc_prop_data,
'arkansas', None, None,
self.resource.name, self.resource.type())
e.store()
@ -114,7 +116,7 @@ class EventTest(EventCommon):
mock_random_uniform.return_value = 2.0 / 100 - .0001
e = event.Event(self.ctx, self.stack, 'TEST',
'IN_PROGRESS', 'Testing',
'alaska', self.resource._rsrc_prop_data,
'alaska', None, None,
self.resource.name, self.resource.type())
e.store()
events = event_object.Event.get_all_by_stack(self.ctx, self.stack.id)
@ -126,7 +128,7 @@ class EventTest(EventCommon):
mock_random_uniform.return_value = 2.0 / 100 + .0001
e = event.Event(self.ctx, self.stack, 'TEST',
'IN_PROGRESS', 'Testing',
'aardvark', self.resource._rsrc_prop_data,
'aardvark', None, None,
self.resource.name, self.resource.type())
e.store()
events = event_object.Event.get_all_by_stack(self.ctx, self.stack.id)
@ -138,16 +140,17 @@ class EventTest(EventCommon):
self.resource.resource_id_set('resource_physical_id')
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'alabama', self.resource._rsrc_prop_data,
'alabama', self.resource._rsrc_prop_data_id,
self.resource._stored_properties_data,
self.resource.name, self.resource.type())
e.store()
rpd1_id = self.resource._rsrc_prop_data.id
rpd1_id = self.resource._rsrc_prop_data_id
rpd2 = rpd_object.ResourcePropertiesData.create(
self.ctx, {'encrypted': False, 'data': {'foo': 'bar'}})
rpd2_id = rpd2.id
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'arizona', rpd2,
'arizona', rpd2_id, rpd2.data,
self.resource.name, self.resource.type())
e.store()
@ -155,7 +158,7 @@ class EventTest(EventCommon):
self.ctx, {'encrypted': False, 'data': {'foo': 'bar'}})
rpd3_id = rpd3.id
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'arkansas', rpd3,
'arkansas', rpd3_id, rpd3.data,
self.resource.name, self.resource.type())
e.store()
@ -163,7 +166,7 @@ class EventTest(EventCommon):
self.ctx, {'encrypted': False, 'data': {'foo': 'bar'}})
rpd4_id = rpd4.id
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'arkansas', rpd4,
'arkansas', rpd4_id, rpd4.data,
self.resource.name, self.resource.type())
e.store()
@ -187,7 +190,8 @@ class EventTest(EventCommon):
def test_identifier(self):
event_uuid = 'abc123yc-9f88-404d-a85b-531529456xyz'
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'wibble', self.resource._rsrc_prop_data,
'wibble', self.resource._rsrc_prop_data_id,
self.resource._stored_properties_data,
self.resource.name, self.resource.type(),
uuid=event_uuid)
@ -202,7 +206,7 @@ class EventTest(EventCommon):
def test_identifier_is_none(self):
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'wibble', self.resource._rsrc_prop_data,
'wibble', None, None,
self.resource.name, self.resource.type())
self.assertIsNone(e.identifier())
@ -211,7 +215,8 @@ class EventTest(EventCommon):
def test_as_dict(self):
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'wibble', self.resource._rsrc_prop_data,
'wibble', self.resource._rsrc_prop_data_id,
self.resource._stored_properties_data,
self.resource.name, self.resource.type())
e.store()
@ -233,7 +238,8 @@ class EventTest(EventCommon):
def test_load_deprecated_prop_data(self):
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'wibble', self.resource._rsrc_prop_data,
'wibble', self.resource._rsrc_prop_data_id,
self.resource._stored_properties_data,
self.resource.name, self.resource.type())
e.store()
@ -273,7 +279,8 @@ class EventEncryptedTest(EventCommon):
def test_props_encrypted(self):
e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
'wibble', self.resource._rsrc_prop_data,
'wibble', self.resource._rsrc_prop_data_id,
self.resource._stored_properties_data,
self.resource.name, self.resource.type())
e.store()

58
heat/tests/test_resource.py
View File

@ -680,13 +680,13 @@ class ResourceTest(common.HeatTestCase):
ctx2, res_obj2.id)
# verify the resource_properties_data association
# can be set by id or object
self.assertEqual(rpd_db_obj.id, res_obj1.rsrc_prop_data.id)
self.assertEqual(res_obj1.rsrc_prop_data.id,
res_obj2.rsrc_prop_data.id)
# can be set by id
self.assertEqual(rpd_db_obj.id, res_obj1.rsrc_prop_data_id)
self.assertEqual(res_obj1.rsrc_prop_data_id,
res_obj2.rsrc_prop_data_id)
# properties data appears unencrypted to resource object
self.assertEqual(data, res_obj1.rsrc_prop_data.data)
self.assertEqual(data, res_obj2.rsrc_prop_data.data)
self.assertEqual(data, res_obj1.properties_data)
self.assertEqual(data, res_obj2.properties_data)
def test_make_replacement(self):
tmpl = rsrc_defn.ResourceDefinition('test_resource', 'Foo')
@ -921,7 +921,10 @@ class ResourceTest(common.HeatTestCase):
res.state_set(res.CREATE, res.IN_PROGRESS, 'test_rpd')
# Modernity, the data is where it belongs
self.assertEqual(res._rsrc_prop_data.data, {'Foo': 'lucky'})
rsrc_prop_data_db_obj = db_api.resource_prop_data_get(
self.stack.context, res._rsrc_prop_data_id)
self.assertEqual(rsrc_prop_data_db_obj['data'], {'Foo': 'lucky'})
# legacy locations aren't being used anymore
self.assertFalse(hasattr(res, 'properties_data'))
self.assertFalse(hasattr(res, 'properties_data_encrypted'))
@ -962,11 +965,14 @@ class ResourceTest(common.HeatTestCase):
# Modernity, the data is where it belongs
# The db object data is encrypted
rsrc_prop_data_db_obj = db_api.resource_prop_data_get(
self.stack.context, res._rsrc_prop_data.id)
self.stack.context, res._rsrc_prop_data_id)
self.assertNotEqual(rsrc_prop_data_db_obj['data'], {'Foo': 'lucky'})
self.assertEqual(rsrc_prop_data_db_obj.encrypted, True)
# But the objects/ rsrc_prop_data.data is always unencrypted
self.assertEqual(res._rsrc_prop_data.data, {'Foo': 'lucky'})
rsrc_prop_data_obj = rpd_object.ResourcePropertiesData._from_db_object(
rpd_object.ResourcePropertiesData(), self.stack.context,
rsrc_prop_data_db_obj)
self.assertEqual(rsrc_prop_data_obj.data, {'Foo': 'lucky'})
# legacy locations aren't being used anymore
self.assertFalse(hasattr(res, 'properties_data'))
self.assertFalse(hasattr(res, 'properties_data_encrypted'))
@ -1996,20 +2002,20 @@ class ResourceTest(common.HeatTestCase):
# The properties data should be decrypted when the object is
# loaded using get_obj
res_obj = resource_objects.Resource.get_obj(res.context, res.id)
self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
self.assertEqual('string', res_obj.properties_data['prop1'])
# _stored_properties_data should be decrypted when the object is
# loaded using get_all_by_stack
res_objs = resource_objects.Resource.get_all_by_stack(res.context,
self.stack.id)
res_obj = res_objs['test_res_enc']
self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
self.assertEqual('string', res_obj.properties_data['prop1'])
# The properties data should be decrypted when the object is
# refreshed
res_obj = resource_objects.Resource.get_obj(res.context, res.id)
res_obj.refresh()
self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
self.assertEqual('string', res_obj.properties_data['prop1'])
def test_properties_data_no_encryption(self):
cfg.CONF.set_override('encrypt_parameters_and_properties', False)
@ -2040,16 +2046,16 @@ class ResourceTest(common.HeatTestCase):
# is loaded using get_obj
prev_rsrc_prop_data_id = db_res.rsrc_prop_data.id
res_obj = resource_objects.Resource.get_obj(res.context, res.id)
self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
self.assertEqual(prev_rsrc_prop_data_id, res_obj.rsrc_prop_data.id)
self.assertEqual('string', res_obj.properties_data['prop1'])
self.assertEqual(prev_rsrc_prop_data_id, res_obj.rsrc_prop_data_id)
# The properties data should not be modified when the object
# is loaded using get_all_by_stack
res_objs = resource_objects.Resource.get_all_by_stack(res.context,
self.stack.id)
res_obj = res_objs['test_res_enc']
self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
self.assertEqual(prev_rsrc_prop_data_id, res_obj.rsrc_prop_data.id)
self.assertEqual('string', res_obj.properties_data['prop1'])
self.assertEqual(prev_rsrc_prop_data_id, res_obj.rsrc_prop_data_id)
def _assert_resource_lock(self, res_id, engine_id, atomic_key):
rs = resource_objects.Resource.get_obj(self.stack.context, res_id)
@ -4555,19 +4561,11 @@ class TestResourcePropDataUpdate(common.HeatTestCase):
def test_create_or_replace_rsrc_prop_data(self):
res = self.res
res._stored_properties_data = self.old_rpd
if self.replaced:
res._rsrc_prop_data = None
res.store()
if res._rsrc_prop_data is None:
old_rpd_id = -1
else:
old_rpd_id = res._rsrc_prop_data.id
res._stored_properties_data = self.new_rpd
if self.replaced:
res._rsrc_prop_data = None
old_rpd_id = res._rsrc_prop_data_id
with mock.patch("heat.engine.function.resolve") as mock_fr:
mock_fr.return_value = self.new_rpd
res._update_stored_properties()
res.store()
if res._rsrc_prop_data is None:
new_rpd_id = -1
else:
new_rpd_id = res._rsrc_prop_data.id
new_rpd_id = res._rsrc_prop_data_id
self.assertEqual(self.replaced, old_rpd_id != new_rpd_id)