openstack
/
glance
Code Issues Proposed changes

Resolving the performance issue for image listing of v2 API on server 

This is a fix on Glance server side regarding to the former change
Ia6598d3c06a5ff516277053c2b6fa5db744fe9cf for glanceclient.

This change optimized the logic of image query in glance.db.ImageRepo
which running in glance-api server and communicated with glance-registry
service by HTTP based RPC calls. Under original logic, each image entry
in the (paginated) result set query RPC returned needs to query the
relevant tag entry set by a separated RPC again, so for example if we
have 1000 images, and page size equals 20, then the count of RPC calls
will be 1099 times. With fixed new logic, the RPC times have been
reduced, it merged the queries of image and its tag entry into one RPC
call.

As comparison [0], under the fixed new logic, the execution time of
image listing is about 2.5 times as fast as old logic (using
glanceclient cli with registry backend). Then the execution time of v2
api is better then v1 now[1] (about 2 times faster).

[0]
# Original logic

# glanceclient cli, with registry
$ time glance --os-image-api-version 2 image-list | grep 'test-' | wc -l
1000
real	0m10.210s
user	0m1.244s
sys	0m0.080s

# curl, with registry
$ time ./img1000-curl-v2.sh > /dev/null 2>&1
real	0m9.185s
user	0m0.168s
sys	0m0.108s

# Fixed logic

# glanceclient cli, with registry
$ time glance --os-image-api-version 2 image-list | grep 'test-' | wc -l
1000
real	0m3.870s
user	0m1.364s
sys	0m0.076s

# curl, with registry
$ time ./img1000-curl-v2.sh > /dev/null 2>&1
real	0m2.718s
user	0m0.160s
sys	0m0.136s

[1]
$ time glance --os-image-api-version 1 image-list | grep 'test-' | wc -l
1000
real	0m7.426s
user	0m0.936s
sys	0m0.100s

Change-Id: I3402e4594fa945d003a6735c54581fc43d07c6da
Signed-off-by: Zhi Yan Liu <zhiyanl@cn.ibm.com>
This commit is contained in:
Zhi Yan Liu 2014-07-09 22:25:08 +08:00
parent 424eb6de70
commit 0711daa7c2
5 changed files with 62 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
glance/db/__init__.py
View File

@ -75,12 +75,11 @@ class ImageRepo(object):
db_api_images = self.db_api.image_get_all(
self.context, filters=filters, marker=marker, limit=limit,
sort_key=sort_key, sort_dir=sort_dir,
member_status=member_status)
member_status=member_status, return_tag=True)
images = []
for db_api_image in db_api_images:
tags = self.db_api.image_tag_get_all(self.context,
db_api_image['id'])
image = self._format_image_from_db(dict(db_api_image), tags)
db_image = dict(db_api_image)
image = self._format_image_from_db(db_image, db_image['tags'])
images.append(image)
return images

8
glance/db/registry/api.py
View File

@ -120,7 +120,7 @@ def is_image_visible(context, image, status=None):
def image_get_all(client, filters=None, marker=None, limit=None,
sort_key='created_at', sort_dir='desc',
member_status='accepted', is_public=None,
admin_as_user=False):
admin_as_user=False, return_tag=False):
"""
Get all images that match zero or more filters.
@ -138,12 +138,16 @@ def image_get_all(client, filters=None, marker=None, limit=None,
:param admin_as_user: For backwards compatibility. If true, then return to
an admin the equivalent set of images which it would see
if it were a regular user
:param return_tag: To indicates whether image entry in result includes it
relevant tag entries. This could improve upper-layer
query performance, to prevent using separated calls
"""
return client.image_get_all(filters=filters, marker=marker, limit=limit,
sort_key=sort_key, sort_dir=sort_dir,
member_status=member_status,
is_public=is_public,
admin_as_user=admin_as_user)
admin_as_user=admin_as_user,
return_tag=return_tag)
@_get_client

4
glance/db/simple/api.py
View File

@ -353,7 +353,7 @@ def image_get(context, image_id, session=None, force_show_deleted=False):
def image_get_all(context, filters=None, marker=None, limit=None,
sort_key='created_at', sort_dir='desc',
member_status='accepted', is_public=None,
admin_as_user=False):
admin_as_user=False, return_tag=False):
filters = filters or {}
images = DATA['images'].values()
images = _filter_images(images, filters, context, member_status,
@ -364,6 +364,8 @@ def image_get_all(context, filters=None, marker=None, limit=None,
for image in images:
image['locations'] = _image_location_get_all(image['id'])
if return_tag:
image['tags'] = image_tag_get_all(context, image['id'])
_normalize_locations(image)
return images

27
glance/db/sqlalchemy/api.py
View File

@ -140,6 +140,12 @@ def _normalize_locations(image):
return image
def _normalize_tags(image):
undeleted_tags = filter(lambda x: not x.deleted, image['tags'])
image['tags'] = [tag['value'] for tag in undeleted_tags]
return image
def image_get(context, image_id, session=None, force_show_deleted=False):
image = _image_get(context, image_id, session=session,
force_show_deleted=force_show_deleted)
@ -479,7 +485,7 @@ def _select_images_query(context, image_conditions, admin_as_user,
def image_get_all(context, filters=None, marker=None, limit=None,
sort_key='created_at', sort_dir='desc',
member_status='accepted', is_public=None,
admin_as_user=False):
admin_as_user=False, return_tag=False):
"""
Get all images that match zero or more filters.
@ -497,6 +503,9 @@ def image_get_all(context, filters=None, marker=None, limit=None,
:param admin_as_user: For backwards compatibility. If true, then return to
an admin the equivalent set of images which it would see
if it were a regular user
:param return_tag: To indicates whether image entry in result includes it
relevant tag entries. This could improve upper-layer
query performance, to prevent using separated calls
"""
filters = filters or {}
@ -545,8 +554,17 @@ def image_get_all(context, filters=None, marker=None, limit=None,
query = query.options(sa_orm.joinedload(models.Image.properties))\
.options(sa_orm.joinedload(models.Image.locations))
if return_tag:
query = query.options(sa_orm.joinedload(models.Image.tags))
return [_normalize_locations(image.to_dict()) for image in query.all()]
images = []
for image in query.all():
image_dict = image.to_dict()
image_dict = _normalize_locations(image_dict)
if return_tag:
image_dict = _normalize_tags(image_dict)
images.append(image_dict)
return images
def _drop_protected_attrs(model_class, values):
@ -1026,12 +1044,11 @@ def image_tag_get_all(context, image_id, session=None):
"""Get a list of tags for a specific image."""
_check_image_id(image_id)
session = session or get_session()
tags = session.query(models.ImageTag)\
tags = session.query(models.ImageTag.value)\
.filter_by(image_id=image_id)\
.filter_by(deleted=False)\
.order_by(sqlalchemy.asc(models.ImageTag.created_at))\
.all()
return [tag['value'] for tag in tags]
return [tag[0] for tag in tags]
def user_get_storage_usage(context, owner_id, image_id=None, session=None):

28
glance/tests/functional/db/base.py
View File

@ -733,6 +733,34 @@ class DriverTests(object):
images = self.db_api.image_get_all(self.context, limit=2)
self.assertEqual(2, len(images))
def test_image_get_all_with_tag_returning(self):
expected_tags = {UUID1: ['foo'], UUID2: ['bar'], UUID3: ['baz']}
self.db_api.image_tag_create(self.context, UUID1,
expected_tags[UUID1][0])
self.db_api.image_tag_create(self.context, UUID2,
expected_tags[UUID2][0])
self.db_api.image_tag_create(self.context, UUID3,
expected_tags[UUID3][0])
images = self.db_api.image_get_all(self.context, return_tag=True)
self.assertEqual(3, len(images))
for image in images:
self.assertIn('tags', image)
self.assertEqual(image['tags'], expected_tags[image['id']])
self.db_api.image_tag_delete(self.context, UUID1,
expected_tags[UUID1][0])
expected_tags[UUID1] = []
images = self.db_api.image_get_all(self.context, return_tag=True)
self.assertEqual(3, len(images))
for image in images:
self.assertIn('tags', image)
self.assertEqual(image['tags'], expected_tags[image['id']])
def test_image_destroy(self):
location_data = [{'url': 'a', 'metadata': {'key': 'value'}},
{'url': 'b', 'metadata': {}}]