openstack/cinder
Code Issues Proposed changes

Merge "DB: Optimize volume_update method"

Browse Source
This commit is contained in:
Jenkins 2016-11-11 17:13:02 +00:00 committed by Gerrit Code Review
commit b15a6b3e6f
6 changed files with 30 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
cinder/db/sqlalchemy/api.py
View File

@ -2173,10 +2173,10 @@ def volume_update(context, volume_id, values):
delete=True, delete=True,
session=session) session=session)
volume_ref = _volume_get(context, volume_id, session=session) query = _volume_get_query(context, session, joined_load=False)
volume_ref.update(values) result = query.filter_by(id=volume_id).update(values)
if not result:
return volume_ref raise exception.VolumeNotFound(volume_id=volume_id)
@handle_db_data_error @handle_db_data_error

9
cinder/tests/unit/api/contrib/test_admin_actions.py
View File

@ -569,8 +569,8 @@ class AdminActionsTest(BaseAdminTest):
expected_status = 400 expected_status = 400
host = 'test2' host = 'test2'
volume = self._migrate_volume_prep() volume = self._migrate_volume_prep()
model_update = {'migration_status': 'migrating'} volume.migration_status = 'migrating'
volume = db.volume_update(self.ctx, volume['id'], model_update) volume.save()
self._migrate_volume_exec(self.ctx, volume, host, expected_status) self._migrate_volume_exec(self.ctx, volume, host, expected_status)
def test_migrate_volume_with_snap(self): def test_migrate_volume_with_snap(self):
@ -1050,8 +1050,7 @@ class AdminActionsAttachDetachTest(BaseAdminTest):
volume = self._create_volume(self.ctx, {'provider_location': '', volume = self._create_volume(self.ctx, {'provider_location': '',
'size': 1}) 'size': 1})
values = {'status': 'attaching', values = {'status': 'attaching'}
'instance_uuid': fake.INSTANCE_ID}
db.volume_update(self.ctx, volume['id'], values) db.volume_update(self.ctx, volume['id'], values)
db.volume_admin_metadata_update(self.ctx, volume['id'], db.volume_admin_metadata_update(self.ctx, volume['id'],
{"attached_mode": 'rw'}, False) {"attached_mode": 'rw'}, False)
@ -1060,7 +1059,7 @@ class AdminActionsAttachDetachTest(BaseAdminTest):
self.volume_api.attach, self.volume_api.attach,
self.ctx, self.ctx,
volume, volume,
values['instance_uuid'], fake.INSTANCE_ID,
None, None,
mountpoint, mountpoint,
'ro') 'ro')

10
cinder/tests/unit/db/test_name_id.py
View File

@ -43,18 +43,16 @@ class NameIDsTestCase(test.TestCase):
def test_name_id_diff(self): def test_name_id_diff(self):
"""Change name ID to mimic volume after migration.""" """Change name ID to mimic volume after migration."""
vol_ref = testutils.create_volume(self.ctxt, size=1) vol_ref = testutils.create_volume(self.ctxt, size=1,
db.volume_update(self.ctxt, vol_ref['id'], _name_id=fake.VOLUME2_ID)
{'name_id': fake.VOLUME2_ID})
vol_ref = db.volume_get(self.ctxt, vol_ref['id']) vol_ref = db.volume_get(self.ctxt, vol_ref['id'])
expected_name = CONF.volume_name_template % fake.VOLUME2_ID expected_name = CONF.volume_name_template % fake.VOLUME2_ID
self.assertEqual(expected_name, vol_ref['name']) self.assertEqual(expected_name, vol_ref['name'])
def test_name_id_snapshot_volume_name(self): def test_name_id_snapshot_volume_name(self):
"""Make sure snapshot['volume_name'] is updated.""" """Make sure snapshot['volume_name'] is updated."""
vol_ref = testutils.create_volume(self.ctxt, size=1) vol_ref = testutils.create_volume(self.ctxt, size=1,
db.volume_update(self.ctxt, vol_ref['id'], _name_id=fake.VOLUME2_ID)
{'name_id': fake.VOLUME2_ID})
snap_ref = testutils.create_snapshot(self.ctxt, vol_ref['id']) snap_ref = testutils.create_snapshot(self.ctxt, vol_ref['id'])
expected_name = CONF.volume_name_template % fake.VOLUME2_ID expected_name = CONF.volume_name_template % fake.VOLUME2_ID
self.assertEqual(expected_name, snap_ref['volume_name']) self.assertEqual(expected_name, snap_ref['volume_name'])

20
cinder/tests/unit/test_db_api.py
View File

@ -942,17 +942,15 @@ class DBAPIVolumeTestCase(BaseTest):
def test_volume_update(self): def test_volume_update(self):
volume = db.volume_create(self.ctxt, {'host': 'h1'}) volume = db.volume_create(self.ctxt, {'host': 'h1'})
ref_a = db.volume_update(self.ctxt, volume['id'], db.volume_update(self.ctxt, volume.id,
{'host': 'h2', {'host': 'h2',
'metadata': {'m1': 'v1'}}) 'metadata': {'m1': 'v1'}})
volume = db.volume_get(self.ctxt, volume['id']) volume = db.volume_get(self.ctxt, volume.id)
self.assertEqual('h2', volume['host']) self.assertEqual('h2', volume.host)
expected = dict(ref_a) self.assertEqual(1, len(volume.volume_metadata))
expected['volume_metadata'] = list(map(dict, db_metadata = volume.volume_metadata[0]
expected['volume_metadata'])) self.assertEqual('m1', db_metadata.key)
result = dict(volume) self.assertEqual('v1', db_metadata.value)
result['volume_metadata'] = list(map(dict, result['volume_metadata']))
self.assertEqual(expected, result)
def test_volume_update_nonexistent(self): def test_volume_update_nonexistent(self):
self.assertRaises(exception.VolumeNotFound, db.volume_update, self.assertRaises(exception.VolumeNotFound, db.volume_update,

32
cinder/tests/unit/test_volume.py
View File

@ -5426,16 +5426,13 @@ class VolumeMigrationTestCase(base.BaseVolumeTestCase):
def _test_delete_volume_in_migration(self, migration_status): def _test_delete_volume_in_migration(self, migration_status):
"""Test deleting a volume that is in migration.""" """Test deleting a volume that is in migration."""
volume = tests_utils.create_volume(self.context, **self.volume_params) volume = tests_utils.create_volume(self.context, host=CONF.host,
volume.status = 'available' migration_status=migration_status)
volume.migration_status = migration_status self.volume.delete_volume(self.context, volume=volume)
volume.save()
self.volume.delete_volume(self.context, volume)
# The volume is successfully removed during the volume delete # The volume is successfully removed during the volume delete
# and won't exist in the database any more. # and won't exist in the database any more.
self.assertRaises(exception.VolumeNotFound, db.volume_get, self.assertRaises(exception.VolumeNotFound, volume.refresh)
self.context, volume.id)
class ReplicationTestCase(base.BaseVolumeTestCase): class ReplicationTestCase(base.BaseVolumeTestCase):
@ -5614,24 +5611,9 @@ class CopyVolumeToImageTestCase(base.BaseVolumeTestCase):
'45b1161abb02' '45b1161abb02'
db.volume_create(self.context, self.volume_attrs) db.volume_create(self.context, self.volume_attrs)
# Storing unmocked db api function reference here, because we have to method = 'volume_update_status_based_on_attachment'
# update volume status (set instance_uuid to None) before calling the with mock.patch.object(db, method,
# 'volume_update_status_based_on_attached_instance_id' db api. wraps=getattr(db, method)) as mock_update:
unmocked_db_api = db.volume_update_status_based_on_attachment
def mock_volume_update_after_upload(context, volume_id):
# First update volume and set 'instance_uuid' to None
# because after deleting instance, instance_uuid of volume is
# set to None
db.volume_update(context, volume_id, {'instance_uuid': None})
# Calling unmocked db api
unmocked_db_api(context, volume_id)
with mock.patch.object(
db,
'volume_update_status_based_on_attachment',
side_effect=mock_volume_update_after_upload) as mock_update:
# Start test # Start test
self.volume.copy_volume_to_image(self.context, self.volume.copy_volume_to_image(self.context,
self.volume_id, self.volume_id,

4
cinder/volume/driver.py
View File

@ -938,8 +938,8 @@ class BaseVD(object):
LOG.debug("Volume %s: creating export", volume['id']) LOG.debug("Volume %s: creating export", volume['id'])
model_update = self.create_export(context, volume, properties) model_update = self.create_export(context, volume, properties)
if model_update: if model_update:
volume = self.db.volume_update(context, volume['id'], volume.update(model_update)
model_update) volume.save()
except exception.CinderException as ex: except exception.CinderException as ex:
if model_update: if model_update:
LOG.exception(_LE("Failed updating model of volume " LOG.exception(_LE("Failed updating model of volume "