From: Mathieu Gagné Date: Tue, 19 Aug 2014 21:07:42 +0000 (-0400) Subject: Improve Cinder API internal cache interface X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4aeb7256ef4cc75bae4ff9258a5bcf6092b3c834;p=openstack-build%2Fcinder-build.git 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 --- diff --git a/cinder/api/contrib/backups.py b/cinder/api/contrib/backups.py index 8ce51dc79..2910227d5 100644 --- a/cinder/api/contrib/backups.py +++ b/cinder/api/contrib/backups.py @@ -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) diff --git a/cinder/api/contrib/extended_snapshot_attributes.py b/cinder/api/contrib/extended_snapshot_attributes.py index 56cc24367..17e1e19da 100644 --- a/cinder/api/contrib/extended_snapshot_attributes.py +++ b/cinder/api/contrib/extended_snapshot_attributes.py @@ -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] diff --git a/cinder/api/contrib/volume_host_attribute.py b/cinder/api/contrib/volume_host_attribute.py index 7c984c154..f30922876 100644 --- a/cinder/api/contrib/volume_host_attribute.py +++ b/cinder/api/contrib/volume_host_attribute.py @@ -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'] diff --git a/cinder/api/contrib/volume_mig_status_attribute.py b/cinder/api/contrib/volume_mig_status_attribute.py index f54db1b8f..54038a1db 100644 --- a/cinder/api/contrib/volume_mig_status_attribute.py +++ b/cinder/api/contrib/volume_mig_status_attribute.py @@ -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 diff --git a/cinder/api/contrib/volume_tenant_attribute.py b/cinder/api/contrib/volume_tenant_attribute.py index 04957e226..07176f346 100644 --- a/cinder/api/contrib/volume_tenant_attribute.py +++ b/cinder/api/contrib/volume_tenant_attribute.py @@ -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'] diff --git a/cinder/api/openstack/wsgi.py b/cinder/api/openstack/wsgi.py index 9e0c2102f..d039ed557 100644 --- a/cinder/api/openstack/wsgi.py +++ b/cinder/api/openstack/wsgi.py @@ -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: diff --git a/cinder/api/v1/snapshots.py b/cinder/api/v1/snapshots.py index 922f67b81..28769344d 100644 --- a/cinder/api/v1/snapshots.py +++ b/cinder/api/v1/snapshots.py @@ -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)} diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py index 5e6968f7b..323a5eb7f 100644 --- a/cinder/api/v1/volumes.py +++ b/cinder/api/v1/volumes.py @@ -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} diff --git a/cinder/api/v2/snapshots.py b/cinder/api/v2/snapshots.py index 2c053ccb5..2d67330a7 100644 --- a/cinder/api/v2/snapshots.py +++ b/cinder/api/v2/snapshots.py @@ -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)} diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index 9c6f17da9..a813f8093 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -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): diff --git a/cinder/tests/api/contrib/test_extended_snapshot_attributes.py b/cinder/tests/api/contrib/test_extended_snapshot_attributes.py index 7df8ebb20..88a47e17c 100644 --- a/cinder/tests/api/contrib/test_extended_snapshot_attributes.py +++ b/cinder/tests/api/contrib/test_extended_snapshot_attributes.py @@ -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) diff --git a/cinder/tests/api/openstack/test_wsgi.py b/cinder/tests/api/openstack/test_wsgi.py index 0a39d8d96..254306439 100644 --- a/cinder/tests/api/openstack/test_wsgi.py +++ b/cinder/tests/api/openstack/test_wsgi.py @@ -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):