openstack/cinder
Code Issues Proposed changes

Improve performance listing detail for volumes

Browse Source 
When large numbers of volumes are created from images in cloud,
and user uses pagination querying, there is no need to query all
volumes' volume_glance_metadata, just get those volumes which are in
response list.

Implement a new method "volume_api.get_list_volumes_image_metadata"
to query image metadata in list of volumes by using one sql querying.

Change-Id: I42307d51133bab58a166a781fccf797f0608843e
Closes-Bug: #1476150
This commit is contained in:
wanghao 2015-07-20 17:09:19 +08:00
parent 7fb767f2d6
commit d31fe66983
7 changed files with 126 additions and 18 deletions

41
cinder/api/contrib/volume_image_metadata.py

@ -57,41 +57,46 @@ class VolumeImageMetadataController(wsgi.Controller):
all_metadata = {}
return all_metadata
def _add_image_metadata(self, context, resp_volume, image_meta=None):
"""Appends the image metadata to the given volume.
def _add_image_metadata(self, context, resp_volume_list, image_metas=None):
"""Appends the image metadata to each of the given volume.
:param context: the request context
:param resp_volume: the response volume
:param image_meta: The image metadata to append, if None is provided it
will be retrieved from the database. An empty dict
means there is no metadata and it should not be
retrieved from the db.
:param resp_volume_list: the response volume list
:param image_metas: The image metadata to append, if None is provided
it will be retrieved from the database. An empty
dict means there is no metadata and it should not
be retrieved from the db.
"""
if image_meta is None:
vol_id_list = []
for vol in resp_volume_list:
vol_id_list.append(vol['id'])
if image_metas is None:
try:
image_meta = self.volume_api.get_volume_image_metadata(
context, resp_volume)
except Exception:
image_metas = self.volume_api.get_list_volumes_image_metadata(
context, vol_id_list)
except Exception as e:
LOG.debug('Get image metadata error: %s', e)
return
if image_meta:
resp_volume['volume_image_metadata'] = dict(image_meta)
if image_metas:
for vol in resp_volume_list:
image_meta = image_metas.get(vol['id'], {})
vol['volume_image_metadata'] = dict(image_meta)
@wsgi.extends
def show(self, req, resp_obj, id):
context = req.environ['cinder.context']
if authorize(context):
resp_obj.attach(xml=VolumeImageMetadataTemplate())
self._add_image_metadata(context, resp_obj.obj['volume'])
self._add_image_metadata(context, [resp_obj.obj['volume']])
@wsgi.extends
def detail(self, req, resp_obj):
context = req.environ['cinder.context']
if authorize(context):
resp_obj.attach(xml=VolumesImageMetadataTemplate())
all_meta = self._get_all_images_metadata(context)
for vol in list(resp_obj.obj.get('volumes', [])):
image_meta = all_meta.get(vol['id'], {})
self._add_image_metadata(context, vol, image_meta)
# Just get the image metadata of those volumes in response.
self._add_image_metadata(context,
list(resp_obj.obj.get('volumes', [])))
@wsgi.action("os-set_image_metadata")
@wsgi.serializers(xml=common.MetadataTemplate)

5
cinder/db/api.py

@ -666,6 +666,11 @@ def volume_glance_metadata_get(context, volume_id):
return IMPL.volume_glance_metadata_get(context, volume_id)
def volume_glance_metadata_list_get(context, volume_id_list):
"""Return the glance metadata for a volume list."""
return IMPL.volume_glance_metadata_list_get(context, volume_id_list)
def volume_snapshot_glance_metadata_get(context, snapshot_id):
"""Return the Glance metadata for the specified snapshot."""
return IMPL.volume_snapshot_glance_metadata_get(context, snapshot_id)

11
cinder/db/sqlalchemy/api.py

@ -3256,6 +3256,17 @@ def volume_glance_metadata_get_all(context):
return _volume_glance_metadata_get_all(context)
@require_context
def volume_glance_metadata_list_get(context, volume_id_list):
"""Return the glance metadata for a volume list."""
query = model_query(context,
models.VolumeGlanceMetadata,
session=None)
query = query.filter(
models.VolumeGlanceMetadata.volume_id.in_(volume_id_list))
return query.all()
@require_context
@require_volume_exists
def _volume_glance_metadata_get(context, volume_id, session=None):

27
cinder/tests/unit/api/contrib/test_volume_image_metadata.py

@ -23,6 +23,7 @@ import webob
from cinder.api import common
from cinder.api.contrib import volume_image_metadata
from cinder.api.openstack import wsgi
from cinder import context
from cinder import db
from cinder import exception
from cinder import test
@ -115,18 +116,44 @@ class VolumeImageMetadataTest(test.TestCase):
for volume in json.loads(body)['volumes']
]
def _create_volume_and_glance_metadata(self):
ctxt = context.get_admin_context()
db.volume_create(ctxt, {'id': 'fake', 'status': 'available',
'host': 'test', 'provider_location': '',
'size': 1})
db.volume_glance_metadata_create(ctxt, 'fake', 'image_id', 'someid')
db.volume_glance_metadata_create(ctxt, 'fake', 'image_name', 'fake')
db.volume_glance_metadata_create(ctxt, 'fake', 'kernel_id',
'somekernel')
db.volume_glance_metadata_create(ctxt, 'fake', 'ramdisk_id',
'someramdisk')
def test_get_volume(self):
self._create_volume_and_glance_metadata()
res = self._make_request('/v2/fake/volumes/%s' % self.UUID)
self.assertEqual(200, res.status_int)
self.assertEqual(fake_image_metadata,
self._get_image_metadata(res.body))
def test_list_detail_volumes(self):
self._create_volume_and_glance_metadata()
res = self._make_request('/v2/fake/volumes/detail')
self.assertEqual(200, res.status_int)
self.assertEqual(fake_image_metadata,
self._get_image_metadata_list(res.body)[0])
def test_list_detail_volumes_with_limit(self):
ctxt = context.get_admin_context()
db.volume_create(ctxt, {'id': 'fake', 'status': 'available',
'host': 'test', 'provider_location': '',
'size': 1})
db.volume_glance_metadata_create(ctxt, 'fake', 'key1', 'value1')
db.volume_glance_metadata_create(ctxt, 'fake', 'key2', 'value2')
res = self._make_request('/v2/fake/volumes/detail?limit=1')
self.assertEqual(200, res.status_int)
self.assertEqual({'key1': 'value1', 'key2': 'value2'},
self._get_image_metadata_list(res.body)[0])
def test_create_image_metadata(self):
self.stubs.Set(volume.API, 'get_volume_image_metadata',
return_empty_image_metadata)

31
cinder/tests/unit/test_db_api.py

@ -1031,6 +1031,37 @@ class DBAPIVolumeTestCase(BaseTest):
image_name = meta_entry.value
self.assertEqual(u'\xe4\xbd\xa0\xe5\xa5\xbd', image_name)
def test_volume_glance_metadata_list_get(self):
"""Test volume_glance_metadata_list_get in DB API."""
db.volume_create(self.ctxt, {'id': 'fake1', 'status': 'available',
'host': 'test', 'provider_location': '',
'size': 1})
db.volume_glance_metadata_create(self.ctxt, 'fake1', 'key1', 'value1')
db.volume_glance_metadata_create(self.ctxt, 'fake1', 'key2', 'value2')
db.volume_create(self.ctxt, {'id': 'fake2', 'status': 'available',
'host': 'test', 'provider_location': '',
'size': 1})
db.volume_glance_metadata_create(self.ctxt, 'fake2', 'key3', 'value3')
db.volume_glance_metadata_create(self.ctxt, 'fake2', 'key4', 'value4')
expect_result = [{'volume_id': 'fake1', 'key': 'key1',
'value': 'value1'},
{'volume_id': 'fake1', 'key': 'key2',
'value': 'value2'},
{'volume_id': 'fake2', 'key': 'key3',
'value': 'value3'},
{'volume_id': 'fake2', 'key': 'key4',
'value': 'value4'}]
self._assertEqualListsOfObjects(expect_result,
db.volume_glance_metadata_list_get(
self.ctxt, ['fake1', 'fake2']),
ignored_keys=['id',
'snapshot_id',
'created_at',
'deleted', 'deleted_at',
'updated_at'])
class DBAPISnapshotTestCase(BaseTest):

20
cinder/tests/unit/test_volume.py

@ -3886,6 +3886,26 @@ class VolumeTestCase(BaseVolumeTestCase):
snapshot_id)
self.assertEqual('test update name', snap.display_name)
def test_volume_api_get_list_volumes_image_metadata(self):
"""Test get_list_volumes_image_metadata in volume API."""
ctxt = context.get_admin_context()
db.volume_create(ctxt, {'id': 'fake1', 'status': 'available',
'host': 'test', 'provider_location': '',
'size': 1})
db.volume_glance_metadata_create(ctxt, 'fake1', 'key1', 'value1')
db.volume_glance_metadata_create(ctxt, 'fake1', 'key2', 'value2')
db.volume_create(ctxt, {'id': 'fake2', 'status': 'available',
'host': 'test', 'provider_location': '',
'size': 1})
db.volume_glance_metadata_create(ctxt, 'fake2', 'key3', 'value3')
db.volume_glance_metadata_create(ctxt, 'fake2', 'key4', 'value4')
volume_api = cinder.volume.api.API()
results = volume_api.get_list_volumes_image_metadata(ctxt, ['fake1',
'fake2'])
expect_results = {'fake1': {'key1': 'value1', 'key2': 'value2'},
'fake2': {'key3': 'value3', 'key4': 'value4'}}
self.assertEqual(expect_results, results)
@mock.patch.object(QUOTAS, 'reserve')
def test_extend_volume(self, reserve):
"""Test volume can be extended at API level."""

9
cinder/volume/api.py

@ -1182,6 +1182,15 @@ class API(base.Base):
resource=volume)
return {meta_entry.key: meta_entry.value for meta_entry in db_data}
def get_list_volumes_image_metadata(self, context, volume_id_list):
db_data = self.db.volume_glance_metadata_list_get(context,
volume_id_list)
results = collections.defaultdict(dict)
for meta_entry in db_data:
results[meta_entry['volume_id']].update({meta_entry['key']:
meta_entry['value']})
return results
def _check_volume_availability(self, volume, force):
"""Check if the volume can be used."""
if volume['status'] not in ['available', 'in-use']: