zuul/zuul
Code Issues Proposed changes

Don't nest alembic transactions

Browse Source 
SqlAlchemy 2.0 has gotten a lot more picky about transactions. In
addition to needing to explicitly set up transactions using SqlAlchemy
2.0 it seems alembic's get_current_revision() call cannot be in the same
transaction as the alembic migrate/stamp calls with MySQL 8.0.

In particular the get_current_revision call seems to get a
SHARED_HIGH_PRIO lock on the alembic_version table. This prevents the
migrate/stamp calls from creating the alembic_version table as this
requires an EXCLUSIVE lock. The SHARED_HIGH_PRIO lock appears to be in
place as long as the get_current_revision transaction is active. To fix
this we simplify our migration tooling and put get_current_revision in a
transaction block of its own. The rest of our migrate function calls
into functions that will setup new transactions and it doesn't need to
be in this block.

Change-Id: Ic71ddf1968610784cef72c4634ccec3a18855a0e
This commit is contained in:
Clark Boylan 2023-01-31 16:17:51 -08:00
parent 8b0b6310d8
commit cfedda0c2f
1 changed files with 19 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
zuul/driver/sql/sqlconnection.py
View File

@ -308,27 +308,31 @@ class SQLConnection(BaseConnection):
def _migrate(self, revision='head'):
"""Perform the alembic migrations for this connection"""
# Note that this method needs to be called with an external lock held.
# The reason for this is we retrieve the alembic version and run the
# alembic migrations in different database transactions which opens
# us to races without an external lock.
with self.engine.begin() as conn:
context = alembic.migration.MigrationContext.configure(conn)
current_rev = context.get_current_revision()
self.log.debug('Current migration revision: %s' % current_rev)
self.log.debug('Current migration revision: %s' % current_rev)
config = alembic.config.Config()
config.set_main_option("script_location",
"zuul:driver/sql/alembic")
config.set_main_option("sqlalchemy.url",
self.connection_config.get('dburi').
replace('%', '%%'))
config = alembic.config.Config()
config.set_main_option("script_location",
"zuul:driver/sql/alembic")
config.set_main_option("sqlalchemy.url",
self.connection_config.get('dburi').
replace('%', '%%'))
# Alembic lets us add arbitrary data in the tag argument. We can
# leverage that to tell the upgrade scripts about the table prefix.
tag = {'table_prefix': self.table_prefix}
# Alembic lets us add arbitrary data in the tag argument. We can
# leverage that to tell the upgrade scripts about the table prefix.
tag = {'table_prefix': self.table_prefix}
if current_rev is None and not self.force_migrations:
self.metadata.create_all(self.engine)
alembic.command.stamp(config, revision, tag=tag)
else:
alembic.command.upgrade(config, revision, tag=tag)
if current_rev is None and not self.force_migrations:
self.metadata.create_all(self.engine)
alembic.command.stamp(config, revision, tag=tag)
else:
alembic.command.upgrade(config, revision, tag=tag)
def onLoad(self, zk_client, component_registry=None):
safe_connection = quote_plus(self.connection_name)