openstack
/
heat
Code Issues Proposed changes

Don't always eagerly load the raw_template for a stack 

Always loading the raw template in situations where we didn't need it -
e.g. in identify_stack, where we just want the name + id (given one of
them), or when getting the summary stack list - uses up DB bandwidth and
memory unnecessarily.

This partially reverts commit 3ab0ede98c.

* The eager_load option to get_stack() is reinstated, but with the default
  flipped to True. In places where we explicitly do not want to load the
  template, we pass False.
* stack_get_by_name() no longer eagerly loads the template. There were no
  instances of this where we subsequently use the template.
* stack_get_all() acquires an eager_load option, with the default set to
  False. Direct users of objects.stack.Stack.get_all() will not eagerly
  load by default, but users of engine.stack.Stack.load_all() will get the
  template eagerly loaded. This practically always corresponds to what you
  want.

Change-Id: I1f156c25ea26322be5b606a61dd77d80cadd65fc
Related-Bug: #1626675
This commit is contained in:
Zane Bitter 2016-10-04 08:25:01 -04:00
parent 80b2fb3155
commit 260b79ed28
7 changed files with 38 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
heat/db/api.py
View File

@ -160,8 +160,9 @@ def resource_get_by_physical_resource_id(context, physical_resource_id):
physical_resource_id)
def stack_get(context, stack_id, show_deleted=False):
return IMPL.stack_get(context, stack_id, show_deleted=show_deleted)
def stack_get(context, stack_id, show_deleted=False, eager_load=True):
return IMPL.stack_get(context, stack_id, show_deleted=show_deleted,
eager_load=eager_load)
def stack_get_status(context, stack_id):
@ -181,11 +182,12 @@ def stack_get_all(context, limit=None, sort_keys=None, marker=None,
sort_dir=None, filters=None,
show_deleted=False, show_nested=False, show_hidden=False,
tags=None, tags_any=None, not_tags=None,
not_tags_any=None):
not_tags_any=None, eager_load=False):
return IMPL.stack_get_all(context, limit, sort_keys,
marker, sort_dir, filters,
show_deleted, show_nested, show_hidden,
tags, tags_any, not_tags, not_tags_any)
tags, tags_any, not_tags, not_tags_any,
eager_load=eager_load)
def stack_get_all_by_owner_id(context, owner_id):

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

@ -430,16 +430,17 @@ def stack_get_by_name_and_owner_id(context, stack_name, owner_id):
def stack_get_by_name(context, stack_name):
query = soft_delete_aware_query(
context, models.Stack
).options(orm.joinedload("raw_template")).filter(sqlalchemy.or_(
models.Stack.tenant == context.tenant_id,
models.Stack.stack_user_project_id == context.tenant_id)
).filter_by(name=stack_name)
).filter(sqlalchemy.or_(
models.Stack.tenant == context.tenant_id,
models.Stack.stack_user_project_id == context.tenant_id)
).filter_by(name=stack_name)
return query.first()
def stack_get(context, stack_id, show_deleted=False):
query = context.session.query(models.Stack).options(
orm.joinedload("raw_template"))
def stack_get(context, stack_id, show_deleted=False, eager_load=True):
query = context.session.query(models.Stack)
if eager_load:
query = query.options(orm.joinedload("raw_template"))
result = query.get(stack_id)
deleted_ok = show_deleted or context.show_deleted
@ -572,14 +573,15 @@ def stack_get_all(context, limit=None, sort_keys=None, marker=None,
sort_dir=None, filters=None,
show_deleted=False, show_nested=False, show_hidden=False,
tags=None, tags_any=None, not_tags=None,
not_tags_any=None):
not_tags_any=None, eager_load=False):
query = _query_stack_get_all(context,
show_deleted=show_deleted,
show_nested=show_nested,
show_hidden=show_hidden, tags=tags,
tags_any=tags_any, not_tags=not_tags,
not_tags_any=not_tags_any)
query = query.options(orm.joinedload("raw_template"))
if eager_load:
query = query.options(orm.joinedload("raw_template"))
return _filter_and_page_query(context, query, limit, sort_keys,
marker, sort_dir, filters).all()

3
heat/engine/service.py
View File

@ -469,7 +469,8 @@ class EngineService(service.Service):
s = stack_object.Stack.get_by_id(
cnxt,
stack_name,
show_deleted=True)
show_deleted=True,
eager_load=False)
# may be the name is in uuid format, so if get by id returns None,
# we should get the info by name again
if not s:

9
heat/engine/stack.py
View File

@ -526,7 +526,8 @@ class Stack(collections.Mapping):
tags=tags,
tags_any=tags_any,
not_tags=not_tags,
not_tags_any=not_tags_any)
not_tags_any=not_tags_any,
eager_load=True)
for stack in stacks:
try:
yield cls._from_db(context, stack)
@ -917,7 +918,8 @@ class Stack(collections.Mapping):
"""Persist stack state to database"""
if self.id is None:
return
stack = stack_object.Stack.get_by_id(self.context, self.id)
stack = stack_object.Stack.get_by_id(self.context, self.id,
eager_load=False)
if stack is not None:
values = {'action': self.action,
'status': self.status,
@ -948,7 +950,8 @@ class Stack(collections.Mapping):
"""Persist stack state to database and release stack lock"""
if self.id is None:
return
stack = stack_object.Stack.get_by_id(self.context, self.id)
stack = stack_object.Stack.get_by_id(self.context, self.id,
eager_load=False)
if stack is not None:
values = {'action': self.action,
'status': self.status,

3
heat/engine/stack_lock.py
View File

@ -67,7 +67,8 @@ class StackLock(object):
return
stack = stack_object.Stack.get_by_id(self.context, self.stack_id,
show_deleted=True)
show_deleted=True,
eager_load=False)
if (lock_engine_id == self.engine_id or
service_utils.engine_alive(self.context, lock_engine_id)):
LOG.debug("Lock on stack %(stack)s is owned by engine "

5
heat/objects/stack.py
View File

@ -114,7 +114,7 @@ class Stack(
sort_dir=None, filters=None,
show_deleted=False, show_nested=False, show_hidden=False,
tags=None, tags_any=None, not_tags=None,
not_tags_any=None):
not_tags_any=None, eager_load=False):
db_stacks = db_api.stack_get_all(
context,
limit=limit,
@ -128,7 +128,8 @@ class Stack(
tags=tags,
tags_any=tags_any,
not_tags=not_tags,
not_tags_any=not_tags_any)
not_tags_any=not_tags_any,
eager_load=eager_load)
for db_stack in db_stacks:
try:
yield cls._from_db_object(context, cls(context), db_stack)

12
heat/tests/test_stack_lock.py
View File

@ -62,7 +62,8 @@ class StackLockTest(common.HeatTestCase):
self.mock_get_by_id.assert_called_once_with(
self.context,
self.stack_id,
show_deleted=True)
show_deleted=True,
eager_load=False)
mock_create.assert_called_once_with(
self.context, self.stack_id, self.engine_id)
@ -96,7 +97,8 @@ class StackLockTest(common.HeatTestCase):
self.mock_get_by_id.assert_called_once_with(
self.context,
self.stack_id,
show_deleted=True)
show_deleted=True,
eager_load=False)
mock_create.assert_called_once_with(
self.context, self.stack_id, self.engine_id)
@ -116,7 +118,8 @@ class StackLockTest(common.HeatTestCase):
self.mock_get_by_id.assert_called_once_with(
self.context,
self.stack_id,
show_deleted=True)
show_deleted=True,
eager_load=False)
mock_create.assert_called_once_with(
self.context, self.stack_id, self.engine_id)
@ -157,7 +160,8 @@ class StackLockTest(common.HeatTestCase):
self.mock_get_by_id.assert_called_with(
self.context,
self.stack_id,
show_deleted=True)
show_deleted=True,
eager_load=False)
mock_create.assert_has_calls(
[mock.call(self.context, self.stack_id, self.engine_id)] * 2)