Improve Cinder API internal cache interface
Improve the internal caching system used by the API layer by borrowing the implementation used by Nova. Unlike the previous implementation, this new interface makes it clear which resource types are added or retrieved from the cache. This change also adds other resources to the cache: - backups - snapshots - volume types It's now possible to remove database access those extensions: - extended_snapshot_attributes - volume_host_attribute - volume_mig_status_attribute - volume_tenant_attribute Closes-bug: #1358524 Change-Id: I1fc673d0c01c41faa98292d5813d4471b455d712
This commit is contained in:
Mathieu Gagné
parent
110f3bcd09
commit
4aeb7256ef
|
|
@ -164,6 +164,7 @@ class BackupsController(wsgi.Controller):
|
|||
|
||||
try:
|
||||
backup = self.backup_api.get(context, backup_id=id)
|
||||
req.cache_db_backup(backup)
|
||||
except exception.BackupNotFound as error:
|
||||
raise exc.HTTPNotFound(explanation=error.msg)
|
||||
|
||||
|
|
@ -215,6 +216,7 @@ class BackupsController(wsgi.Controller):
|
|||
|
||||
backups = self.backup_api.get_all(context, search_opts=filters)
|
||||
limited_list = common.limited(backups, req)
|
||||
req.cache_db_backups(limited_list)
|
||||
|
||||
if is_detail:
|
||||
backups = self._view_builder.detail_list(req, limited_list)
|
||||
|
|
|
|||
|
|
@ -19,7 +19,6 @@ from cinder.api import extensions
|
|||
from cinder.api.openstack import wsgi
|
||||
from cinder.api import xmlutil
|
||||
from cinder.openstack.common import log as logging
|
||||
from cinder import volume
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
|
@ -29,18 +28,8 @@ authorize = extensions.soft_extension_authorizer(
|
|||
|
||||
|
||||
class ExtendedSnapshotAttributesController(wsgi.Controller):
|
||||
def __init__(self, *args, **kwargs):
|
||||
super(ExtendedSnapshotAttributesController, self).__init__(*args,
|
||||
**kwargs)
|
||||
self.volume_api = volume.API()
|
||||
|
||||
def _get_snapshots(self, context):
|
||||
snapshots = self.volume_api.get_all_snapshots(context)
|
||||
rval = dict((snapshot['id'], snapshot) for snapshot in snapshots)
|
||||
return rval
|
||||
|
||||
def _extend_snapshot(self, req, resp_snap):
|
||||
db_snap = req.cached_resource_by_id(resp_snap['id'])
|
||||
db_snap = req.get_db_snapshot(resp_snap['id'])
|
||||
for attr in ['project_id', 'progress']:
|
||||
key = "%s:%s" % (Extended_snapshot_attributes.alias, attr)
|
||||
resp_snap[key] = db_snap[attr]
|
||||
|
|
|
|||
|
|
@ -16,7 +16,6 @@ from cinder.api import extensions
|
|||
from cinder.api.openstack import wsgi
|
||||
from cinder.api import xmlutil
|
||||
from cinder.openstack.common import log as logging
|
||||
from cinder import volume
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
|
@ -25,12 +24,8 @@ authorize = extensions.soft_extension_authorizer('volume',
|
|||
|
||||
|
||||
class VolumeHostAttributeController(wsgi.Controller):
|
||||
def __init__(self, *args, **kwargs):
|
||||
super(VolumeHostAttributeController, self).__init__(*args, **kwargs)
|
||||
self.volume_api = volume.API()
|
||||
|
||||
def _add_volume_host_attribute(self, context, req, resp_volume):
|
||||
db_volume = req.cached_resource_by_id(resp_volume['id'])
|
||||
db_volume = req.get_db_volume(resp_volume['id'])
|
||||
key = "%s:host" % Volume_host_attribute.alias
|
||||
resp_volume[key] = db_volume['host']
|
||||
|
||||
|
|
|
|||
|
|
@ -15,7 +15,6 @@
|
|||
from cinder.api import extensions
|
||||
from cinder.api.openstack import wsgi
|
||||
from cinder.api import xmlutil
|
||||
from cinder import volume
|
||||
|
||||
|
||||
authorize = extensions.soft_extension_authorizer('volume',
|
||||
|
|
@ -23,13 +22,8 @@ authorize = extensions.soft_extension_authorizer('volume',
|
|||
|
||||
|
||||
class VolumeMigStatusAttributeController(wsgi.Controller):
|
||||
def __init__(self, *args, **kwargs):
|
||||
super(VolumeMigStatusAttributeController, self).__init__(*args,
|
||||
**kwargs)
|
||||
self.volume_api = volume.API()
|
||||
|
||||
def _add_volume_mig_status_attribute(self, req, context, resp_volume):
|
||||
db_volume = req.cached_resource_by_id(resp_volume['id'])
|
||||
db_volume = req.get_db_volume(resp_volume['id'])
|
||||
key = "%s:migstat" % Volume_mig_status_attribute.alias
|
||||
resp_volume[key] = db_volume['migration_status']
|
||||
key = "%s:name_id" % Volume_mig_status_attribute.alias
|
||||
|
|
|
|||
|
|
@ -15,7 +15,6 @@
|
|||
from cinder.api import extensions
|
||||
from cinder.api.openstack import wsgi
|
||||
from cinder.api import xmlutil
|
||||
from cinder import volume
|
||||
|
||||
|
||||
authorize = extensions.soft_extension_authorizer('volume',
|
||||
|
|
@ -23,12 +22,8 @@ authorize = extensions.soft_extension_authorizer('volume',
|
|||
|
||||
|
||||
class VolumeTenantAttributeController(wsgi.Controller):
|
||||
def __init__(self, *args, **kwargs):
|
||||
super(VolumeTenantAttributeController, self).__init__(*args, **kwargs)
|
||||
self.volume_api = volume.API()
|
||||
|
||||
def _add_volume_tenant_attribute(self, context, req, resp_volume):
|
||||
db_volume = req.cached_resource_by_id(resp_volume['id'])
|
||||
db_volume = req.get_db_volume(resp_volume['id'])
|
||||
key = "%s:tenant_id" % Volume_tenant_attribute.alias
|
||||
resp_volume[key] = db_volume['project_id']
|
||||
|
||||
|
|
|
|||
|
|
@ -131,6 +131,86 @@ class Request(webob.Request):
|
|||
return None
|
||||
return resources.get(resource_id)
|
||||
|
||||
def cache_db_items(self, key, items, item_key='id'):
|
||||
"""Allow API methods to store objects from a DB query to be
|
||||
used by API extensions within the same API request.
|
||||
|
||||
An instance of this class only lives for the lifetime of a
|
||||
single API request, so there's no need to implement full
|
||||
cache management.
|
||||
"""
|
||||
self.cache_resource(items, item_key, key)
|
||||
|
||||
def get_db_items(self, key):
|
||||
"""Allow an API extension to get previously stored objects within
|
||||
the same API request.
|
||||
|
||||
Note that the object data will be slightly stale.
|
||||
"""
|
||||
return self.cached_resource(key)
|
||||
|
||||
def get_db_item(self, key, item_key):
|
||||
"""Allow an API extension to get a previously stored object
|
||||
within the same API request.
|
||||
|
||||
Note that the object data will be slightly stale.
|
||||
"""
|
||||
return self.get_db_items(key).get(item_key)
|
||||
|
||||
def cache_db_volumes(self, volumes):
|
||||
# NOTE(mgagne) Cache it twice for backward compatibility reasons
|
||||
self.cache_db_items('volumes', volumes, 'id')
|
||||
self.cache_db_items(self.path, volumes, 'id')
|
||||
|
||||
def cache_db_volume(self, volume):
|
||||
# NOTE(mgagne) Cache it twice for backward compatibility reasons
|
||||
self.cache_db_items('volumes', [volume], 'id')
|
||||
self.cache_db_items(self.path, [volume], 'id')
|
||||
|
||||
def get_db_volumes(self):
|
||||
return (self.get_db_items('volumes') or
|
||||
self.get_db_items(self.path))
|
||||
|
||||
def get_db_volume(self, volume_id):
|
||||
return (self.get_db_item('volumes', volume_id) or
|
||||
self.get_db_item(self.path, volume_id))
|
||||
|
||||
def cache_db_volume_types(self, volume_types):
|
||||
self.cache_db_items('volume_types', volume_types, 'id')
|
||||
|
||||
def cache_db_volume_type(self, volume_type):
|
||||
self.cache_db_items('volume_types', [volume_type], 'id')
|
||||
|
||||
def get_db_volume_types(self):
|
||||
return self.get_db_items('volume_types')
|
||||
|
||||
def get_db_volume_type(self, volume_type_id):
|
||||
return self.get_db_item('volume_types', volume_type_id)
|
||||
|
||||
def cache_db_snapshots(self, snapshots):
|
||||
self.cache_db_items('snapshots', snapshots, 'id')
|
||||
|
||||
def cache_db_snapshot(self, snapshot):
|
||||
self.cache_db_items('snapshots', [snapshot], 'id')
|
||||
|
||||
def get_db_snapshots(self):
|
||||
return self.get_db_items('snapshots')
|
||||
|
||||
def get_db_snapshot(self, snapshot_id):
|
||||
return self.get_db_item('snapshots', snapshot_id)
|
||||
|
||||
def cache_db_backups(self, backups):
|
||||
self.cache_db_items('backups', backups, 'id')
|
||||
|
||||
def cache_db_backup(self, backup):
|
||||
self.cache_db_items('backups', [backup], 'id')
|
||||
|
||||
def get_db_backups(self):
|
||||
return self.get_db_items('backups')
|
||||
|
||||
def get_db_backup(self, backup_id):
|
||||
return self.get_db_item('backups', backup_id)
|
||||
|
||||
def best_match_content_type(self):
|
||||
"""Determine the requested response content-type."""
|
||||
if 'cinder.best_content_type' not in self.environ:
|
||||
|
|
|
|||
|
|
@ -107,7 +107,7 @@ class SnapshotsController(wsgi.Controller):
|
|||
|
||||
try:
|
||||
snapshot = self.volume_api.get_snapshot(context, id)
|
||||
req.cache_resource(snapshot)
|
||||
req.cache_db_snapshot(snapshot)
|
||||
except exception.NotFound:
|
||||
raise exc.HTTPNotFound()
|
||||
|
||||
|
|
@ -153,7 +153,7 @@ class SnapshotsController(wsgi.Controller):
|
|||
snapshots = self.volume_api.get_all_snapshots(context,
|
||||
search_opts=search_opts)
|
||||
limited_list = common.limited(snapshots, req)
|
||||
req.cache_resource(limited_list)
|
||||
req.cache_db_snapshots(limited_list)
|
||||
res = [entity_maker(context, snapshot) for snapshot in limited_list]
|
||||
return {'snapshots': res}
|
||||
|
||||
|
|
@ -202,6 +202,7 @@ class SnapshotsController(wsgi.Controller):
|
|||
snapshot.get('display_name'),
|
||||
snapshot.get('display_description'),
|
||||
**kwargs)
|
||||
req.cache_db_snapshot(new_snapshot)
|
||||
|
||||
retval = _translate_snapshot_detail_view(context, new_snapshot)
|
||||
|
||||
|
|
@ -237,6 +238,7 @@ class SnapshotsController(wsgi.Controller):
|
|||
raise exc.HTTPNotFound()
|
||||
|
||||
snapshot.update(update_dict)
|
||||
req.cache_db_snapshot(snapshot)
|
||||
|
||||
return {'snapshot': _translate_snapshot_detail_view(context, snapshot)}
|
||||
|
||||
|
|
|
|||
|
|
@ -232,7 +232,7 @@ class VolumeController(wsgi.Controller):
|
|||
|
||||
try:
|
||||
vol = self.volume_api.get(context, id, viewable_admin_meta=True)
|
||||
req.cache_resource(vol)
|
||||
req.cache_db_volume(vol)
|
||||
except exception.NotFound:
|
||||
raise exc.HTTPNotFound()
|
||||
|
||||
|
|
@ -290,7 +290,8 @@ class VolumeController(wsgi.Controller):
|
|||
utils.add_visible_admin_metadata(volume)
|
||||
|
||||
limited_list = common.limited(volumes, req)
|
||||
req.cache_resource(limited_list)
|
||||
req.cache_db_volumes(limited_list)
|
||||
|
||||
res = [entity_maker(context, vol) for vol in limited_list]
|
||||
return {'volumes': res}
|
||||
|
||||
|
|
|
|||
|
|
@ -107,7 +107,7 @@ class SnapshotsController(wsgi.Controller):
|
|||
|
||||
try:
|
||||
snapshot = self.volume_api.get_snapshot(context, id)
|
||||
req.cache_resource(snapshot)
|
||||
req.cache_db_snapshot(snapshot)
|
||||
except exception.NotFound:
|
||||
msg = _("Snapshot could not be found")
|
||||
raise exc.HTTPNotFound(explanation=msg)
|
||||
|
|
@ -161,7 +161,7 @@ class SnapshotsController(wsgi.Controller):
|
|||
snapshots = self.volume_api.get_all_snapshots(context,
|
||||
search_opts=search_opts)
|
||||
limited_list = common.limited(snapshots, req)
|
||||
req.cache_resource(limited_list)
|
||||
req.cache_db_snapshots(limited_list)
|
||||
res = [entity_maker(context, snapshot) for snapshot in limited_list]
|
||||
return {'snapshots': res}
|
||||
|
||||
|
|
@ -218,6 +218,7 @@ class SnapshotsController(wsgi.Controller):
|
|||
snapshot.get('display_name'),
|
||||
snapshot.get('description'),
|
||||
**kwargs)
|
||||
req.cache_db_snapshot(new_snapshot)
|
||||
|
||||
retval = _translate_snapshot_detail_view(context, new_snapshot)
|
||||
|
||||
|
|
@ -270,6 +271,7 @@ class SnapshotsController(wsgi.Controller):
|
|||
raise exc.HTTPNotFound(explanation=msg)
|
||||
|
||||
snapshot.update(update_dict)
|
||||
req.cache_db_snapshot(snapshot)
|
||||
|
||||
return {'snapshot': _translate_snapshot_detail_view(context, snapshot)}
|
||||
|
||||
|
|
|
|||
|
|
@ -167,7 +167,7 @@ class VolumeController(wsgi.Controller):
|
|||
|
||||
try:
|
||||
vol = self.volume_api.get(context, id, viewable_admin_meta=True)
|
||||
req.cache_resource(vol)
|
||||
req.cache_db_volume(vol)
|
||||
except exception.NotFound:
|
||||
msg = _("Volume could not be found")
|
||||
raise exc.HTTPNotFound(explanation=msg)
|
||||
|
|
@ -238,12 +238,12 @@ class VolumeController(wsgi.Controller):
|
|||
utils.add_visible_admin_metadata(volume)
|
||||
|
||||
limited_list = common.limited(volumes, req)
|
||||
req.cache_db_volumes(limited_list)
|
||||
|
||||
if is_detail:
|
||||
volumes = self._view_builder.detail_list(req, limited_list)
|
||||
else:
|
||||
volumes = self._view_builder.summary_list(req, limited_list)
|
||||
req.cache_resource(limited_list)
|
||||
return volumes
|
||||
|
||||
def _image_uuid_from_href(self, image_href):
|
||||
|
|
|
|||
|
|
@ -78,7 +78,7 @@ class ExtendedSnapshotAttributesTest(test.TestCase):
|
|||
self.assertEqual(snapshot.get('%sprogress' % self.prefix), progress)
|
||||
|
||||
def test_show(self):
|
||||
url = '/v2/fake/snapshots/%s' % UUID2
|
||||
url = '/v2/fake/snapshots/%s' % UUID1
|
||||
res = self._make_request(url)
|
||||
|
||||
self.assertEqual(res.status_int, 200)
|
||||
|
|
|
|||
|
|
@ -140,6 +140,44 @@ class RequestTest(test.TestCase):
|
|||
request.cached_resource_by_id('o-0',
|
||||
name='other-resource'))
|
||||
|
||||
def test_cache_and_retrieve_volumes(self):
|
||||
self._test_cache_and_retrieve_resources('volume')
|
||||
|
||||
def test_cache_and_retrieve_volume_types(self):
|
||||
self._test_cache_and_retrieve_resources('volume_type')
|
||||
|
||||
def test_cache_and_retrieve_snapshots(self):
|
||||
self._test_cache_and_retrieve_resources('snapshot')
|
||||
|
||||
def test_cache_and_retrieve_backups(self):
|
||||
self._test_cache_and_retrieve_resources('backup')
|
||||
|
||||
def _test_cache_and_retrieve_resources(self, resource_name):
|
||||
"""Generic helper for cache tests."""
|
||||
cache_all_func = 'cache_db_%ss' % resource_name
|
||||
cache_one_func = 'cache_db_%s' % resource_name
|
||||
get_db_all_func = 'get_db_%ss' % resource_name
|
||||
get_db_one_func = 'get_db_%s' % resource_name
|
||||
|
||||
r = wsgi.Request.blank('/foo')
|
||||
resources = []
|
||||
for x in xrange(3):
|
||||
resources.append({'id': 'id%s' % x})
|
||||
|
||||
# Store 2
|
||||
getattr(r, cache_all_func)(resources[:2])
|
||||
# Store 1
|
||||
getattr(r, cache_one_func)(resources[2])
|
||||
|
||||
self.assertEqual(getattr(r, get_db_one_func)('id0'), resources[0])
|
||||
self.assertEqual(getattr(r, get_db_one_func)('id1'), resources[1])
|
||||
self.assertEqual(getattr(r, get_db_one_func)('id2'), resources[2])
|
||||
self.assertIsNone(getattr(r, get_db_one_func)('id3'))
|
||||
self.assertEqual(getattr(r, get_db_all_func)(), {
|
||||
'id0': resources[0],
|
||||
'id1': resources[1],
|
||||
'id2': resources[2]})
|
||||
|
||||
|
||||
class ActionDispatcherTest(test.TestCase):
|
||||
def test_dispatch(self):
|
||||
|
|
|
|||
Loading…
Reference in New Issue