From 655ae2e9394dd91e70a52cc504dfab8f4431e2fa Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 30 Jan 2013 16:23:04 -0800 Subject: [PATCH] Optimize floating ip list to make one db query Currently the floating ip code will make 2 db queries for every single associated floating ip when list is called. This adds a couple of joins in the db layer to avoid having to make so many calls. This dramatically improves floating-ip-list. On a simple one node test the time dropped from 2.35 seconds down to 0.5 seconds for a list with 10 associated floating ips. Part of blueprint optimize-nova-network Change-Id: I0571013393b2dbad42c15e690c7783d5ceecaeb2 --- .../openstack/compute/contrib/floating_ips.py | 37 +++++-------------- nova/db/sqlalchemy/api.py | 3 ++ nova/db/sqlalchemy/models.py | 16 +++++++- .../compute/contrib/test_floating_ips.py | 27 +++++--------- 4 files changed, 37 insertions(+), 46 deletions(-) diff --git a/nova/api/openstack/compute/contrib/floating_ips.py b/nova/api/openstack/compute/contrib/floating_ips.py index 81b8c3dc0d04..f7c1100a561e 100644 --- a/nova/api/openstack/compute/contrib/floating_ips.py +++ b/nova/api/openstack/compute/contrib/floating_ips.py @@ -109,33 +109,14 @@ class FloatingIPController(object): self.network_api = network.API() super(FloatingIPController, self).__init__() - def _get_fixed_ip(self, context, fixed_ip_id): - if fixed_ip_id is None: - return None - try: - return self.network_api.get_fixed_ip(context, fixed_ip_id) - except exception.FixedIpNotFound: - return None - - def _get_instance(self, context, instance_id): - return self.compute_api.get(context, instance_id) - - def _set_metadata(self, context, floating_ip): - # When Quantum v2 API is used, 'fixed_ip' and 'instance' are - # already set. In this case we don't need to update the fields. - - if 'fixed_ip' not in floating_ip: - fixed_ip_id = floating_ip['fixed_ip_id'] - floating_ip['fixed_ip'] = self._get_fixed_ip(context, - fixed_ip_id) + def _normalize_ip(self, floating_ip): + # NOTE(vish): translate expects instance to be in the floating_ip + # dict but it is returned in the fixed_ip dict by + # nova-network + fixed_ip = floating_ip.get('fixed_ip') if 'instance' not in floating_ip: - instance_uuid = None - if floating_ip['fixed_ip']: - instance_uuid = floating_ip['fixed_ip']['instance_uuid'] - - if instance_uuid: - floating_ip['instance'] = self._get_instance(context, - instance_uuid) + if fixed_ip: + floating_ip['instance'] = fixed_ip['instance'] else: floating_ip['instance'] = None @@ -151,7 +132,7 @@ class FloatingIPController(object): msg = _("Floating ip not found for id %s") % id raise webob.exc.HTTPNotFound(explanation=msg) - self._set_metadata(context, floating_ip) + self._normalize_ip(floating_ip) return _translate_floating_ip_view(floating_ip) @@ -164,7 +145,7 @@ class FloatingIPController(object): floating_ips = self.network_api.get_floating_ips_by_project(context) for floating_ip in floating_ips: - self._set_metadata(context, floating_ip) + self._normalize_ip(floating_ip) return _translate_floating_ips_view(floating_ips) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 393e1a03c75e..b41dfa625478 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -627,6 +627,7 @@ def certificate_get_all_by_user_and_project(context, user_id, project_id): def floating_ip_get(context, id): result = model_query(context, models.FloatingIp, project_only=True).\ filter_by(id=id).\ + options(joinedload_all('fixed_ip.instance')).\ first() if not result: @@ -841,6 +842,7 @@ def floating_ip_get_all_by_project(context, project_id): return _floating_ip_get_all(context).\ filter_by(project_id=project_id).\ filter_by(auto_assigned=False).\ + options(joinedload_all('fixed_ip.instance')).\ all() @@ -858,6 +860,7 @@ def _floating_ip_get_by_address(context, address, session=None): result = model_query(context, models.FloatingIp, session=session).\ filter_by(address=address).\ + options(joinedload_all('fixed_ip.instance')).\ first() if not result: diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index fd83486780aa..b4c680ac02b8 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -724,7 +724,14 @@ class FixedIp(BASE, NovaBase): foreign_keys=network_id, primaryjoin='and_(' 'FixedIp.network_id == Network.id,' - 'FixedIp.deleted == 0)') + 'FixedIp.deleted == 0,' + 'Network.deleted == 0)') + instance = relationship(Instance, + foreign_keys=instance_uuid, + primaryjoin='and_(' + 'FixedIp.instance_uuid == Instance.uuid,' + 'FixedIp.deleted == 0,' + 'Instance.deleted == 0)') class FloatingIp(BASE, NovaBase): @@ -738,6 +745,13 @@ class FloatingIp(BASE, NovaBase): auto_assigned = Column(Boolean, default=False, nullable=False) pool = Column(String(255)) interface = Column(String(255)) + fixed_ip = relationship(FixedIp, + backref=backref('floating_ips'), + foreign_keys=fixed_ip_id, + primaryjoin='and_(' + 'FloatingIp.fixed_ip_id == FixedIp.id,' + 'FloatingIp.deleted == 0,' + 'FixedIp.deleted == 0)') class DNSDomain(BASE, NovaBase): diff --git a/nova/tests/api/openstack/compute/contrib/test_floating_ips.py b/nova/tests/api/openstack/compute/contrib/test_floating_ips.py index efc9b36cce8c..b6b8dd630f70 100644 --- a/nova/tests/api/openstack/compute/contrib/test_floating_ips.py +++ b/nova/tests/api/openstack/compute/contrib/test_floating_ips.py @@ -36,12 +36,6 @@ from nova.tests import fake_network FAKE_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' -def network_api_get_fixed_ip(self, context, id): - if id is None: - return None - return {'address': '10.0.0.1', 'id': id, 'instance_uuid': 1} - - def network_api_get_floating_ip(self, context, id): return {'id': 1, 'address': '10.10.10.10', 'pool': 'nova', 'fixed_ip_id': None} @@ -56,11 +50,12 @@ def network_api_get_floating_ips_by_project(self, context): return [{'id': 1, 'address': '10.10.10.10', 'pool': 'nova', - 'fixed_ip_id': 20}, + 'fixed_ip': {'address': '10.0.0.1', + 'instance': {'uuid': FAKE_UUID}}}, {'id': 2, 'pool': 'nova', 'interface': 'eth0', 'address': '10.10.10.11', - 'fixed_ip_id': None}] + 'fixed_ip': None}] def compute_api_get(self, context, instance_id): @@ -131,8 +126,6 @@ class FloatingIpTest(test.TestCase): def setUp(self): super(FloatingIpTest, self).setUp() - self.stubs.Set(network.api.API, "get_fixed_ip", - network_api_get_fixed_ip) self.stubs.Set(compute.api.API, "get", compute_api_get) self.stubs.Set(network.api.API, "get_floating_ip", @@ -173,8 +166,9 @@ class FloatingIpTest(test.TestCase): floating_ip_address = self.floating_ip floating_ip = db.floating_ip_get_by_address(self.context, floating_ip_address) - floating_ip['fixed_ip'] = None - floating_ip['instance'] = None + # NOTE(vish): network_get uses the id not the address + floating_ip = db.floating_ip_get(self.context, floating_ip['id']) + self.controller._normalize_ip(floating_ip) view = floating_ips._translate_floating_ip_view(floating_ip) self.assertTrue('floating_ip' in view) self.assertTrue(view['floating_ip']['id']) @@ -185,6 +179,7 @@ class FloatingIpTest(test.TestCase): def test_translate_floating_ip_view_dict(self): floating_ip = {'id': 0, 'address': '10.0.0.10', 'pool': 'nova', 'fixed_ip': None} + self.controller._normalize_ip(floating_ip) view = floating_ips._translate_floating_ip_view(floating_ip) self.assertTrue('floating_ip' in view) @@ -245,19 +240,17 @@ class FloatingIpTest(test.TestCase): def test_show_associated_floating_ip(self): def get_floating_ip(self, context, id): return {'id': 1, 'address': '10.10.10.10', 'pool': 'nova', - 'fixed_ip_id': 11} - - def get_fixed_ip(self, context, id): - return {'address': '10.0.0.1', 'instance_uuid': 1} + 'fixed_ip': {'address': '10.0.0.1', + 'instance': {'uuid': FAKE_UUID}}} self.stubs.Set(network.api.API, "get_floating_ip", get_floating_ip) - self.stubs.Set(network.api.API, "get_fixed_ip", get_fixed_ip) req = fakes.HTTPRequest.blank('/v2/fake/os-floating-ips/1') res_dict = self.controller.show(req, 1) self.assertEqual(res_dict['floating_ip']['id'], 1) self.assertEqual(res_dict['floating_ip']['ip'], '10.10.10.10') + self.assertEqual(res_dict['floating_ip']['fixed_ip'], '10.0.0.1') self.assertEqual(res_dict['floating_ip']['instance_id'], FAKE_UUID) def test_recreation_of_floating_ip(self):