From d52bcc616f2d1c0df7bad8a6f5a00adc7ad1fb27 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Thu, 6 Apr 2017 17:59:34 -0400 Subject: [PATCH] Fix joins in instance_get_all_by_host Some callers of instance_get_all_by_host are passing in columns_to_join=[], like the _sync_scheduler_instance_info periodic task in the compute manager, to avoid unnecessary joins with other tables. The problem was columns_to_join wasn't being passed through to _instance_get_all_query which builds the actual query method, and defaults to join on info_cache and security_groups. This fixes the problem by passing through columns_to_join and provides tests to show it working both with and without the joins. Change-Id: I69f2ddca8fb0935e03b0f426891d01360940a85a Closes-Bug: #1680616 --- nova/db/sqlalchemy/api.py | 5 +++-- nova/tests/unit/db/test_db_api.py | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 75ff2051b68b..38683e31e3ef 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -2526,9 +2526,10 @@ def _instance_get_all_query(context, project_only=False, joins=None): @pick_context_manager_reader_allow_async def instance_get_all_by_host(context, host, columns_to_join=None): + query = _instance_get_all_query(context, joins=columns_to_join) return _instances_fill_metadata(context, - _instance_get_all_query(context).filter_by(host=host).all(), - manual_joins=columns_to_join) + query.filter_by(host=host).all(), + manual_joins=columns_to_join) def _instance_get_all_uuids_by_host(context, host): diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 45781cef2c08..b355917fcf1e 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -1182,6 +1182,28 @@ class SqlAlchemyDbApiTestCase(DbTestCase): result = test(ctxt) self.assertEqual(2, len(result)) + # make sure info_cache and security_groups were auto-joined + instance = result[0] + self.assertIn('info_cache', instance) + self.assertIn('security_groups', instance) + + def test_instance_get_all_by_host_no_joins(self): + """Tests that we don't join on the info_cache and security_groups + tables if columns_to_join is an empty list. + """ + self.create_instance_with_args() + + @sqlalchemy_api.pick_context_manager_reader + def test(ctxt): + return sqlalchemy_api.instance_get_all_by_host( + ctxt, 'host1', columns_to_join=[]) + + result = test(context.get_admin_context()) + self.assertEqual(1, len(result)) + # make sure info_cache and security_groups were not auto-joined + instance = result[0] + self.assertNotIn('info_cache', instance) + self.assertNotIn('security_groups', instance) def test_instance_get_all_uuids_by_host(self): ctxt = context.get_admin_context()