openstack
/
nova
Code Issues Proposed changes

rp: streamline InventoryList.get_all_by_rp_uuid() 

Changes the implementation of the
InventoryList.get_all_by_resource_provider_uuid() method to streamline
the generated SQL for the query against the database and move the
@staticmethod out into module scope to be consistent with other DB
retrieval methods.

The get_all_by_resource_provider_uuid() interface is converted to a
get_all_by_resource_provider() interface, accepting a ResourceProvider
object instead of a UUID string. The reason for this is because all
calling locations of InventoryList.get_all_by_resource_provider_uuid()
were already calling ResourceProvider.get_by_uuid() and therefore
already had a ResourceProvider object that could be used to construct a
list of Inventory objects. Passing in this resource provider object
means we can eliminate the joins from inventories to resource_providers
table and the associated ORM costs.

blueprint: de-orm-resource-providers

Change-Id: I895114872a515a269487a683124b63303818e19c
This commit is contained in:
Jay Pipes 2017-10-02 15:26:56 -04:00
parent cb2e9dbb30
commit de5b8a4f2e
4 changed files with 78 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
nova/api/openstack/placement/handlers/inventory.py
View File

@ -286,17 +286,15 @@ def get_inventories(req):
context = req.environ['placement.context']
uuid = util.wsgi_path_item(req.environ, 'uuid')
try:
resource_provider = rp_obj.ResourceProvider.get_by_uuid(
context, uuid)
rp = rp_obj.ResourceProvider.get_by_uuid(context, uuid)
except exception.NotFound as exc:
raise webob.exc.HTTPNotFound(
_("No resource provider with uuid %(uuid)s found : %(error)s") %
{'uuid': uuid, 'error': exc})
inventories = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
context, resource_provider.uuid)
inv_list = rp_obj.InventoryList.get_all_by_resource_provider(context, rp)
return _send_inventories(req.response, resource_provider, inventories)
return _send_inventories(req.response, rp, inv_list)
@wsgi_wrapper.PlacementWsgify
@ -310,18 +308,22 @@ def get_inventory(req):
context = req.environ['placement.context']
uuid = util.wsgi_path_item(req.environ, 'uuid')
resource_class = util.wsgi_path_item(req.environ, 'resource_class')
try:
rp = rp_obj.ResourceProvider.get_by_uuid(context, uuid)
except exception.NotFound as exc:
raise webob.exc.HTTPNotFound(
_("No resource provider with uuid %(uuid)s found : %(error)s") %
{'uuid': uuid, 'error': exc})
resource_provider = rp_obj.ResourceProvider.get_by_uuid(
context, uuid)
inventory = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
context, resource_provider.uuid).find(resource_class)
inv_list = rp_obj.InventoryList.get_all_by_resource_provider(context, rp)
inventory = inv_list.find(resource_class)
if not inventory:
raise webob.exc.HTTPNotFound(
_('No inventory of class %(class)s for %(rp_uuid)s') %
{'class': resource_class, 'rp_uuid': resource_provider.uuid})
{'class': resource_class, 'rp_uuid': uuid})
return _send_inventory(req.response, resource_provider, inventory)
return _send_inventory(req.response, rp, inventory)
@wsgi_wrapper.PlacementWsgify

49
nova/objects/resource_provider.py
View File

@ -1380,6 +1380,24 @@ class Inventory(_HasAResourceProvider):
return int((self.total - self.reserved) * self.allocation_ratio)
@db_api.api_context_manager.reader
def _get_inventory_by_provider_id(ctx, rp_id):
inv = sa.alias(_INV_TBL, name="i")
cols = [
inv.c.resource_class_id,
inv.c.total,
inv.c.reserved,
inv.c.min_unit,
inv.c.max_unit,
inv.c.step_size,
inv.c.allocation_ratio,
]
sel = sa.select(cols)
sel = sel.where(inv.c.resource_provider_id == rp_id)
return [dict(r) for r in ctx.session.execute(sel)]
@base.NovaObjectRegistry.register_if(False)
class InventoryList(base.ObjectListBase, base.NovaObject):
@ -1403,20 +1421,25 @@ class InventoryList(base.ObjectListBase, base.NovaObject):
if inv_rec.resource_class == res_class:
return inv_rec
@staticmethod
@db_api.api_context_manager.reader
def _get_all_by_resource_provider(context, rp_uuid):
return context.session.query(models.Inventory).\
join(models.Inventory.resource_provider).\
options(contains_eager('resource_provider')).\
filter(models.ResourceProvider.uuid == rp_uuid).all()
@classmethod
def get_all_by_resource_provider_uuid(cls, context, rp_uuid):
db_inventory_list = cls._get_all_by_resource_provider(context,
rp_uuid)
return base.obj_make_list(context, cls(context), Inventory,
db_inventory_list)
def get_all_by_resource_provider(cls, context, rp):
_ensure_rc_cache(context)
db_inv = _get_inventory_by_provider_id(context, rp.id)
# Build up a list of Inventory objects, setting the Inventory object
# fields to the same-named database record field we got from
# _get_inventory_by_provider_id(). We already have the ResourceProvider
# object so we just pass that object to the Inventory object
# constructor as-is
objs = [
Inventory(
context, resource_provider=rp,
resource_class=_RC_CACHE.string_from_id(
rec['resource_class_id']),
**rec)
for rec in db_inv
]
inv_list = cls(context, objects=objs)
return inv_list
@base.NovaObjectRegistry.register_if(False)

40
nova/tests/functional/db/test_resource_provider.py
View File

@ -152,12 +152,12 @@ class ResourceProviderTestCase(ResourceProviderBaseCase):
inv_list = rp_obj.InventoryList(context=self.ctx,
objects=[disk_inventory])
resource_provider.set_inventory(inv_list)
inventories = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, resource_provider.uuid)
inventories = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, resource_provider)
self.assertEqual(1, len(inventories))
resource_provider.destroy()
inventories = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, resource_provider.uuid)
inventories = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, resource_provider)
self.assertEqual(0, len(inventories))
def test_set_inventory_unknown_resource_class(self):
@ -324,8 +324,8 @@ class ResourceProviderTestCase(ResourceProviderBaseCase):
self.assertEqual(saved_generation + 1, rp.generation)
saved_generation = rp.generation
new_inv_list = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, uuidsentinel.rp_uuid)
new_inv_list = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, rp)
self.assertEqual(2, len(new_inv_list))
resource_classes = [inv.resource_class for inv in new_inv_list]
self.assertIn(fields.ResourceClass.VCPU, resource_classes)
@ -339,8 +339,8 @@ class ResourceProviderTestCase(ResourceProviderBaseCase):
self.assertEqual(saved_generation + 1, rp.generation)
saved_generation = rp.generation
new_inv_list = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, uuidsentinel.rp_uuid)
new_inv_list = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, rp)
self.assertEqual(1, len(new_inv_list))
resource_classes = [inv.resource_class for inv in new_inv_list]
self.assertNotIn(fields.ResourceClass.VCPU, resource_classes)
@ -363,8 +363,8 @@ class ResourceProviderTestCase(ResourceProviderBaseCase):
self.assertEqual(saved_generation + 1, rp.generation)
saved_generation = rp.generation
new_inv_list = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, uuidsentinel.rp_uuid)
new_inv_list = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, rp)
self.assertEqual(1, len(new_inv_list))
self.assertEqual(2048, new_inv_list[0].total)
@ -391,22 +391,22 @@ class ResourceProviderTestCase(ResourceProviderBaseCase):
self.assertEqual(saved_generation + 1, rp.generation)
saved_generation = rp.generation
new_inv_list = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, uuidsentinel.rp_uuid)
new_inv_list = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, rp)
result = new_inv_list.find(fields.ResourceClass.DISK_GB)
self.assertIsNone(result)
self.assertRaises(exception.NotFound, rp.delete_inventory,
fields.ResourceClass.DISK_GB)
# check inventory list is empty
inv_list = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, uuidsentinel.rp_uuid)
inv_list = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, rp)
self.assertEqual(0, len(inv_list))
# add some inventory
rp.add_inventory(vcpu_inv)
inv_list = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, uuidsentinel.rp_uuid)
inv_list = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, rp)
self.assertEqual(1, len(inv_list))
# generation has bumped
@ -477,8 +477,8 @@ class ResourceProviderTestCase(ResourceProviderBaseCase):
self.ctx, rp.uuid)
self.assertEqual(allocation.used, usages[0].usage)
inv_list = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, rp.uuid)
inv_list = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, rp)
self.assertEqual(new_total, inv_list[0].total)
mock_log.warning.assert_called_once_with(
mock.ANY, {'uuid': rp.uuid, 'resource': 'DISK_GB'})
@ -541,8 +541,8 @@ class ResourceProviderTestCase(ResourceProviderBaseCase):
# Get inventories for the first resource provider and validate
# the inventory records have a matching resource provider
got_inv = rp_obj.InventoryList.get_all_by_resource_provider_uuid(
self.ctx, rp1.uuid)
got_inv = rp_obj.InventoryList.get_all_by_resource_provider(
self.ctx, rp1)
for inv in got_inv:
self.assertEqual(rp1.id, inv.resource_provider.id)

17
nova/tests/unit/objects/test_resource_provider.py
View File

@ -145,22 +145,23 @@ class TestInventoryNoDB(test_objects._LocalTest):
USES_DB = False
@mock.patch('nova.objects.resource_provider._ensure_rc_cache',
side_effect=_fake_ensure_cache)
@mock.patch('nova.objects.resource_provider.InventoryList.'
'_get_all_by_resource_provider')
side_effect=_fake_ensure_cache)
@mock.patch('nova.objects.resource_provider._get_inventory_by_provider_id')
def test_get_all_by_resource_provider(self, mock_get, mock_ensure_cache):
expected = [dict(_INVENTORY_DB,
resource_provider=dict(_RESOURCE_PROVIDER_DB)),
resource_provider_id=_RESOURCE_PROVIDER_ID),
dict(_INVENTORY_DB,
id=_INVENTORY_DB['id'] + 1,
resource_provider=dict(_RESOURCE_PROVIDER_DB))]
resource_provider_id=_RESOURCE_PROVIDER_ID)]
mock_get.return_value = expected
rp_inv_list = resource_provider.InventoryList
objs = rp_inv_list.get_all_by_resource_provider_uuid(
self.context, _RESOURCE_PROVIDER_DB['uuid'])
rp = resource_provider.ResourceProvider(id=_RESOURCE_PROVIDER_ID,
uuid=_RESOURCE_PROVIDER_UUID)
objs = resource_provider.InventoryList.get_all_by_resource_provider(
self.context, rp)
self.assertEqual(2, len(objs))
self.assertEqual(_INVENTORY_DB['id'], objs[0].id)
self.assertEqual(_INVENTORY_DB['id'] + 1, objs[1].id)
self.assertEqual(_RESOURCE_PROVIDER_ID, objs[0].resource_provider.id)
def test_non_negative_handling(self):
# NOTE(cdent): Just checking, useless to be actually