From db4d6799f1a220f69aa804754a0913065ff9436a Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Mon, 2 Oct 2017 17:17:45 -0400 Subject: [PATCH] rp: remove ability to delete 1 allocation record The Allocation.destroy() method has been removed, making the Allocation object a plain-old data (POD) object. Allocations may only be deleted as an atomic unit -- all allocation records for a given consumer_id are considered a single thing. We already had a function that batch-deleted allocation records by consumer ID so this patch does little more than remove the singular Allocation.destroy() method and plumb AllocationList.delete_all() to use the existing batch-delete function. blueprint: de-orm-resource-providers Change-Id: I456ac2ffe506eba849a29b36343875c4e1423aed --- nova/objects/resource_provider.py | 30 +++++-------------- .../functional/db/test_resource_provider.py | 14 +-------- 2 files changed, 9 insertions(+), 35 deletions(-) diff --git a/nova/objects/resource_provider.py b/nova/objects/resource_provider.py index 77f3189975d4..2c33d6c9676a 100644 --- a/nova/objects/resource_provider.py +++ b/nova/objects/resource_provider.py @@ -1453,26 +1453,16 @@ class Allocation(_HasAResourceProvider): 'used': fields.IntegerField(), } - @staticmethod - @db_api.api_context_manager.writer - def _destroy(context, id): - result = context.session.query(models.Allocation).filter_by( - id=id).delete() - if not result: - raise exception.NotFound() - def destroy(self): - self._destroy(self._context, self.id) - - -def _delete_current_allocs(conn, consumer_id): +@db_api.api_context_manager.writer +def _delete_allocations_for_consumer(ctx, consumer_id): """Deletes any existing allocations that correspond to the allocations to be written. This is wrapped in a transaction, so if the write subsequently fails, the deletion will also be rolled back. """ del_sql = _ALLOC_TBL.delete().where( _ALLOC_TBL.c.consumer_id == consumer_id) - conn.execute(del_sql) + ctx.session.execute(del_sql) def _check_capacity_exceeded(conn, allocs): @@ -1688,13 +1678,6 @@ class AllocationList(base.ObjectListBase, base.NovaObject): 'user_id': fields.StringField(nullable=True), } - @staticmethod - @db_api.api_context_manager.writer - def _delete_allocations(context, allocations): - for allocation in allocations: - allocation._context = context - allocation.destroy() - @staticmethod @db_api.api_context_manager.reader def _get_allocations_from_db(context, resource_provider_uuid=None, @@ -1780,7 +1763,7 @@ class AllocationList(base.ObjectListBase, base.NovaObject): with conn.begin(): # First delete any existing allocations for that rp/consumer combo. consumer_id = allocs[0].consumer_id - _delete_current_allocs(conn, consumer_id) + _delete_allocations_for_consumer(context, consumer_id) # If there are any allocations with string resource class names # that don't exist this will raise a ResourceClassNotFound # exception. @@ -1827,7 +1810,10 @@ class AllocationList(base.ObjectListBase, base.NovaObject): self._set_allocations(self._context, self.objects) def delete_all(self): - self._delete_allocations(self._context, self.objects) + # Allocations can only have a single consumer, so take advantage of + # that fact and do an efficient batch delete + consumer_uuid = self.objects[0].consumer_id + _delete_allocations_for_consumer(self._context, consumer_uuid) def __repr__(self): strings = [repr(x) for x in self.objects] diff --git a/nova/tests/functional/db/test_resource_provider.py b/nova/tests/functional/db/test_resource_provider.py index 90762c33aee6..658359938d03 100644 --- a/nova/tests/functional/db/test_resource_provider.py +++ b/nova/tests/functional/db/test_resource_provider.py @@ -830,7 +830,7 @@ class TestAllocation(ResourceProviderBaseCase): self.assertEqual(DISK_ALLOCATION['used'], allocations[0].used) - allocations[0].destroy() + allocations.delete_all() allocations = rp_obj.AllocationList.get_all_by_resource_provider_uuid( self.ctx, resource_provider.uuid) @@ -971,18 +971,6 @@ class TestAllocation(ResourceProviderBaseCase): self.assertEqual(2, len(consumer_allocs)) - def test_destroy(self): - rp, allocation = self._make_allocation() - allocations = rp_obj.AllocationList.get_all_by_resource_provider_uuid( - self.ctx, rp.uuid) - self.assertEqual(1, len(allocations)) - rp_obj.Allocation._destroy(self.ctx, allocation.id) - allocations = rp_obj.AllocationList.get_all_by_resource_provider_uuid( - self.ctx, rp.uuid) - self.assertEqual(0, len(allocations)) - self.assertRaises(exception.NotFound, rp_obj.Allocation._destroy, - self.ctx, allocation.id) - def test_get_allocations_from_db(self): rp, allocation = self._make_allocation() allocations = rp_obj.AllocationList._get_allocations_from_db(