From b1c0ea337e4b467052ed1c3c3ec354f2ea8c62bc Mon Sep 17 00:00:00 2001 From: Corey Wright Date: Fri, 13 Nov 2015 16:21:36 -0600 Subject: [PATCH] Prevent redundant instance.update notifications Use _sync_cells within Instance.save() to prevent redundant notifications by nova-cells in api cells from updates made in compute cells. Commit eaaa6593 used the Instance.skip_cells_sync() contextmanager with Instance.save() in instance_update_at_top() to prevent update cycles between api and compute cells, but the notification in Instance.save() is not guarded by _sync_cells, so those notifications generated in compute cells are also generated in api cells. Change-Id: I16dca3c3691833931dba77cd700da64ee23f583c Co-Authored-By: Philip Schwartz Closes-Bug: #1516174 --- nova/objects/instance.py | 20 +++++++++++++++----- nova/tests/unit/objects/test_instance.py | 5 ++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/nova/objects/instance.py b/nova/objects/instance.py index b010b7fe07..33218f76de 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -670,12 +670,22 @@ class Instance(base.NovaPersistentObject, base.NovaObject, cells_api = cells_rpcapi.CellsAPI() cells_api.instance_update_at_top(context, stale_instance) - # NOTE(danms): We have to be super careful here not to trigger - # any lazy-loads that will unmigrate or unbackport something. So, - # make a copy of the instance for notifications first. - new_ref = self.obj_clone() + def _notify(): + # NOTE(danms): We have to be super careful here not to trigger + # any lazy-loads that will unmigrate or unbackport something. So, + # make a copy of the instance for notifications first. + new_ref = self.obj_clone() + + notifications.send_update(context, old_ref, new_ref) + + # NOTE(alaski): If cell synchronization is blocked it means we have + # already run this block of code in either the parent or child of this + # cell. Therefore this notification has already been sent. + if not self._sync_cells: + _notify = lambda: None # noqa: F811 + + _notify() - notifications.send_update(context, old_ref, new_ref) self.obj_reset_changes() @base.remotable diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py index eb65e8ca9c..122efb8d3c 100644 --- a/nova/tests/unit/objects/test_instance.py +++ b/nova/tests/unit/objects/test_instance.py @@ -522,11 +522,12 @@ class _TestInstanceObject(object): inst.save() self.assertFalse(mock_upd.called) + @mock.patch.object(notifications, 'send_update') @mock.patch.object(cells_rpcapi.CellsAPI, 'instance_update_from_api') @mock.patch.object(cells_rpcapi.CellsAPI, 'instance_update_at_top') @mock.patch.object(db, 'instance_update_and_get_original') def _test_skip_cells_sync_helper(self, mock_db_update, mock_update_at_top, - mock_update_from_api, cell_type): + mock_update_from_api, mock_notif_update, cell_type): self.flags(enable=True, cell_type=cell_type, group='cells') inst = fake_instance.fake_instance_obj(self.context, cell_name='fake') inst.vm_state = 'foo' @@ -544,6 +545,7 @@ class _TestInstanceObject(object): mock_update_at_top.assert_has_calls([]) mock_update_from_api.assert_has_calls([]) + self.assertFalse(mock_notif_update.called) inst.vm_state = 'bar' inst.task_state = 'foo' @@ -561,6 +563,7 @@ class _TestInstanceObject(object): inst.save() self.assertEqual('foo!bar@baz', inst.cell_name) + self.assertTrue(mock_notif_update.called) if cell_type == 'compute': mock_update_at_top.assert_called_once_with(self.context, mock.ANY) # Compare primitives since we can't check instance object equality