openstack/congress
Code Issues Proposed changes

Critical newton fix: Use new db session for table locking

Browse Source 
Sharing the same db session means if another db operation takes
place during the transaction before an unlock takes place
(synchronizer or another request being served), MySQL complains and
fails to unlock table (because MySQL requires that all tables used
by the session holding a lock must also be locked). This causing all
subsequent db operations to fail.

By using a separate, one-time facade and session for add_policy,
this problem is resolved.

Change-Id: If1493f3bf5e4ae3be5c568c99901667e3aac27dd
Closes-bug: #1629988
This commit is contained in:
Eric K 2016-09-30 19:38:42 -07:00 committed by Tim Hinrichs
parent 6550c5eaf5
commit fcccfb24dd
2 changed files with 13 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
congress/db/api.py
View File

@ -38,8 +38,11 @@ def get_engine():
return facade.get_engine()
def get_session(autocommit=True, expire_on_commit=False):
def get_session(autocommit=True, expire_on_commit=False, make_new=False):
"""Helper method to grab session."""
facade = _create_facade_lazily()
if make_new: # do not reuse existing facade
facade = session.EngineFacade.from_config(cfg.CONF, sqlite_fk=True)
else:
facade = _create_facade_lazily()
return facade.get_session(autocommit=autocommit,
expire_on_commit=expire_on_commit)

9
congress/db/db_policy_rules.py
View File

@ -19,12 +19,15 @@ from __future__ import absolute_import
from oslo_config import cfg
from oslo_log import log as logging
import sqlalchemy as sa
from sqlalchemy.orm import exc as db_exc
from congress.db import api as db
from congress.db import model_base
LOG = logging.getLogger(__name__)
class Policy(model_base.BASE, model_base.HasId, model_base.HasAudit):
__tablename__ = 'policies'
@ -59,13 +62,13 @@ class Policy(model_base.BASE, model_base.HasId, model_base.HasAudit):
def add_policy(id_, name, abbreviation, description, owner, kind,
deleted=False, session=None):
session = session or db.get_session()
mysql = (cfg.CONF.database.connection is not None and
(cfg.CONF.database.connection.split(':/')[0] == 'mysql' or
cfg.CONF.database.connection.split('+')[0] == 'mysql'))
postgres = (cfg.CONF.database.connection is not None and
(cfg.CONF.database.connection.split(':/')[0] == 'postgresql' or
cfg.CONF.database.connection.split('+')[0] == 'postgresql'))
session = session or db.get_session(make_new=(mysql or postgres))
try:
with session.begin(subtransactions=False):
# Note(ekcs): disable subtransactions to prevent interrupting
@ -79,7 +82,9 @@ def add_policy(id_, name, abbreviation, description, owner, kind,
# TODO(ekcs): table locking is special to underlying DB
# change DB schema to prevent duplicate generically without locking
if mysql: # Explicitly LOCK TABLES for MySQL
LOG.debug('locking table `policies`')
session.execute('LOCK TABLES policies WRITE')
LOG.debug('success locking table `policies`')
if postgres: # Explicitly LOCK TABLE for Postgres
session.execute('LOCK TABLE policies IN EXCLUSIVE MODE')
# Do nothing for SQLite; DB auto locked for transaction
@ -95,7 +100,9 @@ def add_policy(id_, name, abbreviation, description, owner, kind,
session.add(policy)
finally: # always release table lock
if mysql:
LOG.debug('unlocking table `policies`')
session.execute('UNLOCK TABLES')
LOG.debug('success unlocking table `policies`')
# postgres automatically releases lock after transaction completes
if not unique:
raise KeyError("Policy with name %s already exists" % name)