From 6443abaac93ec99e1594da15fd21d7876ad46dde Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Thu, 31 Jan 2019 09:03:20 -0600 Subject: [PATCH] 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 --- nova/compute/resource_tracker.py | 6 +++--- nova/tests/unit/compute/test_resource_tracker.py | 14 ++++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index a16eddaa0253..ccb24a34e33e 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -913,6 +913,9 @@ class ResourceTracker(object): return True 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): """Send resource and inventory changes to placement.""" # 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, 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): """Update partial stats locally and populate them to Scheduler.""" if self._resource_change(compute_node): diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index 886fb3586a63..814006a66b90 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -1475,8 +1475,9 @@ class TestUpdateComputeNode(BaseTestCase): exp_inv[rc_fields.ResourceClass.DISK_GB]['reserved'] = 1 self.assertEqual(exp_inv, ptree.data(new_compute.uuid).inventory) - @mock.patch('nova.objects.ComputeNode.save', new=mock.Mock()) - def test_update_retry_success(self): + @mock.patch('nova.compute.resource_tracker.ResourceTracker.' + '_resource_change', return_value=False) + def test_update_retry_success(self, mock_resource_change): self._setup_rt() orig_compute = _COMPUTE_NODE_FIXTURES[0].obj_clone() self.rt.compute_nodes[_NODENAME] = orig_compute @@ -1497,9 +1498,12 @@ class TestUpdateComputeNode(BaseTestCase): self.rt._update(mock.sentinel.ctx, new_compute) 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()) - def test_update_retry_raises(self): + @mock.patch('nova.compute.resource_tracker.ResourceTracker.' + '_resource_change', return_value=False) + def test_update_retry_raises(self, mock_resource_change): self._setup_rt() orig_compute = _COMPUTE_NODE_FIXTURES[0].obj_clone() self.rt.compute_nodes[_NODENAME] = orig_compute @@ -1521,6 +1525,8 @@ class TestUpdateComputeNode(BaseTestCase): self.rt._update, mock.sentinel.ctx, new_compute) 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): """Tests that a ComputeNode object's allocation ratio fields are