From f32890e41daca63b67074eb816b3d3d4458869b3 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 14 Jul 2022 11:33:19 +0100 Subject: Deprecate MySQL NDB Cluster Support Traditionally, the MySQL support in oslo.db has assumed use of the InnoDB storage engine. However, this isn't the only storage engine available and a few years ago an effort was made to add support for another storage engine, MySQL Cluster (NDB). The oslo.db aspects of this effort were tracked via bug 1564110 [1] and from reading this bug and looking at other patches related to this effort [2], it becomes obvious that this was never seen through to the completion and the OpenStack-wide effort never took off [3]. As a result, much of what is here is in-effect dead code now. Given no one is using this engine, there's no reason to keep it around. Deprecate it with an eye on removing it sooner rather than later. [1] https://bugs.launchpad.net/oslo.db/+bug/1564110 [2] https://review.opendev.org/q/owner:octave.orgeron%2540oracle.com [3] https://review.opendev.org/c/openstack/openstack-specs/+/429940 Change-Id: Id5ddf1d6f47b8a572001f58ad8b9b8a7dbe4e8ac Signed-off-by: Stephen Finucane --- oslo_db/options.py | 6 +++ oslo_db/sqlalchemy/enginefacade.py | 9 ++++ oslo_db/sqlalchemy/engines.py | 14 +++++- oslo_db/sqlalchemy/ndb.py | 57 +++++++++++++++++++--- oslo_db/sqlalchemy/types.py | 20 +++++++- oslo_db/sqlalchemy/utils.py | 23 +++++++-- oslo_db/tests/sqlalchemy/test_ndb.py | 34 ++++++++----- oslo_db/tests/sqlalchemy/test_utils.py | 4 +- ...mysql-ndb-cluster-support-cdcaa177b6a6773c.yaml | 5 ++ 9 files changed, 143 insertions(+), 29 deletions(-) create mode 100644 releasenotes/notes/deprecate-mysql-ndb-cluster-support-cdcaa177b6a6773c.yaml diff --git a/oslo_db/options.py b/oslo_db/options.py index df07c33..d827970 100644 --- a/oslo_db/options.py +++ b/oslo_db/options.py @@ -62,6 +62,12 @@ database_opts = [ cfg.BoolOpt( 'mysql_enable_ndb', default=False, + deprecated_for_removal=True, + deprecated_since='12.1.0', + deprecated_reason=( + 'Support for the MySQL NDB Cluster storage engine has been ' + 'deprecated and will be removed in a future release.' + ), help=( 'If True, transparently enables support for handling ' 'MySQL Cluster (NDB).' diff --git a/oslo_db/sqlalchemy/enginefacade.py b/oslo_db/sqlalchemy/enginefacade.py index f69811b..81341b7 100644 --- a/oslo_db/sqlalchemy/enginefacade.py +++ b/oslo_db/sqlalchemy/enginefacade.py @@ -316,6 +316,14 @@ class _TransactionFactory(object): self._configure(False, kw) def _configure(self, as_defaults, kw): + if 'mysql_enable_ndb' in kw: + debtcollector.deprecate( + ( + 'Support for the MySQL NDB Cluster storage engine has ' + 'been deprecated and will be removed in a future release.' + ), + version='12.1.0', + ) if self._started: raise AlreadyStartedError( @@ -1239,6 +1247,7 @@ class LegacyEngineFacade(object): :keyword mysql_enable_ndb: If True, transparently enables support for handling MySQL Cluster (NDB). (defaults to False) + **DEPRECATED** :keyword connection_recycle_time: Time period for connections to be recycled upon checkout (defaults to 3600) :keyword connection_debug: verbosity of SQL debugging information. diff --git a/oslo_db/sqlalchemy/engines.py b/oslo_db/sqlalchemy/engines.py index 32c4def..529db19 100644 --- a/oslo_db/sqlalchemy/engines.py +++ b/oslo_db/sqlalchemy/engines.py @@ -23,6 +23,7 @@ import re import time from urllib import parse +import debtcollector.removals import debtcollector.renames import sqlalchemy from sqlalchemy import event @@ -147,8 +148,19 @@ def _vet_url(url): ) +@debtcollector.removals.removed_kwarg( + 'mysql_enable_ndb', + message=( + 'Support for the MySQL NDB Cluster storage engine has been deprecated ' + 'and will be removed in a future release.' + ), + version='12.1.0', +) @debtcollector.renames.renamed_kwarg( - "idle_timeout", "connection_recycle_time", replace=True) + 'idle_timeout', + 'connection_recycle_time', + replace=True, +) def create_engine(sql_connection, sqlite_fk=False, mysql_sql_mode=None, mysql_enable_ndb=False, connection_recycle_time=3600, diff --git a/oslo_db/sqlalchemy/ndb.py b/oslo_db/sqlalchemy/ndb.py index 270b899..42b3189 100644 --- a/oslo_db/sqlalchemy/ndb.py +++ b/oslo_db/sqlalchemy/ndb.py @@ -15,18 +15,26 @@ import re -from oslo_db.sqlalchemy.types import String - +import debtcollector.removals from sqlalchemy import event, schema from sqlalchemy.ext.compiler import compiles from sqlalchemy.types import String as _String from sqlalchemy.types import to_instance +from oslo_db.sqlalchemy.types import String + engine_regex = re.compile("engine=innodb", re.IGNORECASE) trans_regex = re.compile("savepoint|rollback|release savepoint", re.IGNORECASE) +@debtcollector.removals.remove( + message=( + 'Support for the MySQL NDB Cluster storage engine has been deprecated ' + 'and will be removed in a future release.' + ), + version='12.1.0', +) def enable_ndb_support(engine): """Enable NDB Support. @@ -36,16 +44,45 @@ def enable_ndb_support(engine): engine.dialect._oslodb_enable_ndb_support = True +def _ndb_status(engine_or_compiler): + """Test if NDB Support is enabled. + + Function to test if NDB support is enabled or not. + + .. note:: + + This is for internal use only while we deprecate and remove ndb + support. **Do not use this outside of oslo.db!** + """ + return getattr( + engine_or_compiler.dialect, + '_oslodb_enable_ndb_support', + False, + ) + + +@debtcollector.removals.remove( + message=( + 'Support for the MySQL NDB Cluster storage engine has been deprecated ' + 'and will be removed in a future release.' + ), + version='12.1.0', +) def ndb_status(engine_or_compiler): """Test if NDB Support is enabled. Function to test if NDB support is enabled or not. """ - return getattr(engine_or_compiler.dialect, - '_oslodb_enable_ndb_support', - False) + return _ndb_status(engine_or_compiler) +@debtcollector.removals.remove( + message=( + 'Support for the MySQL NDB Cluster storage engine has been deprecated ' + 'and will be removed in a future release.' + ), + version='12.1.0', +) def init_ndb_events(engine): """Initialize NDB Events. @@ -60,7 +97,7 @@ def init_ndb_events(engine): convert InnoDB to NDBCLUSTER, drop SAVEPOINT requests, drop ROLLBACK requests, and drop RELEASE SAVEPOINT requests. """ - if ndb_status(engine): + if _ndb_status(engine): statement = engine_regex.sub("ENGINE=NDBCLUSTER", statement) if re.match(trans_regex, statement): statement = "SET @oslo_db_ndb_savepoint_rollback_disabled = 0;" @@ -68,6 +105,8 @@ def init_ndb_events(engine): return statement, parameters +# TODO(stephenfin): This is effectively deprecated and should be removed when +# we remove the rest of this module since it'll be a no-op then. @compiles(schema.CreateTable, "mysql") def prefix_inserts(create_table, compiler, **kw): """Replace InnoDB with NDBCLUSTER automatically. @@ -76,12 +115,14 @@ def prefix_inserts(create_table, compiler, **kw): convert InnoDB to NDBCLUSTER. Targets compiler events. """ existing = compiler.visit_create_table(create_table, **kw) - if ndb_status(compiler): + if _ndb_status(compiler): existing = engine_regex.sub("ENGINE=NDBCLUSTER", existing) return existing +# TODO(stephenfin): This is effectively deprecated and should be removed when +# we remove the rest of this module since it'll be a no-op then. @compiles(String, "mysql") def _compile_ndb_string(element, compiler, **kw): """Process ndb specific overrides for String. @@ -95,7 +136,7 @@ def _compile_ndb_string(element, compiler, **kw): mysql_ndb_type will change the column type to the requested data type. """ - if not ndb_status(compiler): + if not _ndb_status(compiler): return compiler.visit_string(element, **kw) if element.mysql_ndb_length: diff --git a/oslo_db/sqlalchemy/types.py b/oslo_db/sqlalchemy/types.py index a86f5ac..b30afce 100644 --- a/oslo_db/sqlalchemy/types.py +++ b/oslo_db/sqlalchemy/types.py @@ -12,6 +12,7 @@ import json +import debtcollector.removals from sqlalchemy.dialects import mysql from sqlalchemy.types import Integer, Text, TypeDecorator, String as _String @@ -113,6 +114,18 @@ class SoftDeleteInteger(TypeDecorator): return int(value) +# NOTE(stephenfin): We deprecate the whole class rather than just the +# ndb-related arguments, since without these arguments this is identical to the +# upstream SQLAlchemy type +@debtcollector.removals.removed_class( + 'String', + message=( + 'Support for the MySQL NDB Cluster storage engine has been ' + 'deprecated and will be removed in a future release. Use the ' + 'standard String type from sqlalchemy.types' + ), + version='12.1.0', +) class String(_String): """String subclass that implements oslo_db specific options. @@ -124,7 +137,12 @@ class String(_String): """ def __init__( - self, length, mysql_ndb_length=None, mysql_ndb_type=None, **kw): + self, + length, + mysql_ndb_length=None, + mysql_ndb_type=None, + **kw, + ): """Initialize options.""" super(String, self).__init__(length, **kw) self.mysql_ndb_type = mysql_ndb_type diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index 3a6a993..09156f5 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -1136,6 +1136,13 @@ def get_non_innodb_tables(connectable, skip_tables=('migrate_version', return [i[0] for i in noninnodb] +@debtcollector.removals.remove( + message=( + 'Support for the MySQL NDB Cluster storage engine has been deprecated ' + 'and will be removed in a future release.' + ), + version='12.1.0', +) def get_non_ndbcluster_tables(connectable, skip_tables=None): """Get a list of tables which don't use MySQL Cluster (NDB) storage engine. @@ -1187,7 +1194,8 @@ def get_foreign_key_constraint_name(engine, table_name, column_name): @contextlib.contextmanager def suspend_fk_constraints_for_col_alter( - engine, table_name, column_name, referents=[]): + engine, table_name, column_name, referents=[], +): """Detect foreign key constraints, drop, and recreate. This is used to guard against a column ALTER that on some backends @@ -1218,11 +1226,16 @@ def suspend_fk_constraints_for_col_alter( :param referents: sequence of string table names to search for foreign key constraints. A future version of this function may no longer require this argument, however for the moment it is required. - """ - if ( - not ndb.ndb_status(engine) - ): + debtcollector.deprecate( + ( + 'Support for the MySQL NDB Cluster storage engine has been ' + 'deprecated and will be removed in a future release.' + ), + version='12.1.0', + ) + + if not ndb._ndb_status(engine): yield else: with engine.connect() as conn: diff --git a/oslo_db/tests/sqlalchemy/test_ndb.py b/oslo_db/tests/sqlalchemy/test_ndb.py index 06c4d82..c516b1b 100644 --- a/oslo_db/tests/sqlalchemy/test_ndb.py +++ b/oslo_db/tests/sqlalchemy/test_ndb.py @@ -15,6 +15,7 @@ import logging from unittest import mock +import warnings from oslo_db import exception from oslo_db.sqlalchemy import enginefacade @@ -38,12 +39,16 @@ from sqlalchemy import Text LOG = logging.getLogger(__name__) _MOCK_CONNECTION = 'mysql+pymysql://' -_TEST_TABLE = Table("test_ndb", MetaData(), - Column('id', Integer, primary_key=True), - Column('test1', String(255, mysql_ndb_type=TEXT)), - Column('test2', String(4096, mysql_ndb_type=TEXT)), - Column('test3', String(255, mysql_ndb_length=64)), - mysql_engine='InnoDB') +with warnings.catch_warnings(): # hide deprecation warning + _TEST_TABLE = Table( + "test_ndb", + MetaData(), + Column('id', Integer, primary_key=True), + Column('test1', String(255, mysql_ndb_type=TEXT)), + Column('test2', String(4096, mysql_ndb_type=TEXT)), + Column('test3', String(255, mysql_ndb_length=64)), + mysql_engine='InnoDB', + ) class NDBMockTestBase(test_base.BaseTestCase): @@ -57,7 +62,9 @@ class NDBMockTestBase(test_base.BaseTestCase): self.addCleanup( setattr, test_engine.dialect, "_oslodb_enable_ndb_support", False ) - ndb.init_ndb_events(test_engine) + # hide deprecation warnings + with warnings.catch_warnings(): + ndb.init_ndb_events(test_engine) class NDBEventTestCase(NDBMockTestBase): @@ -164,9 +171,10 @@ class NDBOpportunisticTestCase( # if we want NDB, make a new local engine that uses the # URL / database / schema etc. of the provisioned engine, # since NDB-ness is a per-table thing - self.engine = engines.create_engine( - self.engine.url, mysql_enable_ndb=True - ) + with warnings.catch_warnings(): # hide deprecation warnings + self.engine = engines.create_engine( + self.engine.url, mysql_enable_ndb=True + ) self.addCleanup(self.engine.dispose) self.test_table = _TEST_TABLE try: @@ -176,7 +184,8 @@ class NDBOpportunisticTestCase( def test_ndb_enabled(self): self.init_db(True) - self.assertTrue(ndb.ndb_status(self.engine)) + with warnings.catch_warnings(): # hide deprecation warnings + self.assertTrue(ndb.ndb_status(self.engine)) self.assertIsInstance(self.test_table.c.test1.type, TINYTEXT) self.assertIsInstance(self.test_table.c.test2.type, Text) self.assertIsInstance(self.test_table.c.test3.type, String) @@ -185,7 +194,8 @@ class NDBOpportunisticTestCase( def test_ndb_disabled(self): self.init_db(False) - self.assertFalse(ndb.ndb_status(self.engine)) + with warnings.catch_warnings(): # hide deprecation warnings + self.assertFalse(ndb.ndb_status(self.engine)) self.assertIsInstance(self.test_table.c.test1.type, String) self.assertEqual(255, self.test_table.c.test1.type.length) self.assertIsInstance(self.test_table.c.test2.type, String) diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py index 27ed640..50973fa 100644 --- a/oslo_db/tests/sqlalchemy/test_utils.py +++ b/oslo_db/tests/sqlalchemy/test_utils.py @@ -1020,7 +1020,7 @@ class TestMigrationUtils(db_test_base._DbTestCase): normalize_fk_entries(existing_foreign_keys) ) - with mock.patch("oslo_db.sqlalchemy.ndb.ndb_status", + with mock.patch("oslo_db.sqlalchemy.ndb._ndb_status", mock.Mock(return_value=True)): with utils.suspend_fk_constraints_for_col_alter( self.engine, 'a', 'id', referents=['b', 'c']): @@ -1034,7 +1034,7 @@ class TestMigrationUtils(db_test_base._DbTestCase): self.assertEqual(existing_foreign_keys, get_fk_entries()) - with mock.patch("oslo_db.sqlalchemy.ndb.ndb_status", + with mock.patch("oslo_db.sqlalchemy.ndb._ndb_status", mock.Mock(return_value=True)): with utils.suspend_fk_constraints_for_col_alter( self.engine, 'b', 'archive_id', referents=['c']): diff --git a/releasenotes/notes/deprecate-mysql-ndb-cluster-support-cdcaa177b6a6773c.yaml b/releasenotes/notes/deprecate-mysql-ndb-cluster-support-cdcaa177b6a6773c.yaml new file mode 100644 index 0000000..0fdf75a --- /dev/null +++ b/releasenotes/notes/deprecate-mysql-ndb-cluster-support-cdcaa177b6a6773c.yaml @@ -0,0 +1,5 @@ +--- +deprecations: + - | + MySQL NDB Cluster support has been deprecated for removal. It appears no + one is using this functionality and it's poorly understood. -- cgit v1.2.1