Fix race condition in reshaper handler
Fix the race condition identified at [1] by indexing the `inventories` argument to the resource_provider.reshape method by ResourceProvider object rather than its UUID. That means we never have to do a fresh lookup for the provider. [1] https://review.openstack.org/#/c/585033/18/nova/api/openstack/placement/objects/resource_provider.py@4195 Change-Id: Ie21bddc186364db2b10af45546c35b9ad5a0208b
This commit is contained in:
parent
d13e5a5011
commit
d98986415c
|
@ -80,7 +80,8 @@ def reshape(req):
|
||||||
inv_obj = inventory.make_inventory_object(
|
inv_obj = inventory.make_inventory_object(
|
||||||
resource_provider, res_class, **inv_data)
|
resource_provider, res_class, **inv_data)
|
||||||
inv_list.append(inv_obj)
|
inv_list.append(inv_obj)
|
||||||
inventory_by_rp[rp_uuid] = rp_obj.InventoryList(objects=inv_list)
|
inventory_by_rp[resource_provider] = rp_obj.InventoryList(
|
||||||
|
objects=inv_list)
|
||||||
|
|
||||||
# Make the consumer objects associated with the allocations.
|
# Make the consumer objects associated with the allocations.
|
||||||
consumers, new_consumers_created = allocation.inspect_consumers(
|
consumers, new_consumers_created = allocation.inspect_consumers(
|
||||||
|
|
|
@ -4147,9 +4147,9 @@ def reshape(ctx, inventories, allocations):
|
||||||
|
|
||||||
:param ctx: `nova.api.openstack.placement.context.RequestContext` object
|
:param ctx: `nova.api.openstack.placement.context.RequestContext` object
|
||||||
containing the DB transaction context.
|
containing the DB transaction context.
|
||||||
:param inventories: dict, keyed by resource provider UUID, of
|
:param inventories: dict, keyed by ResourceProvider, of `InventoryList`
|
||||||
`InventoryList` objects representing the replaced
|
objects representing the replaced inventory information
|
||||||
inventory information for the provider.
|
for the provider.
|
||||||
:param allocations: `AllocationList` object containing all allocations for
|
:param allocations: `AllocationList` object containing all allocations for
|
||||||
all consumers being modified by the reshape operation.
|
all consumers being modified by the reshape operation.
|
||||||
:raises: `exception.ConcurrentUpdateDetected` when any resource provider or
|
:raises: `exception.ConcurrentUpdateDetected` when any resource provider or
|
||||||
|
@ -4177,25 +4177,12 @@ def reshape(ctx, inventories, allocations):
|
||||||
# allocated, but that's allowed by the code. If the final picture is
|
# allocated, but that's allowed by the code. If the final picture is
|
||||||
# overcommitted, we'll get an appropriate exception when we replace the
|
# overcommitted, we'll get an appropriate exception when we replace the
|
||||||
# allocations at the end.
|
# allocations at the end.
|
||||||
for rp_uuid, new_inv_list in inventories.items():
|
for rp, new_inv_list in inventories.items():
|
||||||
LOG.debug("reshaping: *interim* inventory replacement for provider %s",
|
LOG.debug("reshaping: *interim* inventory replacement for provider %s",
|
||||||
rp_uuid)
|
rp.uuid)
|
||||||
# We need the resource provider object.
|
|
||||||
if new_inv_list:
|
|
||||||
# If the inventory list is nonempty, grab it from the first entry.
|
|
||||||
# This overrides (and will replace, below) an object that was
|
|
||||||
# prepopulated from the allocations.
|
|
||||||
rp = new_inv_list[0].resource_provider
|
|
||||||
elif rp_uuid in affected_providers:
|
|
||||||
# No inventory object; second choice is from an allocation object.
|
|
||||||
rp = affected_providers[rp_uuid]
|
|
||||||
else:
|
|
||||||
# No inventory or allocations for this provider - which makes sense
|
|
||||||
# when we're "clearing out" a provider. Look it up.
|
|
||||||
rp = ResourceProvider.get_by_uuid(ctx, rp_uuid)
|
|
||||||
# Update the cache. This may be replacing an entry that came from
|
# Update the cache. This may be replacing an entry that came from
|
||||||
# allocations, or adding a new entry from inventories and/or db lookup.
|
# allocations, or adding a new entry from inventories.
|
||||||
affected_providers[rp_uuid] = rp
|
affected_providers[rp.uuid] = rp
|
||||||
|
|
||||||
# Optimization: If the new inventory is empty, the below would be
|
# Optimization: If the new inventory is empty, the below would be
|
||||||
# replacing it with itself (and incrementing the generation)
|
# replacing it with itself (and incrementing the generation)
|
||||||
|
@ -4229,7 +4216,7 @@ def reshape(ctx, inventories, allocations):
|
||||||
allocations.replace_all()
|
allocations.replace_all()
|
||||||
|
|
||||||
# And finally, we can set the inventories to their actual desired state.
|
# And finally, we can set the inventories to their actual desired state.
|
||||||
for rp_uuid, new_inv_list in inventories.items():
|
for rp, new_inv_list in inventories.items():
|
||||||
LOG.debug("reshaping: *final* inventory replacement for provider %s",
|
LOG.debug("reshaping: *final* inventory replacement for provider %s",
|
||||||
rp_uuid)
|
rp.uuid)
|
||||||
affected_providers[rp_uuid].set_inventory(new_inv_list)
|
rp.set_inventory(new_inv_list)
|
||||||
|
|
|
@ -104,7 +104,7 @@ class ReshapeTestCase(tb.PlacementDbBaseTestCase):
|
||||||
# storage provider.
|
# storage provider.
|
||||||
after_inventories = {
|
after_inventories = {
|
||||||
# cn1 keeps the RAM only
|
# cn1 keeps the RAM only
|
||||||
cn1.uuid: rp_obj.InventoryList(self.ctx, objects=[
|
cn1: rp_obj.InventoryList(self.ctx, objects=[
|
||||||
rp_obj.Inventory(
|
rp_obj.Inventory(
|
||||||
self.ctx, resource_provider=cn1,
|
self.ctx, resource_provider=cn1,
|
||||||
resource_class='MEMORY_MB', total=32768, reserved=0,
|
resource_class='MEMORY_MB', total=32768, reserved=0,
|
||||||
|
@ -112,14 +112,14 @@ class ReshapeTestCase(tb.PlacementDbBaseTestCase):
|
||||||
allocation_ratio=1.0),
|
allocation_ratio=1.0),
|
||||||
]),
|
]),
|
||||||
# each NUMA node gets half of the CPUs
|
# each NUMA node gets half of the CPUs
|
||||||
cn1_numa0.uuid: rp_obj.InventoryList(self.ctx, objects=[
|
cn1_numa0: rp_obj.InventoryList(self.ctx, objects=[
|
||||||
rp_obj.Inventory(
|
rp_obj.Inventory(
|
||||||
self.ctx, resource_provider=cn1_numa0,
|
self.ctx, resource_provider=cn1_numa0,
|
||||||
resource_class='VCPU', total=8, reserved=0,
|
resource_class='VCPU', total=8, reserved=0,
|
||||||
max_unit=8, min_unit=1, step_size=1,
|
max_unit=8, min_unit=1, step_size=1,
|
||||||
allocation_ratio=1.0),
|
allocation_ratio=1.0),
|
||||||
]),
|
]),
|
||||||
cn1_numa1.uuid: rp_obj.InventoryList(self.ctx, objects=[
|
cn1_numa1: rp_obj.InventoryList(self.ctx, objects=[
|
||||||
rp_obj.Inventory(
|
rp_obj.Inventory(
|
||||||
self.ctx, resource_provider=cn1_numa1,
|
self.ctx, resource_provider=cn1_numa1,
|
||||||
resource_class='VCPU', total=8, reserved=0,
|
resource_class='VCPU', total=8, reserved=0,
|
||||||
|
@ -127,7 +127,7 @@ class ReshapeTestCase(tb.PlacementDbBaseTestCase):
|
||||||
allocation_ratio=1.0),
|
allocation_ratio=1.0),
|
||||||
]),
|
]),
|
||||||
# The sharing provider gets a bunch of disk
|
# The sharing provider gets a bunch of disk
|
||||||
ss.uuid: rp_obj.InventoryList(self.ctx, objects=[
|
ss: rp_obj.InventoryList(self.ctx, objects=[
|
||||||
rp_obj.Inventory(
|
rp_obj.Inventory(
|
||||||
self.ctx, resource_provider=ss,
|
self.ctx, resource_provider=ss,
|
||||||
resource_class='DISK_GB', total=100000, reserved=0,
|
resource_class='DISK_GB', total=100000, reserved=0,
|
||||||
|
@ -283,7 +283,7 @@ class ReshapeTestCase(tb.PlacementDbBaseTestCase):
|
||||||
# storage provider.
|
# storage provider.
|
||||||
after_inventories = {
|
after_inventories = {
|
||||||
# cn1 keeps the RAM only
|
# cn1 keeps the RAM only
|
||||||
cn1.uuid: rp_obj.InventoryList(self.ctx, objects=[
|
cn1: rp_obj.InventoryList(self.ctx, objects=[
|
||||||
rp_obj.Inventory(
|
rp_obj.Inventory(
|
||||||
self.ctx, resource_provider=cn1,
|
self.ctx, resource_provider=cn1,
|
||||||
resource_class='MEMORY_MB', total=32768, reserved=0,
|
resource_class='MEMORY_MB', total=32768, reserved=0,
|
||||||
|
@ -291,14 +291,14 @@ class ReshapeTestCase(tb.PlacementDbBaseTestCase):
|
||||||
allocation_ratio=1.0),
|
allocation_ratio=1.0),
|
||||||
]),
|
]),
|
||||||
# each NUMA node gets half of the CPUs
|
# each NUMA node gets half of the CPUs
|
||||||
cn1_numa0.uuid: rp_obj.InventoryList(self.ctx, objects=[
|
cn1_numa0: rp_obj.InventoryList(self.ctx, objects=[
|
||||||
rp_obj.Inventory(
|
rp_obj.Inventory(
|
||||||
self.ctx, resource_provider=cn1_numa0,
|
self.ctx, resource_provider=cn1_numa0,
|
||||||
resource_class='VCPU', total=8, reserved=0,
|
resource_class='VCPU', total=8, reserved=0,
|
||||||
max_unit=8, min_unit=1, step_size=1,
|
max_unit=8, min_unit=1, step_size=1,
|
||||||
allocation_ratio=1.0),
|
allocation_ratio=1.0),
|
||||||
]),
|
]),
|
||||||
cn1_numa1.uuid: rp_obj.InventoryList(self.ctx, objects=[
|
cn1_numa1: rp_obj.InventoryList(self.ctx, objects=[
|
||||||
rp_obj.Inventory(
|
rp_obj.Inventory(
|
||||||
self.ctx, resource_provider=cn1_numa1,
|
self.ctx, resource_provider=cn1_numa1,
|
||||||
resource_class='VCPU', total=8, reserved=0,
|
resource_class='VCPU', total=8, reserved=0,
|
||||||
|
@ -306,7 +306,7 @@ class ReshapeTestCase(tb.PlacementDbBaseTestCase):
|
||||||
allocation_ratio=1.0),
|
allocation_ratio=1.0),
|
||||||
]),
|
]),
|
||||||
# The sharing provider gets a bunch of disk
|
# The sharing provider gets a bunch of disk
|
||||||
ss.uuid: rp_obj.InventoryList(self.ctx, objects=[
|
ss: rp_obj.InventoryList(self.ctx, objects=[
|
||||||
rp_obj.Inventory(
|
rp_obj.Inventory(
|
||||||
self.ctx, resource_provider=ss,
|
self.ctx, resource_provider=ss,
|
||||||
resource_class='DISK_GB', total=100000, reserved=0,
|
resource_class='DISK_GB', total=100000, reserved=0,
|
||||||
|
|
Loading…
Reference in New Issue