Add volume_type to volume object expected_attrs
We haven't had volume_type in expected_attrs for volume objects lists. This resulted in situation in which although we were joining the volume_type explicitely in DB API, the object just dropped the data. Volume type was then lazy loaded when needed, so every "cinder list" call was making additional DB queries per returned volume, causing massive performance drop. Actually there were two unnecessary DB calls per volume, because not only volume_type was fetched, but also volume_type.extra_specs as a result of passing 'extra_specs' in expected_attrs when calling VolumeType._from_db_volume in Volume._from_db_volume (wow, that's complicated…). This commit sorts this out by adding volume_type to expected_attrs to match what we join in the DB. Please note that I'm not adding consistencygroup and volume_attachment on purpose - addition causes some unit tests failure and that late in the release it seems risky to try fixing that. The changes also required minor rework of expected_attrs infrastructure in the o.vo layer to be able to pass different values when we query for just a single volume and when we fetch whole list (as we're doing different joins in the DB layer in both cases). Change-Id: Iabf9c3fab56ffef50695ce45745f193273822b39 Closes-Bug: 1555153
This commit is contained in:
parent
08b74ce328
commit
4e3f61b331
|
@ -149,6 +149,10 @@ class CinderObject(base.VersionedObject):
|
|||
# Return modified dict
|
||||
return changes
|
||||
|
||||
@classmethod
|
||||
def _get_expected_attrs(cls, context):
|
||||
return None
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_by_id(cls, context, id, *args, **kwargs):
|
||||
# To get by id we need to have a model and for the model to
|
||||
|
@ -160,9 +164,10 @@ class CinderObject(base.VersionedObject):
|
|||
|
||||
model = db.get_model_for_versioned_object(cls)
|
||||
orm_obj = db.get_by_id(context, model, id, *args, **kwargs)
|
||||
expected_attrs = cls._get_expected_attrs(context)
|
||||
kargs = {}
|
||||
if hasattr(cls, 'DEFAULT_EXPECTED_ATTR'):
|
||||
kargs = {'expected_attrs': getattr(cls, 'DEFAULT_EXPECTED_ATTR')}
|
||||
if expected_attrs:
|
||||
kargs = {'expected_attrs': expected_attrs}
|
||||
return cls._from_db_object(context, cls(context), orm_obj, **kargs)
|
||||
|
||||
def conditional_update(self, values, expected_values=None, filters=(),
|
||||
|
|
|
@ -37,8 +37,6 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject,
|
|||
# are typically the relationship in the sqlalchemy object.
|
||||
OPTIONAL_FIELDS = ('volume', 'metadata', 'cgsnapshot')
|
||||
|
||||
DEFAULT_EXPECTED_ATTR = ('metadata',)
|
||||
|
||||
fields = {
|
||||
'id': fields.UUIDField(),
|
||||
|
||||
|
@ -66,6 +64,10 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject,
|
|||
'cgsnapshot': fields.ObjectField('CGSnapshot', nullable=True),
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def _get_expected_attrs(cls, context):
|
||||
return 'metadata',
|
||||
|
||||
# NOTE(thangp): obj_extra_fields is used to hold properties that are not
|
||||
# usually part of the model
|
||||
obj_extra_fields = ['name', 'volume_name']
|
||||
|
@ -232,14 +234,16 @@ class SnapshotList(base.ObjectListBase, base.CinderObject):
|
|||
sort_keys=None, sort_dirs=None, offset=None):
|
||||
snapshots = db.snapshot_get_all(context, search_opts, marker, limit,
|
||||
sort_keys, sort_dirs, offset)
|
||||
return base.obj_make_list(context, cls(), objects.Snapshot,
|
||||
snapshots, expected_attrs=['metadata'])
|
||||
expected_attrs = Snapshot._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Snapshot,
|
||||
snapshots, expected_attrs=expected_attrs)
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_by_host(cls, context, host, filters=None):
|
||||
snapshots = db.snapshot_get_by_host(context, host, filters)
|
||||
expected_attrs = Snapshot._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Snapshot,
|
||||
snapshots, expected_attrs=['metadata'])
|
||||
snapshots, expected_attrs=expected_attrs)
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_all_by_project(cls, context, project_id, search_opts, marker=None,
|
||||
|
@ -248,23 +252,27 @@ class SnapshotList(base.ObjectListBase, base.CinderObject):
|
|||
snapshots = db.snapshot_get_all_by_project(
|
||||
context, project_id, search_opts, marker, limit, sort_keys,
|
||||
sort_dirs, offset)
|
||||
expected_attrs = Snapshot._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Snapshot,
|
||||
snapshots, expected_attrs=['metadata'])
|
||||
snapshots, expected_attrs=expected_attrs)
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_all_for_volume(cls, context, volume_id):
|
||||
snapshots = db.snapshot_get_all_for_volume(context, volume_id)
|
||||
expected_attrs = Snapshot._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Snapshot,
|
||||
snapshots, expected_attrs=['metadata'])
|
||||
snapshots, expected_attrs=expected_attrs)
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_active_by_window(cls, context, begin, end):
|
||||
snapshots = db.snapshot_get_active_by_window(context, begin, end)
|
||||
expected_attrs = Snapshot._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Snapshot,
|
||||
snapshots, expected_attrs=['metadata'])
|
||||
snapshots, expected_attrs=expected_attrs)
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_all_for_cgsnapshot(cls, context, cgsnapshot_id):
|
||||
snapshots = db.snapshot_get_all_for_cgsnapshot(context, cgsnapshot_id)
|
||||
expected_attrs = Snapshot._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Snapshot,
|
||||
snapshots, expected_attrs=['metadata'])
|
||||
snapshots, expected_attrs=expected_attrs)
|
||||
|
|
|
@ -62,8 +62,6 @@ class Volume(base.CinderPersistentObject, base.CinderObject,
|
|||
'volume_type', 'volume_attachment', 'consistencygroup',
|
||||
'snapshots')
|
||||
|
||||
DEFAULT_EXPECTED_ATTR = ('admin_metadata', 'metadata')
|
||||
|
||||
fields = {
|
||||
'id': fields.UUIDField(),
|
||||
'_name_id': fields.UUIDField(nullable=True),
|
||||
|
@ -124,6 +122,14 @@ class Volume(base.CinderPersistentObject, base.CinderObject,
|
|||
obj_extra_fields = ['name', 'name_id', 'volume_metadata',
|
||||
'volume_admin_metadata', 'volume_glance_metadata']
|
||||
|
||||
@classmethod
|
||||
def _get_expected_attrs(cls, context):
|
||||
expected_attrs = ['metadata', 'volume_type', 'volume_type.extra_specs']
|
||||
if context.is_admin:
|
||||
expected_attrs.append('admin_metadata')
|
||||
|
||||
return expected_attrs
|
||||
|
||||
@property
|
||||
def name_id(self):
|
||||
return self.id if not self._name_id else self._name_id
|
||||
|
@ -248,9 +254,12 @@ class Volume(base.CinderPersistentObject, base.CinderObject,
|
|||
if 'volume_type' in expected_attrs:
|
||||
db_volume_type = db_volume.get('volume_type')
|
||||
if db_volume_type:
|
||||
vt_expected_attrs = []
|
||||
if 'volume_type.extra_specs' in expected_attrs:
|
||||
vt_expected_attrs.append('extra_specs')
|
||||
volume.volume_type = objects.VolumeType._from_db_object(
|
||||
context, objects.VolumeType(), db_volume_type,
|
||||
expected_attrs='extra_specs')
|
||||
expected_attrs=vt_expected_attrs)
|
||||
if 'volume_attachment' in expected_attrs:
|
||||
attachments = base.obj_make_list(
|
||||
context, objects.VolumeAttachmentList(context),
|
||||
|
@ -430,27 +439,35 @@ class VolumeList(base.ObjectListBase, base.CinderObject):
|
|||
'1.1': '1.1',
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def _get_expected_attrs(cls, context):
|
||||
expected_attrs = ['metadata', 'volume_type']
|
||||
if context.is_admin:
|
||||
expected_attrs.append('admin_metadata')
|
||||
|
||||
return expected_attrs
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_all(cls, context, marker, limit, sort_keys=None, sort_dirs=None,
|
||||
filters=None, offset=None):
|
||||
volumes = db.volume_get_all(context, marker, limit,
|
||||
sort_keys=sort_keys, sort_dirs=sort_dirs,
|
||||
filters=filters, offset=offset)
|
||||
expected_attrs = ['admin_metadata', 'metadata']
|
||||
expected_attrs = cls._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Volume,
|
||||
volumes, expected_attrs=expected_attrs)
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_all_by_host(cls, context, host, filters=None):
|
||||
volumes = db.volume_get_all_by_host(context, host, filters)
|
||||
expected_attrs = ['admin_metadata', 'metadata']
|
||||
expected_attrs = cls._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Volume,
|
||||
volumes, expected_attrs=expected_attrs)
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_all_by_group(cls, context, group_id, filters=None):
|
||||
volumes = db.volume_get_all_by_group(context, group_id, filters)
|
||||
expected_attrs = ['admin_metadata', 'metadata']
|
||||
expected_attrs = cls._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Volume,
|
||||
volumes, expected_attrs=expected_attrs)
|
||||
|
||||
|
@ -462,6 +479,6 @@ class VolumeList(base.ObjectListBase, base.CinderObject):
|
|||
limit, sort_keys=sort_keys,
|
||||
sort_dirs=sort_dirs,
|
||||
filters=filters, offset=offset)
|
||||
expected_attrs = ['admin_metadata', 'metadata']
|
||||
expected_attrs = cls._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context), objects.Volume,
|
||||
volumes, expected_attrs=expected_attrs)
|
||||
|
|
|
@ -31,8 +31,6 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject,
|
|||
# Version 1.0: Initial version
|
||||
VERSION = '1.0'
|
||||
|
||||
DEFAULT_EXPECTED_ATTR = ('extra_specs', 'projects')
|
||||
|
||||
fields = {
|
||||
'id': fields.UUIDField(),
|
||||
'name': fields.StringField(nullable=True),
|
||||
|
@ -42,6 +40,10 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject,
|
|||
'extra_specs': fields.DictOfStringsField(nullable=True),
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def _get_expected_attrs(cls, context):
|
||||
return 'extra_specs', 'projects'
|
||||
|
||||
@staticmethod
|
||||
def _from_db_object(context, type, db_type, expected_attrs=None):
|
||||
if expected_attrs is None:
|
||||
|
@ -118,7 +120,7 @@ class VolumeTypeList(base.ObjectListBase, base.CinderObject):
|
|||
marker=marker, limit=limit,
|
||||
sort_keys=sort_keys,
|
||||
sort_dirs=sort_dirs, offset=offset)
|
||||
expected_attrs = ['extra_specs', 'projects']
|
||||
expected_attrs = VolumeType._get_expected_attrs(context)
|
||||
return base.obj_make_list(context, cls(context),
|
||||
objects.VolumeType, types.values(),
|
||||
expected_attrs=expected_attrs)
|
||||
|
|
|
@ -619,7 +619,8 @@ class VolumeImageActionsTest(test.TestCase):
|
|||
'status': 'uploading',
|
||||
'display_description': 'displaydesc',
|
||||
'size': 1,
|
||||
'volume_type': {'name': 'vol_type_name'},
|
||||
'volume_type': fake_volume.fake_db_volume_type(
|
||||
name='vol_type_name'),
|
||||
'image_id': 1,
|
||||
'container_format': 'bare',
|
||||
'disk_format': 'raw',
|
||||
|
@ -803,7 +804,8 @@ class VolumeImageActionsTest(test.TestCase):
|
|||
'status': 'uploading',
|
||||
'display_description': 'displaydesc',
|
||||
'size': 1,
|
||||
'volume_type': {'name': 'vol_type_name'},
|
||||
'volume_type': fake_volume.fake_db_volume_type(
|
||||
name='vol_type_name'),
|
||||
'image_id': 1,
|
||||
'container_format': 'bare',
|
||||
'disk_format': 'raw',
|
||||
|
@ -860,7 +862,8 @@ class VolumeImageActionsTest(test.TestCase):
|
|||
'status': 'uploading',
|
||||
'display_description': 'displaydesc',
|
||||
'size': 1,
|
||||
'volume_type': {'name': 'vol_type_name'},
|
||||
'volume_type': fake_volume.fake_db_volume_type(
|
||||
name='vol_type_name'),
|
||||
'image_id': 1,
|
||||
'container_format': 'bare',
|
||||
'disk_format': 'raw',
|
||||
|
@ -914,7 +917,8 @@ class VolumeImageActionsTest(test.TestCase):
|
|||
'status': 'uploading',
|
||||
'display_description': 'displaydesc',
|
||||
'size': 1,
|
||||
'volume_type': {'name': 'vol_type_name'},
|
||||
'volume_type': fake_volume.fake_db_volume_type(
|
||||
name='vol_type_name'),
|
||||
'image_id': 1,
|
||||
'container_format': 'bare',
|
||||
'disk_format': 'raw',
|
||||
|
@ -961,7 +965,8 @@ class VolumeImageActionsTest(test.TestCase):
|
|||
'status': 'uploading',
|
||||
'display_description': 'displaydesc',
|
||||
'size': 1,
|
||||
'volume_type': {'name': 'vol_type_name'},
|
||||
'volume_type': fake_volume.fake_db_volume_type(
|
||||
name='vol_type_name'),
|
||||
'image_id': 1,
|
||||
'container_format': 'bare',
|
||||
'disk_format': 'raw',
|
||||
|
|
|
@ -65,7 +65,7 @@ def stub_volume(id, **kwargs):
|
|||
'bootable': False,
|
||||
'launched_at': datetime.datetime(1900, 1, 1, 1, 1, 1,
|
||||
tzinfo=iso8601.iso8601.Utc()),
|
||||
'volume_type': {'name': DEFAULT_VOL_TYPE},
|
||||
'volume_type': fake_volume.fake_db_volume_type(name=DEFAULT_VOL_TYPE),
|
||||
'replication_status': 'disabled',
|
||||
'replication_extended_status': None,
|
||||
'replication_driver_data': None,
|
||||
|
|
Loading…
Reference in New Issue