openstack/nova
Code Issues Proposed changes

Save all instance extras in a single db call

Browse Source 
Currently, changes to instance fields backed by instance_extras table
trigger separate db calls for each individual change when calling
save() on the instance. This reworks that so that all changes gets
saved with a single call to the db.

Change-Id: I2ddcb625ca54f18b1a68f8ddd1f28e541f438a74
This commit is contained in:
Hans Lindgren 2016-04-13 19:22:15 +02:00
parent 9d99081da2
commit e75e9184f3
7 changed files with 37 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
nova/objects/instance.py
View File

@ -536,15 +536,6 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
# NOTE(danms): I don't think we need to worry about this, do we?
pass
def _save_numa_topology(self, context):
if self.numa_topology:
self.numa_topology.instance_uuid = self.uuid
with self.numa_topology.obj_alternate_context(context):
self.numa_topology._save()
else:
objects.InstanceNUMATopology.delete_by_instance_uuid(
context, self.uuid)
def _save_pci_requests(self, context):
# NOTE(danms): No need for this yet.
pass
@ -559,8 +550,6 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
if not any([x in self.obj_what_changed() for x in
('flavor', 'old_flavor', 'new_flavor')]):
return
# FIXME(danms): We can do this smarterly by updating this
# with all the other extra things at the same time
flavor_info = {
'cur': self.flavor.obj_to_primitive(),
'old': (self.old_flavor and
@ -568,9 +557,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
'new': (self.new_flavor and
self.new_flavor.obj_to_primitive() or None),
}
db.instance_extra_update_by_uuid(
context, self.uuid,
{'flavor': jsonutils.dumps(flavor_info)})
self._extra_values_to_save['flavor'] = jsonutils.dumps(flavor_info)
self.obj_reset_changes(['flavor', 'old_flavor', 'new_flavor'])
def _save_old_flavor(self, context):
@ -581,34 +568,22 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
if 'new_flavor' in self.obj_what_changed():
self._save_flavor(context)
def _save_vcpu_model(self, context):
# TODO(yjiang5): should merge the db accesses for all the extra
# fields
if 'vcpu_model' in self.obj_what_changed():
if self.vcpu_model:
update = jsonutils.dumps(self.vcpu_model.obj_to_primitive())
else:
update = None
db.instance_extra_update_by_uuid(
context, self.uuid,
{'vcpu_model': update})
def _save_ec2_ids(self, context):
# NOTE(hanlind): Read-only so no need to save this.
pass
def _save_migration_context(self, context):
if self.migration_context:
self.migration_context.instance_uuid = self.uuid
with self.migration_context.obj_alternate_context(context):
self.migration_context._save()
else:
objects.MigrationContext._destroy(context, self.uuid)
def _save_keypairs(self, context):
# NOTE(danms): Read-only so no need to save this.
pass
def _save_extra_generic(self, field):
if field in self.obj_what_changed():
obj = getattr(self, field)
value = None
if obj is not None:
value = jsonutils.dumps(obj.obj_to_primitive())
self._extra_values_to_save[field] = value
@base.remotable
def save(self, expected_vm_state=None,
expected_task_state=None, admin_state_reset=False):
@ -658,6 +633,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
expected_task_state,
admin_state_reset)
self._extra_values_to_save = {}
updates = {}
changes = self.obj_what_changed()
@ -669,6 +645,9 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
try:
getattr(self, '_save_%s' % field)(context)
except AttributeError:
if field in _INSTANCE_EXTRA_FIELDS:
self._save_extra_generic(field)
continue
LOG.exception(_LE('No save handler for %s'), field,
instance=self)
except db_exc.DBReferenceError as exp:
@ -688,6 +667,10 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
else:
updates[field] = self[field]
if self._extra_values_to_save:
db.instance_extra_update_by_uuid(context, self.uuid,
self._extra_values_to_save)
if not updates:
if cells_update_from_api:
_handle_cell_update_from_api()
@ -925,7 +908,8 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
@base.remotable
def drop_migration_context(self):
if self.migration_context:
objects.MigrationContext._destroy(self._context, self.uuid)
db.instance_extra_update_by_uuid(self._context, self.uuid,
{'migration_context': None})
self.migration_context = None
def clear_numa_topology(self):

15
nova/objects/instance_numa_topology.py
View File

@ -178,26 +178,11 @@ class InstanceNUMATopology(base.NovaObject,
# TODO(ndipanov) Remove this method on the major version bump to 2.0
@base.remotable
def create(self):
self._save()
# NOTE(ndipanov): We can't rename create and want to avoid version bump
# as this needs to be backported to stable so this is not a @remotable
# That's OK since we only call it from inside Instance.save() which is.
def _save(self):
values = {'numa_topology': self._to_json()}
db.instance_extra_update_by_uuid(self._context, self.instance_uuid,
values)
self.obj_reset_changes()
# NOTE(ndipanov): We want to avoid version bump
# as this needs to be backported to stable so this is not a @remotable
# That's OK since we only call it from inside Instance.save() which is.
@classmethod
def delete_by_instance_uuid(cls, context, instance_uuid):
values = {'numa_topology': None}
db.instance_extra_update_by_uuid(context, instance_uuid,
values)
@base.remotable_classmethod
def get_by_instance_uuid(cls, context, instance_uuid):
db_extra = db.instance_extra_get_by_instance_uuid(

14
nova/objects/migration_context.py
View File

@ -49,20 +49,6 @@ class MigrationContext(base.NovaPersistentObject, base.NovaObject):
primitive = jsonutils.loads(db_obj)
return cls.obj_from_primitive(primitive)
def _save(self):
primitive = self.obj_to_primitive()
payload = jsonutils.dumps(primitive)
values = {'migration_context': payload}
db.instance_extra_update_by_uuid(self._context, self.instance_uuid,
values)
self.obj_reset_changes()
@classmethod
def _destroy(cls, context, instance_uuid):
values = {'migration_context': None}
db.instance_extra_update_by_uuid(context, instance_uuid, values)
@base.remotable_classmethod
def get_by_instance_uuid(cls, context, instance_uuid):
db_extra = db.instance_extra_get_by_instance_uuid(

5
nova/tests/unit/compute/test_tracker.py
View File

@ -1729,11 +1729,12 @@ class TestMoveClaim(BaseTestCase):
self.assertNotEqual(expected, self.rt.compute_node)
claim.instance.migration_context = mig_context_obj
with mock.patch('nova.objects.MigrationContext._destroy') as destroy_m:
with mock.patch.object(claim.instance,
'drop_migration_context') as drop_mig_ctxt:
claim.abort()
self.assertTrue(obj_base.obj_equal_prims(expected,
self.rt.compute_node))
destroy_m.assert_called_once_with(self.ctx, claim.instance.uuid)
drop_mig_ctxt.assert_called_once_with()
def test_revert_reserve_source(
self, pci_mock, inst_list_mock, inst_by_uuid, migr_mock,

15
nova/tests/unit/objects/test_instance.py
View File

@ -590,6 +590,21 @@ class _TestInstanceObject(object):
inst.save()
self.assertFalse(mock_upd.called)
@mock.patch('nova.db.instance_extra_update_by_uuid')
def test_save_multiple_extras_updates_once(self, mock_update):
inst = fake_instance.fake_instance_obj(self.context)
inst.numa_topology = None
inst.migration_context = None
inst.vcpu_model = test_vcpu_model.fake_vcpumodel
inst.save()
json_vcpu_model = jsonutils.dumps(
test_vcpu_model.fake_vcpumodel.obj_to_primitive())
expected_vals = {'numa_topology': None,
'migration_context': None,
'vcpu_model': json_vcpu_model}
mock_update.assert_called_once_with(self.context, inst.uuid,
expected_vals)
@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')

7
nova/tests/unit/objects/test_instance_numa_topology.py
View File

@ -62,13 +62,6 @@ class _TestInstanceNUMATopology(object):
topo_obj.create()
self.assertEqual(1, len(mock_update.call_args_list))
@mock.patch('nova.db.instance_extra_update_by_uuid')
def test_save(self, mock_update):
topo_obj = get_fake_obj_numa_topology(self.context)
topo_obj.instance_uuid = fake_db_topology['instance_uuid']
topo_obj._save()
self.assertEqual(1, len(mock_update.call_args_list))
def _test_get_by_instance_uuid(self):
numa_topology = objects.InstanceNUMATopology.get_by_instance_uuid(
self.context, fake_db_topology['instance_uuid'])

21
nova/tests/unit/objects/test_migration_context.py
View File

@ -48,27 +48,6 @@ def get_fake_migration_context_obj(ctxt):
class _TestMigrationContext(object):
@mock.patch('nova.db.instance_extra_update_by_uuid')
def test_create(self, mock_update):
ctxt_obj = get_fake_migration_context_obj(self.context)
ctxt_obj._save()
self.assertEqual(1, len(mock_update.call_args_list))
update_call = mock_update.call_args
self.assertEqual(self.context, update_call[0][0])
self.assertEqual(fake_instance_uuid, update_call[0][1])
self.assertIsInstance(ctxt_obj.new_numa_topology,
objects.InstanceNUMATopology)
self.assertIsNone(ctxt_obj.old_numa_topology)
@mock.patch('nova.db.instance_extra_update_by_uuid')
def test_destroy(self, mock_update):
objects.MigrationContext._destroy(self.context, fake_instance_uuid)
self.assertEqual(1, len(mock_update.call_args_list))
update_call = mock_update.call_args
self.assertEqual(self.context, update_call[0][0])
self.assertEqual(fake_instance_uuid, update_call[0][1])
self.assertEqual({'migration_context': None}, update_call[0][2])
def _test_get_by_instance_uuid(self, db_data):
mig_context = objects.MigrationContext.get_by_instance_uuid(
self.context, fake_db_context['instance_uuid'])