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

Author: Eric Fried

nova/compute/resource_tracker.py

@@ -913,6 +913,9 @@ def _resource_change(self, compute_node):
             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 @@ def _update_to_placement(self, context, compute_node, startup):
         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):

nova/tests/unit/compute/test_resource_tracker.py

@@ -1475,8 +1475,9 @@ def fake_upt(ptree, nodename, allocations=None):
         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 @@ def test_update_retry_success(self):
         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 @@ def test_update_retry_raises(self):
                           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