openstack
/
nova
Code Issues Proposed changes

Merge "Fix Instance object assumptions about joins"

This commit is contained in:
Jenkins 2013-09-24 03:02:12 +00:00 committed by Gerrit Code Review
parent f6197c4c11 19953cee68
commit 91de035005
6 changed files with 112 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/db/sqlalchemy/api.py
View File

@ -1716,6 +1716,9 @@ def _build_instance_get(context, session=None, columns_to_join=None):
if columns_to_join is None:
columns_to_join = ['metadata', 'system_metadata']
for column in columns_to_join:
if column in ['info_cache', 'security_groups']:
# Already always joined above
continue
query = query.options(joinedload(column))
#NOTE(alaski) Stop lazy loading of columns not needed.
for col in ['metadata', 'system_metadata']:

100
nova/objects/instance.py
View File

@ -34,19 +34,27 @@ CONF = cfg.CONF
LOG = logging.getLogger(__name__)
# These are fields that can be specified as expected_attrs but are
# also joined often by default.
INSTANCE_OPTIONAL_COMMON_FIELDS = ['metadata', 'system_metadata']
# These are fields that can be specified as expected_attrs
INSTANCE_OPTIONAL_FIELDS = (INSTANCE_OPTIONAL_COMMON_FIELDS +
['fault', 'pci_devices'])
# These are fields that are always joined by the db right now
INSTANCE_IMPLIED_FIELDS = ['info_cache', 'security_groups']
# List of fields that can be joined in DB layer.
_INSTANCE_OPTIONAL_JOINED_FIELDS = ['metadata', 'system_metadata',
'info_cache', 'security_groups',
'pci_devices']
# These are fields that are optional but don't translate to db columns
INSTANCE_OPTIONAL_NON_COLUMNS = ['fault']
# These are all fields that most query calls load by default
INSTANCE_DEFAULT_FIELDS = (INSTANCE_OPTIONAL_COMMON_FIELDS +
INSTANCE_IMPLIED_FIELDS)
_INSTANCE_OPTIONAL_NON_COLUMN_FIELDS = ['fault']
# These are fields that can be specified as expected_attrs
INSTANCE_OPTIONAL_ATTRS = (_INSTANCE_OPTIONAL_JOINED_FIELDS +
_INSTANCE_OPTIONAL_NON_COLUMN_FIELDS)
# These are fields that most query calls load by default
INSTANCE_DEFAULT_FIELDS = ['metadata', 'system_metadata',
'info_cache', 'security_groups']
def _expected_cols(expected_attrs):
"""Return expected_attrs that are columns needing joining."""
if not expected_attrs:
return expected_attrs
return [attr for attr in expected_attrs
if attr in _INSTANCE_OPTIONAL_JOINED_FIELDS]
class Instance(base.NovaPersistentObject, base.NovaObject):
@ -244,7 +252,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject):
expected_attrs = []
# Most of the field names match right now, so be quick
for field in instance.fields:
if field in INSTANCE_OPTIONAL_FIELDS + INSTANCE_IMPLIED_FIELDS:
if field in INSTANCE_OPTIONAL_ATTRS:
continue
elif field == 'deleted':
instance.deleted = db_inst['deleted'] == db_inst['id']
@ -285,26 +293,11 @@ class Instance(base.NovaPersistentObject, base.NovaObject):
instance.obj_reset_changes()
return instance
@staticmethod
def _attrs_to_columns(attrs):
"""Translate instance attributes into columns needing joining."""
columns_to_join = []
if 'metadata' in attrs:
columns_to_join.append('metadata')
if 'system_metadata' in attrs:
columns_to_join.append('system_metadata')
if 'pci_devices' in attrs:
columns_to_join.append('pci_devices')
# NOTE(danms): The DB API currently always joins info_cache and
# security_groups for get operations, so don't add them to the
# list of columns
return columns_to_join
@base.remotable_classmethod
def get_by_uuid(cls, context, uuid, expected_attrs=None):
if expected_attrs is None:
expected_attrs = ['info_cache', 'security_groups']
columns_to_join = cls._attrs_to_columns(expected_attrs)
columns_to_join = _expected_cols(expected_attrs)
db_inst = db.instance_get_by_uuid(context, uuid,
columns_to_join=columns_to_join)
return cls._from_db_object(context, cls(), db_inst,
@ -314,7 +307,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject):
def get_by_id(cls, context, inst_id, expected_attrs=None):
if expected_attrs is None:
expected_attrs = ['info_cache', 'security_groups']
columns_to_join = cls._attrs_to_columns(expected_attrs)
columns_to_join = _expected_cols(expected_attrs)
db_inst = db.instance_get(context, inst_id,
columns_to_join=columns_to_join)
return cls._from_db_object(context, cls(), db_inst,
@ -447,14 +440,11 @@ class Instance(base.NovaPersistentObject, base.NovaObject):
if expected_vm_state is not None:
updates['expected_vm_state'] = expected_vm_state
expected_attrs = []
for attr in INSTANCE_OPTIONAL_FIELDS + INSTANCE_IMPLIED_FIELDS:
if self.obj_attr_is_set(attr):
expected_attrs.append(attr)
expected_attrs = [attr for attr in _INSTANCE_OPTIONAL_JOINED_FIELDS
if self.obj_attr_is_set(attr)]
old_ref, inst_ref = db.instance_update_and_get_original(
context, self.uuid, updates, update_cells=False,
columns_to_join=self._attrs_to_columns(expected_attrs))
columns_to_join=_expected_cols(expected_attrs))
if stale_instance:
_handle_cell_update_from_api()
@ -468,10 +458,8 @@ class Instance(base.NovaPersistentObject, base.NovaObject):
@base.remotable
def refresh(self, context):
extra = []
for field in INSTANCE_OPTIONAL_FIELDS + INSTANCE_IMPLIED_FIELDS:
if self.obj_attr_is_set(field):
extra.append(field)
extra = [field for field in INSTANCE_OPTIONAL_ATTRS
if self.obj_attr_is_set(field)]
current = self.__class__.get_by_uuid(context, uuid=self.uuid,
expected_attrs=extra)
for field in self.fields:
@ -480,29 +468,15 @@ class Instance(base.NovaPersistentObject, base.NovaObject):
self.obj_reset_changes()
def obj_load_attr(self, attrname):
extra = []
if attrname == 'system_metadata':
extra.append('system_metadata')
elif attrname == 'metadata':
extra.append('metadata')
elif attrname == 'info_cache':
extra.append('info_cache')
elif attrname == 'security_groups':
extra.append('security_groups')
elif attrname == 'pci_devices':
extra.append('pci_devices')
elif attrname == 'fault':
extra.append('fault')
if not extra:
if attrname not in INSTANCE_OPTIONAL_ATTRS:
raise exception.ObjectActionError(
action='obj_load_attr',
reason='attribute %s not lazy-loadable' % attrname)
# NOTE(danms): This could be optimized to just load the bits we need
# FIXME(comstud): This should be optimized to only load the attr.
instance = self.__class__.get_by_uuid(self._context,
uuid=self.uuid,
expected_attrs=extra)
expected_attrs=[attrname])
# NOTE(danms): Never allow us to recursively-load
if instance.obj_attr_is_set(attrname):
@ -537,14 +511,6 @@ def _make_instance_list(context, inst_list, db_inst_list, expected_attrs):
return inst_list
def expected_cols(expected_attrs):
"""Return expected_attrs that are columns needing joining."""
if expected_attrs:
return list(set(expected_attrs) - set(INSTANCE_OPTIONAL_NON_COLUMNS))
else:
return expected_attrs
class InstanceList(base.ObjectListBase, base.NovaObject):
@base.remotable_classmethod
def get_by_filters(cls, context, filters,
@ -552,14 +518,14 @@ class InstanceList(base.ObjectListBase, base.NovaObject):
marker=None, expected_attrs=None):
db_inst_list = db.instance_get_all_by_filters(
context, filters, sort_key, sort_dir, limit=limit, marker=marker,
columns_to_join=expected_cols(expected_attrs))
columns_to_join=_expected_cols(expected_attrs))
return _make_instance_list(context, cls(), db_inst_list,
expected_attrs)
@base.remotable_classmethod
def get_by_host(cls, context, host, expected_attrs=None):
db_inst_list = db.instance_get_all_by_host(
context, host, columns_to_join=expected_cols(expected_attrs))
context, host, columns_to_join=_expected_cols(expected_attrs))
return _make_instance_list(context, cls(), db_inst_list,
expected_attrs)

2
nova/tests/compute/test_compute.py
View File

@ -6524,7 +6524,7 @@ class ComputeAPITestCase(BaseTestCase):
expected = obj_base.obj_to_primitive(
instance_obj.Instance._from_db_object(
self.context, instance_obj.Instance(), exp_instance,
instance_obj.INSTANCE_DEFAULT_FIELDS + ['fields']))
instance_obj.INSTANCE_DEFAULT_FIELDS + ['fault']))
def fake_db_get(_context, _instance_id, columns_to_join=None):
return exp_instance

95
nova/tests/objects/test_instance.py
View File

@ -101,32 +101,45 @@ class _TestInstanceObject(object):
def test_get_without_expected(self):
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
db.instance_get_by_uuid(self.context, 'uuid', columns_to_join=[]
db.instance_get_by_uuid(self.context, 'uuid',
columns_to_join=[]
).AndReturn(self.fake_instance)
self.mox.ReplayAll()
inst = instance.Instance.get_by_uuid(self.context, 'uuid')
# Make sure these weren't loaded
for attr in instance.INSTANCE_OPTIONAL_FIELDS:
inst = instance.Instance.get_by_uuid(self.context, 'uuid',
expected_attrs=[])
for attr in instance.INSTANCE_OPTIONAL_ATTRS:
self.assertFalse(inst.obj_attr_is_set(attr))
self.assertRemotes()
def test_get_with_expected(self):
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
self.mox.StubOutWithMock(db, 'instance_fault_get_by_instance_uuids')
exp_cols = instance.INSTANCE_OPTIONAL_ATTRS[:]
exp_cols.remove('fault')
db.instance_get_by_uuid(
self.context, 'uuid',
columns_to_join=['metadata', 'system_metadata', 'pci_devices']
columns_to_join=exp_cols
).AndReturn(self.fake_instance)
fake_faults = test_instance_fault.fake_faults
db.instance_fault_get_by_instance_uuids(
self.context, [self.fake_instance['uuid']]
).AndReturn(fake_faults)
self.mox.ReplayAll()
inst = instance.Instance.get_by_uuid(
self.context, 'uuid',
expected_attrs=instance.INSTANCE_OPTIONAL_FIELDS)
for attr in instance.INSTANCE_OPTIONAL_FIELDS:
expected_attrs=instance.INSTANCE_OPTIONAL_ATTRS)
for attr in instance.INSTANCE_OPTIONAL_ATTRS:
self.assertTrue(inst.obj_attr_is_set(attr))
self.assertRemotes()
def test_get_by_id(self):
self.mox.StubOutWithMock(db, 'instance_get')
db.instance_get(self.context, 'instid', columns_to_join=[]
db.instance_get(self.context, 'instid',
columns_to_join=['info_cache',
'security_groups']
).AndReturn(self.fake_instance)
self.mox.ReplayAll()
inst = instance.Instance.get_by_id(self.context, 'instid')
@ -136,7 +149,9 @@ class _TestInstanceObject(object):
def test_load(self):
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
fake_uuid = self.fake_instance['uuid']
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(self.fake_instance)
fake_inst2 = dict(self.fake_instance,
system_metadata=[{'key': 'foo', 'value': 'bar'}])
@ -163,7 +178,9 @@ class _TestInstanceObject(object):
# isotime doesn't have microseconds and is always UTC
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
fake_instance = self.fake_instance
db.instance_get_by_uuid(self.context, 'fake-uuid', columns_to_join=[]
db.instance_get_by_uuid(self.context, 'fake-uuid',
columns_to_join=['info_cache',
'security_groups']
).AndReturn(fake_instance)
self.mox.ReplayAll()
inst = instance.Instance.get_by_uuid(self.context, 'fake-uuid')
@ -178,10 +195,14 @@ class _TestInstanceObject(object):
def test_refresh(self):
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
fake_uuid = self.fake_instance['uuid']
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(dict(self.fake_instance,
host='orig-host'))
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(dict(self.fake_instance,
host='new-host'))
self.mox.ReplayAll()
@ -225,12 +246,15 @@ class _TestInstanceObject(object):
self.mox.StubOutWithMock(cells_rpcapi, 'CellsAPI',
use_mock_anything=True)
self.mox.StubOutWithMock(notifications, 'send_update')
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(old_ref)
db.instance_update_and_get_original(
self.context, fake_uuid, expected_updates,
update_cells=False,
columns_to_join=[]).AndReturn((old_ref, new_ref))
columns_to_join=['info_cache', 'security_groups']
).AndReturn((old_ref, new_ref))
if cell_type == 'api':
cells_rpcapi.CellsAPI().AndReturn(cells_api_mock)
cells_api_mock.instance_update_from_api(
@ -293,11 +317,14 @@ class _TestInstanceObject(object):
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
self.mox.StubOutWithMock(db, 'instance_update_and_get_original')
self.mox.StubOutWithMock(notifications, 'send_update')
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(old_ref)
db.instance_update_and_get_original(
self.context, fake_uuid, expected_updates, update_cells=False,
columns_to_join=[]).AndReturn((old_ref, new_ref))
columns_to_join=['info_cache', 'security_groups']
).AndReturn((old_ref, new_ref))
notifications.send_update(self.context, mox.IgnoreArg(),
mox.IgnoreArg())
@ -314,7 +341,9 @@ class _TestInstanceObject(object):
fake_inst = dict(self.fake_instance, id=123, deleted=123)
fake_uuid = fake_inst['uuid']
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(fake_inst)
self.mox.ReplayAll()
inst = instance.Instance.get_by_uuid(self.context, fake_uuid)
@ -325,7 +354,9 @@ class _TestInstanceObject(object):
fake_inst = dict(self.fake_instance, id=123, cleaned=None)
fake_uuid = fake_inst['uuid']
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(fake_inst)
self.mox.ReplayAll()
inst = instance.Instance.get_by_uuid(self.context, fake_uuid)
@ -336,7 +367,9 @@ class _TestInstanceObject(object):
fake_inst = dict(self.fake_instance, id=123, cleaned=1)
fake_uuid = fake_inst['uuid']
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(fake_inst)
self.mox.ReplayAll()
inst = instance.Instance.get_by_uuid(self.context, fake_uuid)
@ -355,7 +388,9 @@ class _TestInstanceObject(object):
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
self.mox.StubOutWithMock(db, 'instance_update_and_get_original')
self.mox.StubOutWithMock(db, 'instance_info_cache_update')
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(fake_inst)
db.instance_info_cache_update(self.context, fake_uuid,
{'network_info': nwinfo2_json})
@ -370,9 +405,8 @@ class _TestInstanceObject(object):
fake_inst = dict(self.fake_instance, info_cache=None)
fake_uuid = fake_inst['uuid']
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
# NOTE(comstud): info_cache is always joined right now
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=[]
columns_to_join=['info_cache']
).AndReturn(fake_inst)
self.mox.ReplayAll()
inst = instance.Instance.get_by_uuid(self.context, fake_uuid,
@ -395,7 +429,9 @@ class _TestInstanceObject(object):
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
self.mox.StubOutWithMock(db, 'instance_update_and_get_original')
self.mox.StubOutWithMock(db, 'security_group_update')
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(fake_inst)
db.security_group_update(self.context, 1, {'description': 'changed'}
).AndReturn(fake_inst['security_groups'][0])
@ -417,7 +453,9 @@ class _TestInstanceObject(object):
fake_inst = dict(self.fake_instance, security_groups=[])
fake_uuid = fake_inst['uuid']
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=['info_cache',
'security_groups']
).AndReturn(fake_inst)
self.mox.ReplayAll()
inst = instance.Instance.get_by_uuid(self.context, fake_uuid)
@ -489,7 +527,8 @@ class _TestInstanceObject(object):
for x in test_instance_fault.fake_faults['fake-uuid']]
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
self.mox.StubOutWithMock(db, 'instance_fault_get_by_instance_uuids')
db.instance_get_by_uuid(self.context, fake_uuid, columns_to_join=[]
db.instance_get_by_uuid(self.context, fake_uuid,
columns_to_join=[],
).AndReturn(self.fake_instance)
db.instance_fault_get_by_instance_uuids(
self.context, [fake_uuid]).AndReturn({fake_uuid: fake_faults})
@ -840,6 +879,6 @@ class TestRemoteInstanceListObject(test_objects._RemoteTest,
class TestInstanceObjectMisc(test.TestCase):
def test_expected_cols(self):
self.stubs.Set(instance, 'INSTANCE_OPTIONAL_NON_COLUMNS', ['bar'])
self.assertEqual(['foo'], instance.expected_cols(['foo', 'bar']))
self.assertEqual(None, instance.expected_cols(None))
self.stubs.Set(instance, '_INSTANCE_OPTIONAL_JOINED_FIELDS', ['bar'])
self.assertEqual(['bar'], instance._expected_cols(['foo', 'bar']))
self.assertEqual(None, instance._expected_cols(None))

3
nova/tests/objects/test_migration.py
View File

@ -100,7 +100,8 @@ class _TestMigrationObject(object):
fake_inst = fake_instance.fake_db_instance()
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
db.instance_get_by_uuid(ctxt, fake_migration['instance_uuid'],
columns_to_join=[]
columns_to_join=['info_cache',
'security_groups']
).AndReturn(fake_inst)
mig = migration.Migration._from_db_object(ctxt,
migration.Migration(),

8
nova/tests/virt/libvirt/test_libvirt.py
View File

@ -3698,7 +3698,9 @@ class LibvirtConnTestCase(test.TestCase):
libvirt_driver.LibvirtDriver._undefine_domain(instance)
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
db.instance_get_by_uuid(mox.IgnoreArg(), mox.IgnoreArg(),
columns_to_join=[]).AndReturn(instance)
columns_to_join=['info_cache',
'security_groups']
).AndReturn(instance)
self.mox.StubOutWithMock(shutil, "rmtree")
shutil.rmtree(os.path.join(CONF.instances_path,
'instance-%08x' % int(instance['id'])))
@ -3784,7 +3786,9 @@ class LibvirtConnTestCase(test.TestCase):
self.mox.StubOutWithMock(shutil, "rmtree")
db.instance_get_by_uuid(mox.IgnoreArg(), mox.IgnoreArg(),
columns_to_join=[]).AndReturn(instance)
columns_to_join=['info_cache',
'security_groups']
).AndReturn(instance)
os.path.exists(mox.IgnoreArg()).AndReturn(False)
os.path.exists(mox.IgnoreArg()).AndReturn(True)
shutil.rmtree(os.path.join(CONF.instances_path, instance['uuid']))