diff options
author | melanie witt <melwittt@gmail.com> | 2020-10-08 02:41:02 +0000 |
---|---|---|
committer | melanie witt <melwittt@gmail.com> | 2020-11-02 20:48:04 +0000 |
commit | d696a636bbd8c07bd8c609abfcb513453757b1fc (patch) | |
tree | db17462d72897478781471782704b98a4f69efcc | |
parent | 7d2a491695078742c3e11cd22bbeae0a1e4e1591 (diff) | |
download | nova-d696a636bbd8c07bd8c609abfcb513453757b1fc.tar.gz |
[stable-only] Use a separate transaction for reading after race
As part of data migration code to ensure a Project/User record,
integrated placement creates a record if it does not exist. However,
during a race situation, the logic to read back a record written by
a racing request fails because a change written in a separate
transaction will not be reflected in the current transaction when the
isolation level is REPEATABLE_READ (the default):
https://dev.mysql.com/doc/refman/8.0/en/innodb-transaction-isolation-levels.html
This changes the read after catching DBDuplicateEntry to occur in a
separate transaction so it can pick up the change written by the racing
parallel request.
Note: this is a stable-only change because the placement code
containing the bug was temporary data-transitioning code that no longer
exists in placement after Queens.
Closes-Bug: #1731668
Change-Id: Ic90beaaa9848a4f39f4223f31a55cd2c681959cd
-rw-r--r-- | nova/objects/resource_provider.py | 8 | ||||
-rw-r--r-- | nova/tests/functional/db/test_resource_provider.py | 11 |
2 files changed, 14 insertions, 5 deletions
diff --git a/nova/objects/resource_provider.py b/nova/objects/resource_provider.py index 7ce5ec9685..903f90e924 100644 --- a/nova/objects/resource_provider.py +++ b/nova/objects/resource_provider.py @@ -1911,7 +1911,13 @@ def _ensure_lookup_table_entry(ctx, tbl, external_id): except db_exc.DBDuplicateEntry: # Another thread added it just before us, so just read the # internal ID that that thread created... - res = ctx.session.execute(sel).fetchall() + # NOTE(melwitt): We need to use a separate transaction to read + # back the external_id written by another thread. REPEATABLE_READ + # is the default isolation level for InnoDB, so reads from within + # the same transaction will be consistent in such a case. + # https://dev.mysql.com/doc/refman/8.0/en/innodb-transaction-isolation-levels.html + with db_api.api_context_manager.reader.independent.using(ctx): + res = ctx.session.execute(sel).fetchall() return res[0][0] diff --git a/nova/tests/functional/db/test_resource_provider.py b/nova/tests/functional/db/test_resource_provider.py index 460c504e24..0c4bf7ed03 100644 --- a/nova/tests/functional/db/test_resource_provider.py +++ b/nova/tests/functional/db/test_resource_provider.py @@ -19,10 +19,12 @@ from oslo_config import cfg from oslo_db import exception as db_exc from oslo_db.sqlalchemy import enginefacade from oslo_db.sqlalchemy import test_base +from oslo_db.sqlalchemy import test_fixtures as db_fixtures import sqlalchemy as sa import nova from nova import context +from nova.db.sqlalchemy import api as db_api from nova.db.sqlalchemy import api_models from nova.db.sqlalchemy import migration as sa_migration from nova import exception @@ -3044,6 +3046,10 @@ class TestTransactionIsolation(test_base.MySQLOpportunisticTestCase): # to read/write to our test MySQL database. self.test_context_manager = enginefacade.transaction_context() self.test_context_manager.patch_factory(self.enginefacade) + # We need to patch our global API database transaction context manager + # in order to have it read/write to our test MySQL database too. + self.useFixture(db_fixtures.ReplaceEngineFacadeFixture( + db_api.api_context_manager, self.test_context_manager)) self.ctx = context.RequestContext('fake-user', 'fake-project') @@ -3077,9 +3083,6 @@ class TestTransactionIsolation(test_base.MySQLOpportunisticTestCase): # Now run the lookup table method to test the behavior of the scenario. with self.test_context_manager.writer.using(self.ctx): - # FIXME(melwitt): Because of the bug, we expect IndexError to be - # raised when we call _ensure_lookup_table_entry. - self.assertRaises( - IndexError, rp_obj._ensure_lookup_table_entry, + rp_obj._ensure_lookup_table_entry( self.ctx, api_models.Project.__table__, uuidsentinel.external_id) |