From edb09c5f91821dbee92749686843460e5697b168 Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Fri, 30 Aug 2019 18:10:26 +0100 Subject: [PATCH] Switch ConsumerType to use an AttributeCache ConsumerType has the same structure and behavior as a Trait or ResourceClass and, like them, we really only ever care about the name and/or the id. This makes it a good candidate to use the AttributeCache, simplifying the code a fair amount. Co-Authored-By: melanie witt Change-Id: I32499b2616e253a06f8a003fd3eef91006b58f42 --- placement/attribute_cache.py | 16 ++++++ placement/context.py | 1 + placement/handlers/allocation.py | 14 ++--- placement/handlers/util.py | 4 +- placement/objects/consumer_type.py | 59 +++------------------- placement/tests/unit/base.py | 2 + placement/tests/unit/handlers/test_util.py | 24 +++------ 7 files changed, 39 insertions(+), 81 deletions(-) diff --git a/placement/attribute_cache.py b/placement/attribute_cache.py index e4f735211..d4ef1014c 100644 --- a/placement/attribute_cache.py +++ b/placement/attribute_cache.py @@ -15,7 +15,9 @@ import sqlalchemy as sa from placement.db.sqlalchemy import models from placement import db_api from placement import exception +from placement.objects import consumer_type as ct_obj +_CONSUMER_TYPE_TBL = models.ConsumerType.__table__ _RC_TBL = models.ResourceClass.__table__ _TRAIT_TBL = models.Trait.__table__ @@ -149,6 +151,20 @@ class _AttributeCache(object): self._all_cache = {r[1]: r for r in res} +class ConsumerTypeCache(_AttributeCache): + """An _AttributeCache for consumer types.""" + + _table = _CONSUMER_TYPE_TBL + _not_found = exception.ConsumerTypeNotFound + + @db_api.placement_context_manager.reader + def _refresh_from_db(self, ctx): + super(ConsumerTypeCache, self)._refresh_from_db(ctx) + # The consumer_type_id is nullable and records with a NULL (None) + # consumer_type_id are considered as 'unknown'. + self._id_cache[None] = ct_obj.NULL_CONSUMER_TYPE_ALIAS + + class ResourceClassCache(_AttributeCache): """An _AttributeCache for resource classes.""" diff --git a/placement/context.py b/placement/context.py index fc1e30ad8..053d88056 100644 --- a/placement/context.py +++ b/placement/context.py @@ -23,6 +23,7 @@ class RequestContext(context.RequestContext): def __init__(self, *args, **kwargs): self.config = kwargs.pop('config', None) + self.ct_cache = attribute_cache.ConsumerTypeCache(self) self.rc_cache = attribute_cache.ResourceClassCache(self) self.trait_cache = attribute_cache.TraitCache(self) super(RequestContext, self).__init__(*args, **kwargs) diff --git a/placement/handlers/allocation.py b/placement/handlers/allocation.py index c3120be7d..2207a0e04 100644 --- a/placement/handlers/allocation.py +++ b/placement/handlers/allocation.py @@ -28,7 +28,6 @@ from placement import exception from placement.handlers import util as data_util from placement import microversion from placement.objects import allocation as alloc_obj -from placement.objects import consumer_type from placement.objects import resource_provider as rp_obj from placement.policies import allocation as policies from placement.schemas import allocation as schema @@ -111,16 +110,9 @@ def _serialize_allocations_for_consumer(context, allocations, want_version): result['consumer_generation'] = consumer.generation show_consumer_type = want_version.matches((1, 38)) if show_consumer_type: - # TODO(cdent): This should either access a subclass of - # AttributeCache or the data returned from the persistence layer - # should already have a name. We want to avoid accessing the - # database from the handler layer repeated times. - if consumer.consumer_type_id: - con_type = consumer_type.ConsumerType.get_by_id( - context, consumer.consumer_type_id).name - else: - con_type = consumer_type.DEFAULT_CONSUMER_TYPE - result['consumer_type'] = con_type + con_name = context.ct_cache.string_from_id( + consumer.consumer_type_id) + result['consumer_type'] = con_name return result diff --git a/placement/handlers/util.py b/placement/handlers/util.py index c37e9ba09..d397663b5 100644 --- a/placement/handlers/util.py +++ b/placement/handlers/util.py @@ -34,15 +34,15 @@ def fetch_consumer_type_id(ctx, name): :returns: The id of the ConsumerType object. """ try: - cons_type = consumer_type_obj.ConsumerType.get_by_name(ctx, name) + return ctx.ct_cache.id_from_string(name) except exception.ConsumerTypeNotFound: cons_type = consumer_type_obj.ConsumerType(ctx, name=name) try: cons_type.create() + return cons_type.id except exception.ConsumerTypeExists: # another thread created concurrently, so try again return fetch_consumer_type_id(ctx, name) - return cons_type.id def _get_or_create_project(ctx, project_id): diff --git a/placement/objects/consumer_type.py b/placement/objects/consumer_type.py index 9a7011f67..b98ed3286 100644 --- a/placement/objects/consumer_type.py +++ b/placement/objects/consumer_type.py @@ -11,7 +11,6 @@ # under the License. from oslo_db import exception as db_exc -import sqlalchemy as sa from placement.db.sqlalchemy import models from placement import db_api @@ -23,52 +22,6 @@ _CONSUMER_TYPES_SYNCED = False NULL_CONSUMER_TYPE_ALIAS = 'unknown' -@db_api.placement_context_manager.reader -def _get_consumer_type_by_id(ctx, id): - # The SQL for this looks like the following: - # SELECT - # c.id, c.name, - # c.updated_at, c.created_at - # FROM consumer_types c - # WHERE c.id = $id - consumer_types = sa.alias(CONSUMER_TYPE_TBL, name="c") - cols = [ - consumer_types.c.id, - consumer_types.c.name, - consumer_types.c.updated_at, - consumer_types.c.created_at - ] - sel = sa.select(cols).where(consumer_types.c.id == id) - res = ctx.session.execute(sel).fetchone() - if not res: - raise exception.ConsumerTypeNotFound(name=id) - - return dict(res) - - -@db_api.placement_context_manager.reader -def _get_consumer_type_by_name(ctx, name): - # The SQL for this looks like the following: - # SELECT - # c.id, c.name, - # c.updated_at, c.created_at - # FROM consumer_types c - # WHERE c.name = $name - consumer_types = sa.alias(CONSUMER_TYPE_TBL, name="c") - cols = [ - consumer_types.c.id, - consumer_types.c.name, - consumer_types.c.updated_at, - consumer_types.c.created_at - ] - sel = sa.select(cols).where(consumer_types.c.name == name) - res = ctx.session.execute(sel).fetchone() - if not res: - raise exception.ConsumerTypeNotFound(name=name) - - return dict(res) - - @db_api.placement_context_manager.writer def _create_in_db(ctx, name): db_obj = models.ConsumerType(name=name) @@ -99,16 +52,18 @@ class ConsumerType(object): target._context = ctx return target + # NOTE(cdent): get_by_id and get_by_name are not currently used + # but are left in place to indicate the smooth migration from + # direct db access to using the AttributeCache. @classmethod def get_by_id(cls, ctx, id): - res = _get_consumer_type_by_id(ctx, id) - return cls._from_db_object(ctx, cls(ctx), res) + return ctx.ct_cache.all_from_string(ctx.ct_cache.string_from_id(id)) @classmethod def get_by_name(cls, ctx, name): - res = _get_consumer_type_by_name(ctx, name) - return cls._from_db_object(ctx, cls(ctx), res) + return ctx.ct_cache.all_from_string(name) def create(self): ct = _create_in_db(self._context, self.name) - return self._from_db_object(self._context, self, ct) + self._from_db_object(self._context, self, ct) + self._context.ct_cache.clear() diff --git a/placement/tests/unit/base.py b/placement/tests/unit/base.py index f7ef0e92d..40d955242 100644 --- a/placement/tests/unit/base.py +++ b/placement/tests/unit/base.py @@ -20,6 +20,8 @@ class ContextTestCase(testtools.TestCase): def setUp(self): super(ContextTestCase, self).setUp() + self.useFixture( + fixtures.MockPatch('placement.attribute_cache.ConsumerTypeCache')) self.useFixture( fixtures.MockPatch('placement.attribute_cache.ResourceClassCache')) self.useFixture( diff --git a/placement/tests/unit/handlers/test_util.py b/placement/tests/unit/handlers/test_util.py index 96f7e5265..56e3b6afe 100644 --- a/placement/tests/unit/handlers/test_util.py +++ b/placement/tests/unit/handlers/test_util.py @@ -25,7 +25,6 @@ from placement import exception from placement.handlers import util from placement import microversion from placement.objects import consumer as consumer_obj -from placement.objects import consumer_type as consumer_type_obj from placement.objects import project as project_obj from placement.objects import user as user_obj from placement.tests.unit import base @@ -58,9 +57,6 @@ class TestEnsureConsumer(base.ContextTestCase): self.mock_consumer_update = self.useFixture(fixtures.MockPatch( 'placement.objects.consumer.' 'Consumer.update')).mock - self.mock_consumer_type_get = self.useFixture(fixtures.MockPatch( - 'placement.objects.consumer_type.' - 'ConsumerType.get_by_name')).mock self.ctx = context.RequestContext(user_id='fake', project_id='fake') self.ctx.config = self.conf self.consumer_id = uuidsentinel.consumer @@ -245,10 +241,6 @@ class TestEnsureConsumer(base.ContextTestCase): self.ctx, id=1, project=proj, user=user, generation=1, consumer_type_id=1) self.mock_consumer_get.return_value = consumer - # Supplied consumer type ID = 2 - consumer_type = consumer_type_obj.ConsumerType( - self.ctx, id=2, name='TYPE') - self.mock_consumer_type_get.return_value = consumer_type consumer_gen = 1 util.ensure_consumer( @@ -256,8 +248,10 @@ class TestEnsureConsumer(base.ContextTestCase): consumer_gen, 'TYPE', self.cons_type_req_version) # Expect 1 call to update() to update to the supplied consumer type ID self.mock_consumer_update.assert_called_once_with() - # Consumer should have the new consumer type ID = 2 - self.assertEqual(2, consumer.consumer_type_id) + # Consumer should have the new consumer type from the cache + self.assertEqual( + self.ctx.ct_cache.id_from_string.return_value, + consumer.consumer_type_id) def test_consumer_create_exists_different_consumer_type_supplied(self): """Tests that we update a consumer's type ID if the one supplied by a @@ -273,10 +267,6 @@ class TestEnsureConsumer(base.ContextTestCase): self.ctx, id=1, project=proj, user=user, generation=1, consumer_type_id=1, uuid=uuidsentinel.consumer) self.mock_consumer_get.return_value = consumer - # Request B supplied consumer type ID = 2 - consumer_type = consumer_type_obj.ConsumerType( - self.ctx, id=2, name='TYPE') - self.mock_consumer_type_get.return_value = consumer_type # Request B will encounter ConsumerExists as Request A just created it self.mock_consumer_create.side_effect = ( exception.ConsumerExists(uuid=uuidsentinel.consumer)) @@ -287,5 +277,7 @@ class TestEnsureConsumer(base.ContextTestCase): consumer_gen, 'TYPE', self.cons_type_req_version) # Expect 1 call to update() to update to the supplied consumer type ID self.mock_consumer_update.assert_called_once_with() - # Consumer should have the new consumer type ID = 2 - self.assertEqual(2, consumer.consumer_type_id) + # Consumer should have the new consumer type from the cache + self.assertEqual( + self.ctx.ct_cache.id_from_string.return_value, + consumer.consumer_type_id)