diff --git a/oslo_limit/fixture.py b/oslo_limit/fixture.py index 032b7d0..dfea2cb 100644 --- a/oslo_limit/fixture.py +++ b/oslo_limit/fixture.py @@ -31,7 +31,7 @@ class LimitFixture(fixtures.Fixture): As in reality, only per-project overrides need be provided here; any unmentioned projects or resources will take the registered limit defaults. - :type reglimits: dict + :type projlimits: dict """ self.reglimits = reglimits self.projlimits = projlimits diff --git a/oslo_limit/limit.py b/oslo_limit/limit.py index d63f430..65c71eb 100644 --- a/oslo_limit/limit.py +++ b/oslo_limit/limit.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +from collections import defaultdict from collections import namedtuple from keystoneauth1 import exceptions as ksa_exceptions @@ -55,31 +56,34 @@ def _get_keystone_connection(): class Enforcer(object): - def __init__(self, usage_callback): + def __init__(self, usage_callback, cache=True): """An object for checking usage against resource limits and requests. :param usage_callback: A callable function that accepts a project_id string as a parameter and calculates the current usage of a resource. :type usage_callback: callable function + :param cache: Whether to cache resource limits for the lifetime of this + enforcer. Defaults to True. + :type cache: boolean """ if not callable(usage_callback): msg = 'usage_callback must be a callable function.' raise ValueError(msg) self.connection = _get_keystone_connection() - self.model = self._get_model_impl(usage_callback) + self.model = self._get_model_impl(usage_callback, cache=cache) def _get_enforcement_model(self): """Query keystone for the configured enforcement model.""" return self.connection.get('/limits/model').json()['model']['name'] - def _get_model_impl(self, usage_callback): + def _get_model_impl(self, usage_callback, cache=True): """get the enforcement model based on configured model in keystone.""" model = self._get_enforcement_model() for impl in _MODELS: if model == impl.name: - return impl(usage_callback) + return impl(usage_callback, cache=cache) raise ValueError("enforcement model %s is not supported" % model) def enforce(self, project_id, deltas): @@ -167,16 +171,16 @@ class Enforcer(object): usage = self.model.get_project_usage(project_id, resources_to_check) return {resource: ProjectUsage(limit, usage[resource]) - for resource, limit in dict(limits).items()} + for resource, limit in limits} class _FlatEnforcer(object): name = 'flat' - def __init__(self, usage_callback): + def __init__(self, usage_callback, cache=True): self._usage_callback = usage_callback - self._utils = _EnforcerUtils() + self._utils = _EnforcerUtils(cache=cache) def get_project_limits(self, project_id, resources_to_check): return self._utils.get_project_limits(project_id, resources_to_check) @@ -202,7 +206,7 @@ class _StrictTwoLevelEnforcer(object): name = 'strict-two-level' - def __init__(self, usage_callback): + def __init__(self, usage_callback, cache=True): self._usage_callback = usage_callback def get_project_limits(self, project_id, resources_to_check): @@ -229,8 +233,13 @@ class _LimitNotFound(Exception): class _EnforcerUtils(object): """Logic common used by multiple enforcers""" - def __init__(self): + def __init__(self, cache=True): self.connection = _get_keystone_connection() + self.should_cache = cache + # {project_id: {resource_name: project_limit}} + self.plimit_cache = defaultdict(dict) + # {resource_name: registered_limit} + self.rlimit_cache = {} # get and cache endpoint info endpoint_id = CONF.oslo_limit.endpoint_id @@ -298,12 +307,32 @@ class _EnforcerUtils(object): return project_limits def _get_limit(self, project_id, resource_name): - # TODO(johngarbutt): might need to cache here + # If we are configured to cache limits, look in the cache first and use + # the cached value if there is one. Else, retrieve the limit and add it + # to the cache. Do this for both project limits and registered limits. + + # Look for a project limit first. + if (project_id in self.plimit_cache and + resource_name in self.plimit_cache[project_id]): + return self.plimit_cache[project_id][resource_name].resource_limit + project_limit = self._get_project_limit(project_id, resource_name) + + if self.should_cache and project_limit: + self.plimit_cache[project_id][resource_name] = project_limit + if project_limit: return project_limit.resource_limit + # If there is no project limit, look for a registered limit. + if resource_name in self.rlimit_cache: + return self.rlimit_cache[resource_name].default_limit + registered_limit = self._get_registered_limit(resource_name) + + if self.should_cache and registered_limit: + self.rlimit_cache[resource_name] = registered_limit + if registered_limit: return registered_limit.default_limit diff --git a/oslo_limit/tests/test_limit.py b/oslo_limit/tests/test_limit.py index c229b6d..4c3d7ff 100644 --- a/oslo_limit/tests/test_limit.py +++ b/oslo_limit/tests/test_limit.py @@ -29,6 +29,7 @@ from oslo_config import fixture as config_fixture from oslotest import base from oslo_limit import exception +from oslo_limit import fixture from oslo_limit import limit from oslo_limit import opts @@ -309,3 +310,41 @@ class TestEnforcerUtils(base.BaseTestCase): self.assertEqual(0, over_d.limit) self.assertEqual(0, over_d.current_usage) self.assertEqual(0, over_d.delta) + + def test_get_limit_cache(self, cache=True): + # No project limit and registered limit = 5 + fix = self.useFixture(fixture.LimitFixture({'foo': 5}, {})) + project_id = uuid.uuid4().hex + + utils = limit._EnforcerUtils(cache=cache) + foo_limit = utils._get_limit(project_id, 'foo') + + self.assertEqual(5, foo_limit) + self.assertEqual(1, fix.mock_conn.registered_limits.call_count) + + # Second call should be cached, so call_count for registered limits + # should remain 1. When cache is disabled, it should increase to 2 + foo_limit = utils._get_limit(project_id, 'foo') + self.assertEqual(5, foo_limit) + count = 1 if cache else 2 + self.assertEqual(count, fix.mock_conn.registered_limits.call_count) + + # Add a project limit = 1 + fix.projlimits[project_id] = {'foo': 1} + + foo_limit = utils._get_limit(project_id, 'foo') + + self.assertEqual(1, foo_limit) + # Project limits should have been queried 3 times total, once per + # _get_limit call + self.assertEqual(3, fix.mock_conn.limits.call_count) + + # Fourth call should be cached, so call_count for project limits should + # remain 3. When cache is disabled, it should increase to 4 + foo_limit = utils._get_limit(project_id, 'foo') + self.assertEqual(1, foo_limit) + count = 3 if cache else 4 + self.assertEqual(count, fix.mock_conn.limits.call_count) + + def test_get_limit_no_cache(self): + self.test_get_limit_cache(cache=False) diff --git a/releasenotes/notes/enforcer-limit-caching-fb59725aad88b039.yaml b/releasenotes/notes/enforcer-limit-caching-fb59725aad88b039.yaml new file mode 100644 index 0000000..c2b45e4 --- /dev/null +++ b/releasenotes/notes/enforcer-limit-caching-fb59725aad88b039.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + ``Enforcer`` objects now cache limits by default for the lifetime of the + object to provide improved performance when multiple calls of ``enforce()`` + are needed. This behavior is controlled by the boolean ``cache`` keyword + argument to the ``__init__`` method.