From 1c66a4b029cc9f939f596cd08ae4c20894d3550e Mon Sep 17 00:00:00 2001 From: Yikun Jiang Date: Tue, 31 Jul 2018 18:27:39 +0800 Subject: [PATCH] Remove redundant join in _anchors_for_sharing_providers There is a redundant join when we want to get id from _anchors_for_sharing_providers. The last outerjoin is used to get the rp.UUID according rp.id, if we set get_id=True, we no longer need this outer join. So, we remove the redundant join in this patch. Change-Id: Ib5fc6e4efae29dd88ce92df834700d2121ed8076 Closes-bug: #1784604 --- .../openstack/placement/objects/resource_provider.py | 11 ++++++----- .../openstack/placement/db/test_resource_provider.py | 6 ++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/nova/api/openstack/placement/objects/resource_provider.py b/nova/api/openstack/placement/objects/resource_provider.py index d5eb8436e90b..a31ec76eff42 100644 --- a/nova/api/openstack/placement/objects/resource_provider.py +++ b/nova/api/openstack/placement/objects/resource_provider.py @@ -471,13 +471,14 @@ def _anchors_for_sharing_providers(context, rp_ids, get_id=False): join_chain = sa.join( join_chain, shr_with_sps, shr_with_sps_aggs.c.resource_provider_id == shr_with_sps.c.id) - # TODO(efried): Change this to an inner join when we are sure all - # root_provider_id values are NOT NULL - join_chain = sa.outerjoin( - join_chain, rps, shr_with_sps.c.root_provider_id == rps.c.id) if get_id: - sel = sa.select([sps.c.id, func.coalesce(rps.c.id, shr_with_sps.c.id)]) + sel = sa.select([sps.c.id, func.coalesce( + shr_with_sps.c.root_provider_id, shr_with_sps.c.id)]) else: + # TODO(efried): Change this to an inner join when we are sure all + # root_provider_id values are NOT NULL + join_chain = sa.outerjoin( + join_chain, rps, shr_with_sps.c.root_provider_id == rps.c.id) sel = sa.select([sps.c.uuid, func.coalesce(rps.c.uuid, shr_with_sps.c.uuid)]) sel = sel.select_from(join_chain) diff --git a/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py b/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py index 7b69235bfb28..a93c3f2e13c7 100644 --- a/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py +++ b/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py @@ -1046,6 +1046,12 @@ class TestResourceProviderAggregates(tb.PlacementDbBaseTestCase): self.assertItemsEqual( expected, rp_obj._anchors_for_sharing_providers(self.ctx, [s1.id])) + # Get same result (id format) when we set get_id=True + expected = set([(s1.id, rp.id) for rp in (s1, r1, r2, r3, s5)]) + self.assertItemsEqual( + expected, rp_obj._anchors_for_sharing_providers(self.ctx, [s1.id], + get_id=True)) + # s2 gets s2 (self) and r3 via agg4 expected = set([(s2.uuid, rp.uuid) for rp in (s2, r3)]) self.assertItemsEqual(