From: Liang Chen Date: Sun, 7 Jul 2013 06:43:45 +0000 (+0800) Subject: avoid excessive database calls while loading events X-Git-Tag: 2014.1~371^2 X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=3fa39dd2ccc9013cfba4ec8bd5fb79caeba0350e;p=openstack-build%2Fheat-build.git avoid excessive database calls while loading events list_events constructs Event objects through Event.load, which causes every event record previously loaded into memory to be reloaded again. Furthermore, Event.load will also reload the associated stack, thus all its resources and template. Another bad thing with that is reloading a stack will lead to many extra stack static resolution (including calls to nova to resovle Fn::GetAZs if it's referenced in the template). Fixes bug #1195793 Change-Id: I4f14c9a2a4f990b753a6e1405e27eb44c53537c9 --- diff --git a/heat/engine/event.py b/heat/engine/event.py index 50e85ff2..8fe17614 100644 --- a/heat/engine/event.py +++ b/heat/engine/event.py @@ -48,25 +48,25 @@ class Event(object): self.id = id @classmethod - def load(cls, context, event_id): + def load(cls, context, event_id, event=None, stack=None): '''Retrieve an Event from the database.''' from heat.engine import parser - ev = db_api.event_get(context, event_id) + ev = event if event is not None else\ + db_api.event_get(context, event_id) if ev is None: message = 'No event exists with id "%s"' % str(event_id) raise exception.NotFound(message) - stack = parser.Stack.load(context, ev.stack_id) - resource = stack[ev.logical_resource_id] + st = stack if stack is not None else\ + parser.Stack.load(context, ev.stack_id) + resource = st[ev.logical_resource_id] - event = cls(context, stack, resource, - ev.resource_action, ev.resource_status, - ev.resource_status_reason, - ev.physical_resource_id, ev.resource_properties, - ev.created_at, ev.id) - - return event + return cls(context, st, resource, + ev.resource_action, ev.resource_status, + ev.resource_status_reason, + ev.physical_resource_id, ev.resource_properties, + ev.created_at, ev.id) def store(self): '''Store the Event in the database.''' diff --git a/heat/engine/service.py b/heat/engine/service.py index 5abb939f..4a859610 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -366,6 +366,7 @@ class EngineService(service.Service): arg1 -> RPC context. arg2 -> Name of the stack you want to get events for. """ + if stack_identity is not None: st = self._get_stack(cnxt, stack_identity) @@ -373,7 +374,17 @@ class EngineService(service.Service): else: events = db_api.event_get_all_by_tenant(cnxt) - return [api.format_event(Event.load(cnxt, e.id)) for e in events] + stacks = {} + + def get_stack(stack_id): + if stack_id not in stacks: + stacks[stack_id] = parser.Stack.load(cnxt, stack_id) + return stacks[stack_id] + + return [api.format_event(Event.load(cnxt, + e.id, e, + get_stack(e.stack_id))) + for e in events] def _authorize_stack_user(self, cnxt, stack, resource_name): ''' diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index 8a71eaee..011b82d4 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -18,6 +18,8 @@ import json import sys import mox +from testtools import matchers + from oslo.config import cfg from heat.common import config @@ -221,9 +223,9 @@ class stackCreateTest(HeatTestCase): rsrc = stack.resources['WebServer'] self.assertEqual((rsrc.DELETE, rsrc.COMPLETE), rsrc.state) self.assertEqual((stack.DELETE, stack.COMPLETE), rsrc.state) - self.assertEqual(db_api.stack_get(ctx, stack_id), None) - self.assertEqual(db_s.action, 'DELETE') - self.assertEqual(db_s.status, 'COMPLETE') + self.assertEqual(None, db_api.stack_get(ctx, stack_id)) + self.assertEqual('DELETE', db_s.action) + self.assertEqual('COMPLETE', db_s.status, ) class stackServiceCreateUpdateDeleteTest(HeatTestCase): @@ -263,7 +265,7 @@ class stackServiceCreateUpdateDeleteTest(HeatTestCase): result = self.man.create_stack(self.ctx, stack_name, template, params, None, {}) - self.assertEqual(result, stack.identifier()) + self.assertEqual(stack.identifier(), result) self.assertTrue(isinstance(result, dict)) self.assertTrue(result['stack_id']) self.m.VerifyAll() @@ -360,8 +362,8 @@ class stackServiceCreateUpdateDeleteTest(HeatTestCase): self.m.ReplayAll() - self.assertEqual(self.man.delete_stack(self.ctx, stack.identifier()), - None) + self.assertEqual(None, + self.man.delete_stack(self.ctx, stack.identifier())) self.m.VerifyAll() def test_stack_delete_nonexist(self): @@ -408,7 +410,7 @@ class stackServiceCreateUpdateDeleteTest(HeatTestCase): result = self.man.update_stack(self.ctx, old_stack.identifier(), template, params, None, {}) - self.assertEqual(result, old_stack.identifier()) + self.assertEqual(old_stack.identifier(), result) self.assertTrue(isinstance(result, dict)) self.assertTrue(result['stack_id']) self.m.VerifyAll() @@ -492,7 +494,7 @@ class stackServiceSuspendResumeTest(HeatTestCase): self.m.ReplayAll() result = self.man.stack_suspend(self.ctx, stack.identifier()) - self.assertEqual(result, None) + self.assertEqual(None, result) self.m.VerifyAll() @@ -509,7 +511,7 @@ class stackServiceSuspendResumeTest(HeatTestCase): self.m.ReplayAll() result = self.man.stack_resume(self.ctx, self.stack.identifier()) - self.assertEqual(result, None) + self.assertEqual(None, result) self.m.VerifyAll() def test_stack_suspend_nonexist(self): @@ -556,7 +558,7 @@ class stackServiceTest(HeatTestCase): self.m.ReplayAll() identity = self.eng.identify_stack(self.ctx, self.stack.name) - self.assertEqual(identity, self.stack.identifier()) + self.assertEqual(self.stack.identifier(), identity) self.m.VerifyAll() @@ -568,7 +570,7 @@ class stackServiceTest(HeatTestCase): self.m.ReplayAll() identity = self.eng.identify_stack(self.ctx, self.stack.id) - self.assertEqual(identity, self.stack.identifier()) + self.assertEqual(self.stack.identifier(), identity) self.m.VerifyAll() @@ -601,14 +603,14 @@ class stackServiceTest(HeatTestCase): events = self.eng.list_events(self.ctx, self.stack.identifier()) - self.assertEqual(len(events), 2) + self.assertEqual(2, len(events)) for ev in events: self.assertTrue('event_identity' in ev) - self.assertEqual(type(ev['event_identity']), dict) + self.assertEqual(dict, type(ev['event_identity'])) self.assertTrue(ev['event_identity']['path'].rsplit('/', 1)[1]) self.assertTrue('logical_resource_id' in ev) - self.assertEqual(ev['logical_resource_id'], 'WebServer') + self.assertEqual('WebServer', ev['logical_resource_id']) self.assertTrue('physical_resource_id' in ev) @@ -617,30 +619,73 @@ class stackServiceTest(HeatTestCase): # a few times so this should work. user_data = ev['resource_properties']['UserData'] self.assertNotEqual(user_data.find('wordpress'), -1) - self.assertEqual(ev['resource_properties']['ImageId'], - 'F17-x86_64-gold') - self.assertEqual(ev['resource_properties']['InstanceType'], - 'm1.large') + self.assertEqual('F17-x86_64-gold', + ev['resource_properties']['ImageId']) + self.assertEqual('m1.large', + ev['resource_properties']['InstanceType']) - self.assertEqual(ev['resource_action'], 'CREATE') + self.assertEqual('CREATE', ev['resource_action']) self.assertTrue(ev['resource_status'] in ('IN_PROGRESS', 'COMPLETE')) self.assertTrue('resource_status_reason' in ev) - self.assertEqual(ev['resource_status_reason'], 'state changed') + self.assertEqual('state changed', ev['resource_status_reason']) self.assertTrue('resource_type' in ev) - self.assertEqual(ev['resource_type'], 'AWS::EC2::Instance') + self.assertEqual('AWS::EC2::Instance', ev['resource_type']) self.assertTrue('stack_identity' in ev) self.assertTrue('stack_name' in ev) - self.assertEqual(ev['stack_name'], self.stack.name) + self.assertEqual(self.stack.name, ev['stack_name']) self.assertTrue('event_time' in ev) self.m.VerifyAll() + @stack_context('service_event_list_test_stack') + def test_stack_event_list_by_tenant(self): + events = self.eng.list_events(self.ctx, None) + + self.assertEqual(2, len(events)) + for ev in events: + self.assertIn('event_identity', ev) + self.assertThat(ev['event_identity'], matchers.IsInstance(dict)) + self.assertTrue(ev['event_identity']['path'].rsplit('/', 1)[1]) + + self.assertTrue('logical_resource_id' in ev) + self.assertEqual('WebServer', ev['logical_resource_id']) + + self.assertTrue('physical_resource_id' in ev) + + self.assertTrue('resource_properties' in ev) + # Big long user data field.. it mentions 'wordpress' + # a few times so this should work. + user_data = ev['resource_properties']['UserData'] + self.assertIn('wordpress', user_data) + self.assertEqual('F17-x86_64-gold', + ev['resource_properties']['ImageId']) + self.assertEqual('m1.large', + ev['resource_properties']['InstanceType']) + + self.assertEqual('CREATE', ev['resource_action']) + self.assertIn(ev['resource_status'], ('IN_PROGRESS', 'COMPLETE')) + + self.assertIn('resource_status_reason', ev) + self.assertEqual('state changed', ev['resource_status_reason']) + + self.assertIn('resource_type', ev) + self.assertEqual('AWS::EC2::Instance', ev['resource_type']) + + self.assertIn('stack_identity', ev) + + self.assertIn('stack_name', ev) + self.assertEqual(self.stack.name, ev['stack_name']) + + self.assertIn('event_time', ev) + + self.m.VerifyAll() + @stack_context('service_list_all_test_stack') def test_stack_list_all(self): self.m.StubOutWithMock(parser.Stack, 'load') @@ -650,14 +695,14 @@ class stackServiceTest(HeatTestCase): self.m.ReplayAll() sl = self.eng.list_stacks(self.ctx) - self.assertEqual(len(sl), 1) + self.assertEqual(1, len(sl)) for s in sl: self.assertTrue('creation_time' in s) self.assertTrue('updated_time' in s) self.assertTrue('stack_identity' in s) self.assertNotEqual(s['stack_identity'], None) self.assertTrue('stack_name' in s) - self.assertEqual(s['stack_name'], self.stack.name) + self.assertEqual(self.stack.name, s['stack_name']) self.assertTrue('stack_status' in s) self.assertTrue('stack_status_reason' in s) self.assertTrue('description' in s) @@ -706,7 +751,7 @@ class stackServiceTest(HeatTestCase): sl = self.eng.show_stack(self.ctx, self.stack.identifier()) - self.assertEqual(len(sl), 1) + self.assertEqual(1, len(sl)) s = sl[0] self.assertTrue('creation_time' in s) @@ -714,7 +759,7 @@ class stackServiceTest(HeatTestCase): self.assertTrue('stack_identity' in s) self.assertNotEqual(s['stack_identity'], None) self.assertTrue('stack_name' in s) - self.assertEqual(s['stack_name'], self.stack.name) + self.assertEqual(self.stack.name, s['stack_name']) self.assertTrue('stack_status' in s) self.assertTrue('stack_status_reason' in s) self.assertTrue('description' in s) @@ -727,7 +772,7 @@ class stackServiceTest(HeatTestCase): def test_stack_describe_all(self): sl = self.eng.show_stack(self.ctx, None) - self.assertEqual(len(sl), 1) + self.assertEqual(1, len(sl)) s = sl[0] self.assertTrue('creation_time' in s) @@ -735,7 +780,7 @@ class stackServiceTest(HeatTestCase): self.assertTrue('stack_identity' in s) self.assertNotEqual(s['stack_identity'], None) self.assertTrue('stack_name' in s) - self.assertEqual(s['stack_name'], self.stack.name) + self.assertEqual(self.stack.name, s['stack_name']) self.assertTrue('stack_status' in s) self.assertTrue('stack_status_reason' in s) self.assertTrue('description' in s) @@ -764,14 +809,14 @@ class stackServiceTest(HeatTestCase): self.assertTrue('stack_identity' in r) self.assertNotEqual(r['stack_identity'], None) self.assertTrue('stack_name' in r) - self.assertEqual(r['stack_name'], self.stack.name) + self.assertEqual(self.stack.name, r['stack_name']) self.assertTrue('metadata' in r) self.assertTrue('resource_status' in r) self.assertTrue('resource_status_reason' in r) self.assertTrue('resource_type' in r) self.assertTrue('physical_resource_id' in r) self.assertTrue('logical_resource_id' in r) - self.assertEqual(r['logical_resource_id'], 'WebServer') + self.assertEqual('WebServer', r['logical_resource_id']) self.m.VerifyAll() @@ -858,7 +903,7 @@ class stackServiceTest(HeatTestCase): self.stack.identifier(), 'WebServer') - self.assertEqual(len(resources), 1) + self.assertEqual(1, len(resources)) r = resources[0] self.assertTrue('resource_identity' in r) self.assertTrue('description' in r) @@ -866,13 +911,13 @@ class stackServiceTest(HeatTestCase): self.assertTrue('stack_identity' in r) self.assertNotEqual(r['stack_identity'], None) self.assertTrue('stack_name' in r) - self.assertEqual(r['stack_name'], self.stack.name) + self.assertEqual(self.stack.name, r['stack_name']) self.assertTrue('resource_status' in r) self.assertTrue('resource_status_reason' in r) self.assertTrue('resource_type' in r) self.assertTrue('physical_resource_id' in r) self.assertTrue('logical_resource_id' in r) - self.assertEqual(r['logical_resource_id'], 'WebServer') + self.assertEqual('WebServer', r['logical_resource_id']) self.m.VerifyAll() @@ -887,10 +932,10 @@ class stackServiceTest(HeatTestCase): self.stack.identifier(), None) - self.assertEqual(len(resources), 1) + self.assertEqual(1, len(resources)) r = resources[0] self.assertTrue('logical_resource_id' in r) - self.assertEqual(r['logical_resource_id'], 'WebServer') + self.assertEqual('WebServer', r['logical_resource_id']) self.m.VerifyAll() @@ -924,8 +969,8 @@ class stackServiceTest(HeatTestCase): result = self.eng.find_physical_resource(self.ctx, phys_id) self.assertTrue(isinstance(result, dict)) resource_identity = identifier.ResourceIdentifier(**result) - self.assertEqual(resource_identity.stack(), self.stack.identifier()) - self.assertEqual(resource_identity.resource_name, 'WebServer') + self.assertEqual(self.stack.identifier(), resource_identity.stack()) + self.assertEqual('WebServer', resource_identity.resource_name) def test_find_physical_resource_nonexist(self): self.assertRaises(exception.PhysicalResourceNotFound, @@ -942,13 +987,13 @@ class stackServiceTest(HeatTestCase): resources = self.eng.list_stack_resources(self.ctx, self.stack.identifier()) - self.assertEqual(len(resources), 1) + self.assertEqual(1, len(resources)) r = resources[0] self.assertTrue('resource_identity' in r) self.assertTrue('updated_time' in r) self.assertTrue('physical_resource_id' in r) self.assertTrue('logical_resource_id' in r) - self.assertEqual(r['logical_resource_id'], 'WebServer') + self.assertEqual('WebServer', r['logical_resource_id']) self.assertTrue('resource_status' in r) self.assertTrue('resource_status_reason' in r) self.assertTrue('resource_type' in r) @@ -989,7 +1034,7 @@ class stackServiceTest(HeatTestCase): 'WebServer', test_metadata) # metadata_update is a no-op for all resources except # WaitConditionHandle so we don't expect this to have changed - self.assertEqual(result, pre_update_meta) + self.assertEqual(pre_update_meta, result) self.m.VerifyAll() @@ -1157,23 +1202,23 @@ class stackServiceTest(HeatTestCase): result = self.eng.set_watch_state(self.ctx, watch_name="OverrideAlarm", state=state) - self.assertEqual(result[engine_api.WATCH_STATE_VALUE], state) - self.assertEqual(self.eng.stg[self.stack.id].threads, []) + self.assertEqual(state, result[engine_api.WATCH_STATE_VALUE]) + self.assertEqual([], self.eng.stg[self.stack.id].threads) state = watchrule.WatchRule.NORMAL result = self.eng.set_watch_state(self.ctx, watch_name="OverrideAlarm", state=state) - self.assertEqual(result[engine_api.WATCH_STATE_VALUE], state) - self.assertEqual(self.eng.stg[self.stack.id].threads, []) + self.assertEqual(state, result[engine_api.WATCH_STATE_VALUE]) + self.assertEqual([], self.eng.stg[self.stack.id].threads) state = watchrule.WatchRule.ALARM result = self.eng.set_watch_state(self.ctx, watch_name="OverrideAlarm", state=state) - self.assertEqual(result[engine_api.WATCH_STATE_VALUE], state) - self.assertEqual(self.eng.stg[self.stack.id].threads, - [DummyAction.alarm]) + self.assertEqual(state, result[engine_api.WATCH_STATE_VALUE]) + self.assertEqual([DummyAction.alarm], + self.eng.stg[self.stack.id].threads) self.m.VerifyAll() @@ -1228,9 +1273,9 @@ class stackServiceTest(HeatTestCase): def test_stack_list_all_empty(self): sl = self.eng.list_stacks(self.ctx) - self.assertEqual(len(sl), 0) + self.assertEqual(0, len(sl)) def test_stack_describe_all_empty(self): sl = self.eng.show_stack(self.ctx, None) - self.assertEqual(len(sl), 0) + self.assertEqual(0, len(sl)) diff --git a/heat/tests/test_event.py b/heat/tests/test_event.py index 160eba42..0cadb861 100644 --- a/heat/tests/test_event.py +++ b/heat/tests/test_event.py @@ -71,15 +71,39 @@ class EventTest(HeatTestCase): loaded_e = event.Event.load(self.ctx, e.id) - self.assertEqual(loaded_e.stack.id, self.stack.id) - self.assertEqual(loaded_e.resource.name, self.resource.name) - self.assertEqual(loaded_e.resource.id, self.resource.id) - self.assertEqual(loaded_e.physical_resource_id, 'wibble') - self.assertEqual(loaded_e.action, 'TEST') - self.assertEqual(loaded_e.status, 'IN_PROGRESS') - self.assertEqual(loaded_e.reason, 'Testing') - self.assertNotEqual(loaded_e.timestamp, None) - self.assertEqual(loaded_e.resource_properties, {'Foo': 'goo'}) + self.assertEqual(self.stack.id, loaded_e.stack.id) + self.assertEqual(self.resource.name, loaded_e.resource.name) + self.assertEqual(self.resource.id, loaded_e.resource.id) + self.assertEqual('wibble', loaded_e.physical_resource_id) + self.assertEqual('TEST', loaded_e.action) + self.assertEqual('IN_PROGRESS', loaded_e.status) + self.assertEqual('Testing', loaded_e.reason) + self.assertNotEqual(None, loaded_e.timestamp) + self.assertEqual({'Foo': 'goo'}, loaded_e.resource_properties) + + def test_load_given_stack_event(self): + self.resource.resource_id_set('resource_physical_id') + + e = event.Event(self.ctx, self.stack, self.resource, + 'TEST', 'IN_PROGRESS', 'Testing', + 'wibble', self.resource.properties) + + e.store() + self.assertNotEqual(e.id, None) + + ev = db_api.event_get(self.ctx, e.id) + + loaded_e = event.Event.load(self.ctx, e.id, stack=self.stack, event=ev) + + self.assertEqual(self.stack.id, loaded_e.stack.id) + self.assertEqual(self.resource.name, loaded_e.resource.name) + self.assertEqual(self.resource.id, loaded_e.resource.id) + self.assertEqual('wibble', loaded_e.physical_resource_id) + self.assertEqual('TEST', loaded_e.action) + self.assertEqual('IN_PROGRESS', loaded_e.status) + self.assertEqual('Testing', loaded_e.reason) + self.assertNotEqual(None, loaded_e.timestamp) + self.assertEqual({'Foo': 'goo'}, loaded_e.resource_properties) def test_identifier(self): e = event.Event(self.ctx, self.stack, self.resource, @@ -93,7 +117,7 @@ class EventTest(HeatTestCase): 'tenant': self.ctx.tenant_id, 'path': '/resources/EventTestResource/events/%s' % str(eid) } - self.assertEqual(e.identifier(), expected_identifier) + self.assertEqual(expected_identifier, e.identifier()) def test_badprop(self): tmpl = {'Type': 'ResourceWithRequiredProps',