From ee58a8cc976ab8885965ec983636d8d7071fd1b9 Mon Sep 17 00:00:00 2001 From: ajayaa Date: Wed, 23 Jul 2014 17:23:31 +0530 Subject: [PATCH] cache the catalog bp expand-keystone-caching Change-Id: I448a7dc66210abbfa8568567fe5acd88a3b2771a --- etc/keystone.conf.sample | 8 +++ keystone/catalog/core.py | 26 ++++++- keystone/common/config.py | 7 ++ keystone/tests/test_backend.py | 127 +++++++++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 3 deletions(-) diff --git a/etc/keystone.conf.sample b/etc/keystone.conf.sample index bddcc24c47..631084a853 100644 --- a/etc/keystone.conf.sample +++ b/etc/keystone.conf.sample @@ -590,6 +590,14 @@ # Catalog backend driver. (string value) #driver=keystone.catalog.backends.sql.Catalog +# Toggle for catalog caching. This has no effect unless +# global caching is enabled. (boolean value) +#caching=true + +# TTL (in seconds) to cache catalog data. This has no +# effect unless global caching is enabled. (integer value) +#cache_time= + # Maximum number of entities that will be returned in a # catalog collection. (integer value) #list_limit= diff --git a/keystone/catalog/core.py b/keystone/catalog/core.py index 565f353851..fc24deb121 100644 --- a/keystone/catalog/core.py +++ b/keystone/catalog/core.py @@ -19,6 +19,7 @@ import abc import six +from keystone.common import cache from keystone.common import dependency from keystone.common import driver_hints from keystone.common import manager @@ -30,6 +31,9 @@ from keystone.openstack.common import log CONF = config.CONF LOG = log.getLogger(__name__) +SHOULD_CACHE = cache.should_cache_fn('catalog') + +EXPIRATION_TIME = lambda: CONF.catalog.cache_time def format_url(url, data): @@ -90,6 +94,8 @@ class Manager(manager.Manager): parent_region_id = region_ref.get('parent_region_id') raise exception.RegionNotFound(region_id=parent_region_id) + @cache.on_arguments(should_cache_fn=SHOULD_CACHE, + expiration_time=EXPIRATION_TIME) def get_region(self, region_id): try: return self.driver.get_region(region_id) @@ -98,7 +104,9 @@ class Manager(manager.Manager): def delete_region(self, region_id): try: - return self.driver.delete_region(region_id) + ret = self.driver.delete_region(region_id) + self.get_region.invalidate(self, region_id) + return ret except exception.NotFound: raise exception.RegionNotFound(region_id=region_id) @@ -106,6 +114,8 @@ class Manager(manager.Manager): service_ref.setdefault('enabled', True) return self.driver.create_service(service_id, service_ref) + @cache.on_arguments(should_cache_fn=SHOULD_CACHE, + expiration_time=EXPIRATION_TIME) def get_service(self, service_id): try: return self.driver.get_service(service_id) @@ -114,7 +124,13 @@ class Manager(manager.Manager): def delete_service(self, service_id): try: - return self.driver.delete_service(service_id) + endpoints = self.list_endpoints() + ret = self.driver.delete_service(service_id) + self.get_service.invalidate(self, service_id) + for endpoint in endpoints: + if endpoint['service_id'] == service_id: + self.get_endpoint.invalidate(self, endpoint['id']) + return ret except exception.NotFound: raise exception.ServiceNotFound(service_id=service_id) @@ -131,10 +147,14 @@ class Manager(manager.Manager): def delete_endpoint(self, endpoint_id): try: - return self.driver.delete_endpoint(endpoint_id) + ret = self.driver.delete_endpoint(endpoint_id) + self.get_endpoint.invalidate(self, endpoint_id) + return ret except exception.NotFound: raise exception.EndpointNotFound(endpoint_id=endpoint_id) + @cache.on_arguments(should_cache_fn=SHOULD_CACHE, + expiration_time=EXPIRATION_TIME) def get_endpoint(self, endpoint_id): try: return self.driver.get_endpoint(endpoint_id) diff --git a/keystone/common/config.py b/keystone/common/config.py index b5e347e8e0..073980fd19 100644 --- a/keystone/common/config.py +++ b/keystone/common/config.py @@ -742,6 +742,13 @@ FILE_OPTIONS = { cfg.StrOpt('driver', default='keystone.catalog.backends.sql.Catalog', help='Catalog backend driver.'), + cfg.BoolOpt('caching', default=True, + help='Toggle for catalog caching. This has no ' + 'effect unless global caching is enabled.'), + cfg.IntOpt('cache_time', + help='Time to cache catalog data (in seconds). This has no ' + 'effect unless global and catalog caching are ' + 'enabled.'), cfg.IntOpt('list_limit', help='Maximum number of entities that will be returned ' 'in a catalog collection.'), diff --git a/keystone/tests/test_backend.py b/keystone/tests/test_backend.py index 4e9bb6a9f2..32c7cb0fb5 100644 --- a/keystone/tests/test_backend.py +++ b/keystone/tests/test_backend.py @@ -3583,6 +3583,34 @@ class CatalogTests(object): self.catalog_api.get_region, region_id) + @tests.skip_if_cache_disabled('catalog') + def test_cache_layer_region_crud(self): + region_id = uuid.uuid4().hex + new_region = { + 'id': region_id, + 'description': uuid.uuid4().hex, + } + self.catalog_api.create_region(new_region.copy()) + updated_region = copy.deepcopy(new_region) + updated_region['description'] = uuid.uuid4().hex + # cache the result + self.catalog_api.get_region(region_id) + # update the region bypassing catalog_api + self.catalog_api.driver.update_region(region_id, updated_region) + self.assertDictContainsSubset(new_region, + self.catalog_api.get_region(region_id)) + self.catalog_api.get_region.invalidate(self.catalog_api, region_id) + self.assertDictContainsSubset(updated_region, + self.catalog_api.get_region(region_id)) + # delete the region + self.catalog_api.driver.delete_region(region_id) + # still get the old region + self.assertDictContainsSubset(updated_region, + self.catalog_api.get_region(region_id)) + self.catalog_api.get_region.invalidate(self.catalog_api, region_id) + self.assertRaises(exception.RegionNotFound, + self.catalog_api.get_region, region_id) + def test_create_region_with_duplicate_id(self): region_id = uuid.uuid4().hex new_region = { @@ -3644,6 +3672,42 @@ class CatalogTests(object): self.catalog_api.get_service, service_id) + @tests.skip_if_cache_disabled('catalog') + def test_cache_layer_service_crud(self): + service_id = uuid.uuid4().hex + new_service = { + 'id': service_id, + 'type': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'description': uuid.uuid4().hex, + } + res = self.catalog_api.create_service( + service_id, + new_service.copy()) + new_service['enabled'] = True + self.assertDictEqual(new_service, res) + self.catalog_api.get_service(service_id) + updated_service = copy.deepcopy(new_service) + updated_service['description'] = uuid.uuid4().hex + self.catalog_api.update_service(service_id, updated_service) + self.assertDictContainsSubset(new_service, + self.catalog_api.get_service(service_id)) + self.catalog_api.get_service.invalidate(self.catalog_api, service_id) + self.assertDictContainsSubset(updated_service, + self.catalog_api.get_service(service_id)) + + # delete bypassing catalog api + self.catalog_api.driver.delete_service(service_id) + self.assertDictContainsSubset(updated_service, + self.catalog_api.get_service(service_id)) + self.catalog_api.get_service.invalidate(self.catalog_api, service_id) + self.assertRaises(exception.ServiceNotFound, + self.catalog_api.delete_service, + service_id) + self.assertRaises(exception.ServiceNotFound, + self.catalog_api.get_service, + service_id) + def test_delete_service_with_endpoint(self): # create a service service = { @@ -3673,6 +3737,69 @@ class CatalogTests(object): self.catalog_api.delete_endpoint, endpoint['id']) + def test_cache_layer_delete_service_with_endpoint(self): + service = { + 'id': uuid.uuid4().hex, + 'type': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'description': uuid.uuid4().hex, + } + self.catalog_api.create_service(service['id'], service) + + # create an endpoint attached to the service + endpoint = { + 'id': uuid.uuid4().hex, + 'region': uuid.uuid4().hex, + 'interface': uuid.uuid4().hex[:8], + 'url': uuid.uuid4().hex, + 'service_id': service['id'], + } + self.catalog_api.create_endpoint(endpoint['id'], endpoint) + # cache the result + self.catalog_api.get_service(service['id']) + self.catalog_api.get_endpoint(endpoint['id']) + # delete the service bypassing catalog api + self.catalog_api.driver.delete_service(service['id']) + self.assertDictContainsSubset(endpoint, + self.catalog_api. + get_endpoint(endpoint['id'])) + self.assertDictContainsSubset(service, + self.catalog_api. + get_service(service['id'])) + self.catalog_api.get_endpoint.invalidate(self.catalog_api, + endpoint['id']) + self.assertRaises(exception.EndpointNotFound, + self.catalog_api.get_endpoint, + endpoint['id']) + self.assertRaises(exception.EndpointNotFound, + self.catalog_api.delete_endpoint, + endpoint['id']) + # multiple endpoints associated with a service + second_endpoint = { + 'id': uuid.uuid4().hex, + 'region': uuid.uuid4().hex, + 'interface': uuid.uuid4().hex[:8], + 'url': uuid.uuid4().hex, + 'service_id': service['id'], + } + self.catalog_api.create_service(service['id'], service) + self.catalog_api.create_endpoint(endpoint['id'], endpoint) + self.catalog_api.create_endpoint(second_endpoint['id'], + second_endpoint) + self.catalog_api.delete_service(service['id']) + self.assertRaises(exception.EndpointNotFound, + self.catalog_api.get_endpoint, + endpoint['id']) + self.assertRaises(exception.EndpointNotFound, + self.catalog_api.delete_endpoint, + endpoint['id']) + self.assertRaises(exception.EndpointNotFound, + self.catalog_api.get_endpoint, + second_endpoint['id']) + self.assertRaises(exception.EndpointNotFound, + self.catalog_api.delete_endpoint, + second_endpoint['id']) + def test_get_service_404(self): self.assertRaises(exception.ServiceNotFound, self.catalog_api.get_service,