openstack
/
nova
Code Issues Proposed changes

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 b1ed92c7af)
This commit is contained in:
melanie witt 2018-03-21 22:57:50 +00:00
parent 07a1cbb5a1
commit 22b2a8e464
2 changed files with 14 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
nova/objects/instance.py
View File

@ -1235,18 +1235,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

17
nova/tests/functional/regressions/test_bug_1746509.py
View File

@ -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)