From 8c374b1e7f7d0de12651a8985097286411bd9d65 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 12 Jun 2017 18:13:47 -0400 Subject: [PATCH] Use oslo_db for create_engine The use of plain SQLAlchemy create_engine in Zaqar bypasses lots of oslo_db features that are associated with its version of create_engine, some useful and some critical. Among favorable behaviors that are added: * The SQLite PRAGMA FOREIGN KEYS call is abstracted into the sqlite_fk flag * The engine is given a pessimistic "ping" event, which emits a "SELECT 1" upon connection checkout which will then recycle the connection if the backing database is no longer connected. As we are using a connection pool, if the database has been restarted, or in the case of MySQL a default timeout of eight hours idle has passed, existing pooled connections will be stale. Upcoming SQLAlchemy 1.2 includes this feature as a simple flag but for now, oslo_db implements the event listener as directed by SQLA docs * The errors raised by the DBAPI, and then wrapped by SQLAlchemy, are further sub-classified. In particular, Zaqar seems to use a lot of IntegrityError catches to detect duplicate entry and foreign key constraint conditions. oslo_db parses these out on a per-database-basis into individual DBDuplicateEntry, DBReferenceError, and other error classes, allowing Zaqar to respond specifically to the sub-class of IntegrityError or allow it to propagate if the condition is unhandled. * checks for MySQL SQL_MODE and best practice driver (pymysql) will emit warnings if these conditions are not met. * Gets Zaqar ready for further oslo.db integration and guides new features into oslo_db, including a potential "mysql timezone" setting Change-Id: I16c3ed89e006e132bbd0295be1dfd0b561b2037c Resolves-bug: https://bugs.launchpad.net/tripleo/+bug/1691951 --- requirements.txt | 1 + zaqar/storage/sqlalchemy/catalogue.py | 5 ++++- zaqar/storage/sqlalchemy/driver.py | 20 +++++++------------- zaqar/storage/sqlalchemy/flavors.py | 3 ++- zaqar/storage/sqlalchemy/pools.py | 5 +++-- zaqar/storage/sqlalchemy/queues.py | 3 ++- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/requirements.txt b/requirements.txt index 124164ae0..7613bb556 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,6 +17,7 @@ six>=1.9.0 # MIT oslo.cache>=1.5.0 # Apache-2.0 oslo.config>=4.0.0 # Apache-2.0 oslo.context>=2.14.0 # Apache-2.0 +oslo.db>=4.21.1 # Apache-2.0 oslo.i18n!=3.15.2,>=2.1.0 # Apache-2.0 oslo.log>=3.22.0 # Apache-2.0 oslo.messaging>=5.19.0 # Apache-2.0 diff --git a/zaqar/storage/sqlalchemy/catalogue.py b/zaqar/storage/sqlalchemy/catalogue.py index 4a92d2ef5..09bc8f500 100644 --- a/zaqar/storage/sqlalchemy/catalogue.py +++ b/zaqar/storage/sqlalchemy/catalogue.py @@ -22,6 +22,7 @@ project: string queue: string """ +import oslo_db.exception import sqlalchemy as sa from zaqar.storage import base @@ -70,7 +71,9 @@ class CatalogueController(base.CatalogueBase): ) self.driver.run(stmt) - except sa.exc.IntegrityError: + except oslo_db.exception.DBReferenceError: + self._update(project, queue, pool) + except oslo_db.exception.DBDuplicateError: self._update(project, queue, pool) def delete(self, project, queue): diff --git a/zaqar/storage/sqlalchemy/driver.py b/zaqar/storage/sqlalchemy/driver.py index fed11e784..394734e80 100644 --- a/zaqar/storage/sqlalchemy/driver.py +++ b/zaqar/storage/sqlalchemy/driver.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations under # the License. +from oslo_db.sqlalchemy import engines from osprofiler import profiler from osprofiler import sqlalchemy as sa_tracer import sqlalchemy as sa @@ -31,11 +32,6 @@ class ControlDriver(storage.ControlDriverBase): group=options.MANAGEMENT_SQLALCHEMY_GROUP) self.sqlalchemy_conf = self.conf[options.MANAGEMENT_SQLALCHEMY_GROUP] - def _sqlite_on_connect(self, conn, record): - # NOTE(flaper87): This is necessary in order - # to ensure FK are treated correctly by sqlite. - conn.execute('pragma foreign_keys=ON') - def _mysql_on_connect(self, conn, record): # NOTE(flaper87): This is necessary in order # to ensure that all date operations in mysql @@ -43,18 +39,16 @@ class ControlDriver(storage.ControlDriverBase): conn.query('SET time_zone = "+0:00"') @decorators.lazy_property(write=False) - def engine(self, *args, **kwargs): + def engine(self): uri = self.sqlalchemy_conf.uri - engine = sa.create_engine(uri, **kwargs) - - # TODO(flaper87): Find a better way - # to do this. - if uri.startswith('sqlite://'): - sa.event.listen(engine, 'connect', - self._sqlite_on_connect) + engine = engines.create_engine(uri, sqlite_fk=True) if (uri.startswith('mysql://') or uri.startswith('mysql+pymysql://')): + # oslo_db.create_engine makes a test connection, throw that out + # first. mysql time_zone can be added to oslo_db as a + # startup option + engine.dispose() sa.event.listen(engine, 'connect', self._mysql_on_connect) diff --git a/zaqar/storage/sqlalchemy/flavors.py b/zaqar/storage/sqlalchemy/flavors.py index c0c977855..4a0030d35 100644 --- a/zaqar/storage/sqlalchemy/flavors.py +++ b/zaqar/storage/sqlalchemy/flavors.py @@ -17,6 +17,7 @@ controller for sqlalchemy. """ +import oslo_db.exception import sqlalchemy as sa from zaqar.storage import base @@ -80,7 +81,7 @@ class FlavorsController(base.FlavorsBase): capabilities=cap ) self.driver.run(stmt) - except sa.exc.IntegrityError: + except oslo_db.exception.DBDuplicateEntry: if not self._pools_ctrl.get_pools_by_group(pool_group): raise errors.PoolGroupDoesNotExist(pool_group) diff --git a/zaqar/storage/sqlalchemy/pools.py b/zaqar/storage/sqlalchemy/pools.py index 451ccf915..1156479d2 100644 --- a/zaqar/storage/sqlalchemy/pools.py +++ b/zaqar/storage/sqlalchemy/pools.py @@ -19,6 +19,7 @@ controller for sqlalchemy. import functools +import oslo_db.exception import sqlalchemy as sa from zaqar.common import utils as common_utils @@ -81,7 +82,7 @@ class PoolsController(base.PoolsBase): stmt = sa.sql.expression.insert(tables.PoolGroup).values(name=name) self.driver.run(stmt) return True - except sa.exc.IntegrityError: + except oslo_db.exception.DBDuplicateEntry: return False # TODO(cpp-cabrera): rename to upsert @@ -98,7 +99,7 @@ class PoolsController(base.PoolsBase): ) self.driver.run(stmt) - except sa.exc.IntegrityError: + except oslo_db.exception.DBDuplicateEntry: # TODO(cpp-cabrera): merge update/create into a single # method with introduction of upsert self._update(name, weight=weight, uri=uri, diff --git a/zaqar/storage/sqlalchemy/queues.py b/zaqar/storage/sqlalchemy/queues.py index 19ca0ba80..ea7e962cd 100644 --- a/zaqar/storage/sqlalchemy/queues.py +++ b/zaqar/storage/sqlalchemy/queues.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations under # the License. +import oslo_db.exception import sqlalchemy as sa from zaqar import storage @@ -85,7 +86,7 @@ class QueueController(storage.Queue): name=name, metadata=smeta) res = self.driver.run(ins) - except sa.exc.IntegrityError: + except oslo_db.exception.DBDuplicateEntry: return False return res.rowcount == 1