From 9a6310ddc4ec2eeef39a51dd5a2fa86ae9c4b972 Mon Sep 17 00:00:00 2001 From: melanie witt Date: Wed, 21 Mar 2018 22:57:50 +0000 Subject: [PATCH] Move _make_instance_list call outside of DB transaction context The _make_instance_list method is used to make an InstanceList object out of database dict-like instance objects. It's possible while making the list that the various _from_db_object methods that are called might do their own database writes. Currently, we're calling _make_instance_list nested inside of a 'reader' database transaction context and we hit the error: TypeError: Can't upgrade a READER transaction to a WRITER mid-transaction during the _make_instance_list call if anything tries to do a database write. The scenario encountered was after an upgrade to Pike, older service records without UUIDs were attempted to be updated with UUIDs upon access, and that access happened to be during an instance list, so it failed when trying to write the service UUID while nested inside the 'reader' database transaction context. This simply moves the _make_instance_list method call out from the @db.select_db_reader_mode decorated _get_by_filters_impl method to the get_by_filters method to remove the nesting. Closes-Bug: #1746509 Change-Id: Ifadf408802cc15eb9769d2dc1fc920426bb7fc20 (cherry picked from commit b1ed92c7af01a9ac7e122a541ce1bdb9be0524c4) (cherry picked from commit 22b2a8e4645e15990a0f130a8866746497c5b5ee) --- nova/objects/instance.py | 12 +++++++++--- .../functional/regressions/test_bug_1746509.py | 17 +++++------------ 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/nova/objects/instance.py b/nova/objects/instance.py index 7cac0f8d2c4d..e175eaf25009 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -1232,18 +1232,24 @@ class InstanceList(base.ObjectListBase, base.NovaObject): db_inst_list = db.instance_get_all_by_filters( context, filters, sort_key, sort_dir, limit=limit, marker=marker, columns_to_join=_expected_cols(expected_attrs)) - return _make_instance_list(context, cls(), db_inst_list, - expected_attrs) + return db_inst_list @base.remotable_classmethod def get_by_filters(cls, context, filters, sort_key='created_at', sort_dir='desc', limit=None, marker=None, expected_attrs=None, use_slave=False, sort_keys=None, sort_dirs=None): - return cls._get_by_filters_impl( + db_inst_list = cls._get_by_filters_impl( context, filters, sort_key=sort_key, sort_dir=sort_dir, limit=limit, marker=marker, expected_attrs=expected_attrs, use_slave=use_slave, sort_keys=sort_keys, sort_dirs=sort_dirs) + # NOTE(melwitt): _make_instance_list could result in joined objects' + # (from expected_attrs) _from_db_object methods being called during + # Instance._from_db_object, each of which might choose to perform + # database writes. So, we call this outside of _get_by_filters_impl to + # avoid being nested inside a 'reader' database transaction context. + return _make_instance_list(context, cls(), db_inst_list, + expected_attrs) @staticmethod @db.select_db_reader_mode diff --git a/nova/tests/functional/regressions/test_bug_1746509.py b/nova/tests/functional/regressions/test_bug_1746509.py index a69161cab73b..e5e86f2660c3 100644 --- a/nova/tests/functional/regressions/test_bug_1746509.py +++ b/nova/tests/functional/regressions/test_bug_1746509.py @@ -10,8 +10,6 @@ # License for the specific language governing permissions and limitations # under the License. -import six - import nova.context from nova import db from nova import objects @@ -57,13 +55,8 @@ class InstanceListWithServicesTestCase(test.TestCase): host='fake-host') inst.create() - # TODO(melwitt): Remove these asserts when the bug is fixed. - ex = self.assertRaises(TypeError, objects.InstanceList.get_by_filters, - self.context, {}, expected_attrs=['services']) - self.assertIn("Can't upgrade a READER transaction to a WRITER " - "mid-transaction", six.text_type(ex)) - - # TODO(melwitt): Uncomment this assert when the bug is fixed. - # insts = objects.InstanceList.get_by_filters( - # self.context, {}, expected_attrs=['services']) - # self.assertEqual(1, len(insts)) + insts = objects.InstanceList.get_by_filters( + self.context, {}, expected_attrs=['services']) + self.assertEqual(1, len(insts)) + self.assertEqual(1, len(insts[0].services)) + self.assertIsNotNone(insts[0].services[0].uuid)