From 48621685389a491d0519faf8fe52dc682e020c67 Mon Sep 17 00:00:00 2001 From: Boris Pavlovic Date: Fri, 19 Apr 2013 09:41:07 +0400 Subject: Performance optimization for contrib.flavorextraspecs In method FlavorExtraSpecsController.index(): This method calls FlavorExtraSpecsController._get_extra_specs() that returns all extra_specs that has flavor. In db.api layer we are returning already extra_specs as plain object, so there is no reason to make one more time copy of extra_specs. In method FlavorExtraSpecsController.show(): This method should return extra_spec that corresponds to specified key and flavor_id. Was: Get all extra_specs by flavor_id and filter by key locally Now: Filter by flavor_id and key in db. Change-Id: I4c39f5f56511e67e37482cda1b4e9771ee9c7d7d --- nova/api/openstack/compute/contrib/flavorextraspecs.py | 14 ++++++-------- nova/db/api.py | 5 +++++ nova/db/sqlalchemy/api.py | 12 ++++++++++++ .../openstack/compute/contrib/test_flavors_extra_specs.py | 8 ++++++-- nova/tests/test_db_api.py | 10 ++++++++++ 5 files changed, 39 insertions(+), 10 deletions(-) (limited to 'nova') diff --git a/nova/api/openstack/compute/contrib/flavorextraspecs.py b/nova/api/openstack/compute/contrib/flavorextraspecs.py index de68eb7d6..6e33d3603 100644 --- a/nova/api/openstack/compute/contrib/flavorextraspecs.py +++ b/nova/api/openstack/compute/contrib/flavorextraspecs.py @@ -48,10 +48,7 @@ class FlavorExtraSpecsController(object): def _get_extra_specs(self, context, flavor_id): extra_specs = db.instance_type_extra_specs_get(context, flavor_id) - specs_dict = {} - for key, value in extra_specs.iteritems(): - specs_dict[key] = value - return dict(extra_specs=specs_dict) + return dict(extra_specs=extra_specs) def _check_body(self, body): if body is None or body == "": @@ -103,10 +100,11 @@ class FlavorExtraSpecsController(object): """Return a single extra spec item.""" context = req.environ['nova.context'] authorize(context, action='show') - specs = self._get_extra_specs(context, flavor_id) - if id in specs['extra_specs']: - return {id: specs['extra_specs'][id]} - else: + try: + extra_spec = db.instance_type_extra_specs_get_item(context, + flavor_id, id) + return extra_spec + except exception.InstanceTypeExtraSpecsNotFound as e: raise exc.HTTPNotFound() def delete(self, req, flavor_id, id): diff --git a/nova/db/api.py b/nova/db/api.py index f454a2d15..81350ec20 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -1313,6 +1313,11 @@ def instance_type_extra_specs_get(context, flavor_id): return IMPL.instance_type_extra_specs_get(context, flavor_id) +def instance_type_extra_specs_get_item(context, flavor_id, key): + """Get extra specs by key and flavor_id.""" + return IMPL.instance_type_extra_specs_get_item(context, flavor_id, key) + + def instance_type_extra_specs_delete(context, flavor_id, key): """Delete the given extra specs item.""" IMPL.instance_type_extra_specs_delete(context, flavor_id, key) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 55be0da2d..a269dd263 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -3874,6 +3874,18 @@ def instance_type_extra_specs_get(context, flavor_id): return dict([(row['key'], row['value']) for row in rows]) +@require_context +def instance_type_extra_specs_get_item(context, flavor_id, key): + result = _instance_type_extra_specs_get_query(context, flavor_id).\ + filter(models.InstanceTypeExtraSpecs.key == key).\ + first() + if not result: + raise exception.InstanceTypeExtraSpecsNotFound( + extra_specs_key=key, instance_type_id=flavor_id) + + return {result["key"]: result["value"]} + + @require_context def instance_type_extra_specs_delete(context, flavor_id, key): _instance_type_extra_specs_get_query(context, flavor_id).\ diff --git a/nova/tests/api/openstack/compute/contrib/test_flavors_extra_specs.py b/nova/tests/api/openstack/compute/contrib/test_flavors_extra_specs.py index 269937b82..f0f064f4f 100644 --- a/nova/tests/api/openstack/compute/contrib/test_flavors_extra_specs.py +++ b/nova/tests/api/openstack/compute/contrib/test_flavors_extra_specs.py @@ -32,6 +32,10 @@ def return_flavor_extra_specs(context, flavor_id): return stub_flavor_extra_specs() +def return_flavor_extra_specs_item(context, flavor_id, key): + return {key: stub_flavor_extra_specs()[key]} + + def return_empty_flavor_extra_specs(context, flavor_id): return {} @@ -76,8 +80,8 @@ class FlavorsExtraSpecsTest(test.TestCase): self.assertEqual(0, len(res_dict['extra_specs'])) def test_show(self): - self.stubs.Set(nova.db, 'instance_type_extra_specs_get', - return_flavor_extra_specs) + self.stubs.Set(nova.db, 'instance_type_extra_specs_get_item', + return_flavor_extra_specs_item) req = fakes.HTTPRequest.blank('/v2/fake/flavors/1/os-extra_specs' + '/key5') diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py index 5f9a8a038..efdba9b78 100644 --- a/nova/tests/test_db_api.py +++ b/nova/tests/test_db_api.py @@ -2241,6 +2241,16 @@ class InstanceTypeExtraSpecsTestCase(BaseInstanceTypeTestCase): it['flavorid']) self._assertEqualObjects(it['extra_specs'], real_specs) + def test_instance_type_extra_specs_get_item(self): + expected = dict(f1=dict(a='a', b='b', c='c'), + f2=dict(d='d', e='e', f='f')) + + for flavor, specs in expected.iteritems(): + for key, val in specs.iteritems(): + spec = db.instance_type_extra_specs_get_item(self.ctxt, flavor, + key) + self.assertEqual(spec[key], val) + def test_instance_type_extra_specs_delete(self): for it in self.inst_types: specs = it['extra_specs'] -- cgit