Move retry from _update to _update_to_placement
It was noted [1] that we don't need to wrap all of _update in a retry on ResourceProviderUpdateConflict; only _update_to_placement can raise that exception, and that exception only indicates a need to redrive with data ret-GETted from within that method. So this patch moves the retry decorator further in, to _update_to_placement, to avoid the additional redundant compute node and pci tracker writes. [1] https://review.openstack.org/#/c/615705/20/nova/compute/resource_tracker.py@974 Change-Id: I85e567181cdeefde0a9ef91cd460cd200e79bdba
This commit is contained in:
parent
f4ec394e86
commit
6443abaac9
|
@ -913,6 +913,9 @@ class ResourceTracker(object):
|
||||||
return True
|
return True
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
@retrying.retry(stop_max_attempt_number=4,
|
||||||
|
retry_on_exception=lambda e: isinstance(
|
||||||
|
e, exception.ResourceProviderUpdateConflict))
|
||||||
def _update_to_placement(self, context, compute_node, startup):
|
def _update_to_placement(self, context, compute_node, startup):
|
||||||
"""Send resource and inventory changes to placement."""
|
"""Send resource and inventory changes to placement."""
|
||||||
# NOTE(jianghuaw): Some resources(e.g. VGPU) are not saved in the
|
# NOTE(jianghuaw): Some resources(e.g. VGPU) are not saved in the
|
||||||
|
@ -969,9 +972,6 @@ class ResourceTracker(object):
|
||||||
self.reportclient.update_from_provider_tree(context, prov_tree,
|
self.reportclient.update_from_provider_tree(context, prov_tree,
|
||||||
allocations=allocs)
|
allocations=allocs)
|
||||||
|
|
||||||
@retrying.retry(stop_max_attempt_number=4,
|
|
||||||
retry_on_exception=lambda e: isinstance(
|
|
||||||
e, exception.ResourceProviderUpdateConflict))
|
|
||||||
def _update(self, context, compute_node, startup=False):
|
def _update(self, context, compute_node, startup=False):
|
||||||
"""Update partial stats locally and populate them to Scheduler."""
|
"""Update partial stats locally and populate them to Scheduler."""
|
||||||
if self._resource_change(compute_node):
|
if self._resource_change(compute_node):
|
||||||
|
|
|
@ -1475,8 +1475,9 @@ class TestUpdateComputeNode(BaseTestCase):
|
||||||
exp_inv[rc_fields.ResourceClass.DISK_GB]['reserved'] = 1
|
exp_inv[rc_fields.ResourceClass.DISK_GB]['reserved'] = 1
|
||||||
self.assertEqual(exp_inv, ptree.data(new_compute.uuid).inventory)
|
self.assertEqual(exp_inv, ptree.data(new_compute.uuid).inventory)
|
||||||
|
|
||||||
@mock.patch('nova.objects.ComputeNode.save', new=mock.Mock())
|
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||||
def test_update_retry_success(self):
|
'_resource_change', return_value=False)
|
||||||
|
def test_update_retry_success(self, mock_resource_change):
|
||||||
self._setup_rt()
|
self._setup_rt()
|
||||||
orig_compute = _COMPUTE_NODE_FIXTURES[0].obj_clone()
|
orig_compute = _COMPUTE_NODE_FIXTURES[0].obj_clone()
|
||||||
self.rt.compute_nodes[_NODENAME] = orig_compute
|
self.rt.compute_nodes[_NODENAME] = orig_compute
|
||||||
|
@ -1497,9 +1498,12 @@ class TestUpdateComputeNode(BaseTestCase):
|
||||||
self.rt._update(mock.sentinel.ctx, new_compute)
|
self.rt._update(mock.sentinel.ctx, new_compute)
|
||||||
|
|
||||||
self.assertEqual(2, ufpt_mock.call_count)
|
self.assertEqual(2, ufpt_mock.call_count)
|
||||||
|
# The retry is restricted to _update_to_placement
|
||||||
|
self.assertEqual(1, mock_resource_change.call_count)
|
||||||
|
|
||||||
@mock.patch('nova.objects.ComputeNode.save', new=mock.Mock())
|
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||||
def test_update_retry_raises(self):
|
'_resource_change', return_value=False)
|
||||||
|
def test_update_retry_raises(self, mock_resource_change):
|
||||||
self._setup_rt()
|
self._setup_rt()
|
||||||
orig_compute = _COMPUTE_NODE_FIXTURES[0].obj_clone()
|
orig_compute = _COMPUTE_NODE_FIXTURES[0].obj_clone()
|
||||||
self.rt.compute_nodes[_NODENAME] = orig_compute
|
self.rt.compute_nodes[_NODENAME] = orig_compute
|
||||||
|
@ -1521,6 +1525,8 @@ class TestUpdateComputeNode(BaseTestCase):
|
||||||
self.rt._update, mock.sentinel.ctx, new_compute)
|
self.rt._update, mock.sentinel.ctx, new_compute)
|
||||||
|
|
||||||
self.assertEqual(4, ufpt_mock.call_count)
|
self.assertEqual(4, ufpt_mock.call_count)
|
||||||
|
# The retry is restricted to _update_to_placement
|
||||||
|
self.assertEqual(1, mock_resource_change.call_count)
|
||||||
|
|
||||||
def test_copy_resources_no_update_allocation_ratios(self):
|
def test_copy_resources_no_update_allocation_ratios(self):
|
||||||
"""Tests that a ComputeNode object's allocation ratio fields are
|
"""Tests that a ComputeNode object's allocation ratio fields are
|
||||||
|
|
Loading…
Reference in New Issue