From 705625cee79a566ad5f13b0969a5b2b58f118987 Mon Sep 17 00:00:00 2001 From: Crag Wolfe Date: Tue, 21 Mar 2017 01:52:50 -0400 Subject: [PATCH] Eager load resource_properties_data in resource 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 --- heat/db/sqlalchemy/api.py | 11 ++++-- heat/engine/event.py | 21 ++++------- heat/engine/resource.py | 20 +++++----- heat/engine/stack.py | 2 +- heat/objects/resource.py | 29 +++++++++++---- heat/tests/test_engine_api_utils.py | 7 +++- heat/tests/test_event.py | 37 ++++++++++-------- heat/tests/test_resource.py | 58 ++++++++++++++--------------- 8 files changed, 102 insertions(+), 83 deletions(-) diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py index 273ca4fbb0..8375173597 100644 --- a/heat/db/sqlalchemy/api.py +++ b/heat/db/sqlalchemy/api.py @@ -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() diff --git a/heat/engine/event.py b/heat/engine/event.py index da717587c4..14c54edd3f 100644 --- a/heat/engine/event.py +++ b/heat/engine/event.py @@ -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) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 6f255becad..2fa127f90b 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -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: diff --git a/heat/engine/stack.py b/heat/engine/stack.py index d28617adba..01c68211bd 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -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() diff --git a/heat/objects/resource.py b/heat/objects/resource.py index 102aa45c76..49c21ac7b6 100644 --- a/heat/objects/resource.py +++ b/heat/objects/resource.py @@ -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 diff --git a/heat/tests/test_engine_api_utils.py b/heat/tests/test_engine_api_utils.py index 948d8fd321..f21687da11 100644 --- a/heat/tests/test_engine_api_utils.py +++ b/heat/tests/test_engine_api_utils.py @@ -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]) diff --git a/heat/tests/test_event.py b/heat/tests/test_event.py index 40f598ac3c..2506d70934 100644 --- a/heat/tests/test_event.py +++ b/heat/tests/test_event.py @@ -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() diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index bfd39ccd98..96dc8c8887 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -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)