From b689b6320cdb7dbbb366bf35f23083814f5e79ec Mon Sep 17 00:00:00 2001
From: Ayumu Ueha <ueha.ayumu@fujitsu.com>
Date: Tue, 17 Jan 2023 08:57:20 +0000
Subject: [PATCH] Fix default value for wsrep_sync_wait option

The default value for wsrep_sync_wait option should be `None`.
However, since 0 is set incorrectly, an unintended process is executed.

This patch fixes default value for wsrep_sync_wait option to `None`
instead of `0`.

Change-Id: Ifb1dc7ddcb127a69ea01234922caa7ca5ab111ce
---
 oslo_db/options.py                                  |  3 ++-
 oslo_db/sqlalchemy/enginefacade.py                  |  9 +++++----
 oslo_db/tests/sqlalchemy/test_enginefacade.py       | 13 +++++++++++++
 oslo_db/tests/sqlalchemy/test_options.py            |  1 +
 .../notes/fix_mysql_wsrsp-0ef98dec5ea3759f.yaml     |  9 +++++++++
 5 files changed, 30 insertions(+), 5 deletions(-)
 create mode 100644 releasenotes/notes/fix_mysql_wsrsp-0ef98dec5ea3759f.yaml

diff --git a/oslo_db/options.py b/oslo_db/options.py
index 824a4073..f124e8fd 100644
--- a/oslo_db/options.py
+++ b/oslo_db/options.py
@@ -64,7 +64,8 @@ database_opts = [
         default=None,
         help=(
             'For Galera only, configure wsrep_sync_wait causality '
-            'checks on new connections'
+            'checks on new connections.  Default is None, meaning don\'t '
+            'configure any setting.'
         ),
     ),
     cfg.BoolOpt(
diff --git a/oslo_db/sqlalchemy/enginefacade.py b/oslo_db/sqlalchemy/enginefacade.py
index cb6dd9a9..3a316b3c 100644
--- a/oslo_db/sqlalchemy/enginefacade.py
+++ b/oslo_db/sqlalchemy/enginefacade.py
@@ -145,7 +145,7 @@ class _TransactionFactory(object):
         self._engine_cfg = {
             'sqlite_fk': _Default(False),
             'mysql_sql_mode': _Default('TRADITIONAL'),
-            'mysql_wsrep_sync_wait': _Default(0),
+            'mysql_wsrep_sync_wait': _Default(),
             'mysql_enable_ndb': _Default(False),
             'connection_recycle_time': _Default(3600),
             'connection_debug': _Default(0),
@@ -219,8 +219,8 @@ class _TransactionFactory(object):
 
         :param mysql_sql_mode: MySQL SQL mode, defaults to TRADITIONAL
 
-        :param mysql_wsrep_sync_wait: MySQL wsrep_sync_wait, defaults to False
-         (i.e. '0')
+        :param mysql_wsrep_sync_wait: MySQL wsrep_sync_wait, defaults to None,
+         which indicates no setting will be passed
 
         :param mysql_enable_ndb: enable MySQL Cluster (NDB) support
 
@@ -1249,7 +1249,8 @@ class LegacyEngineFacade(object):
     :keyword mysql_sql_mode: the SQL mode to be used for MySQL sessions.
                              (defaults to TRADITIONAL)
     :keyword mysql_wsrep_sync_wait: value of wsrep_sync_wait for Galera
-                             (defaults to '0')
+                             (defaults to None, which indicates no setting
+                             will be passed)
     :keyword mysql_enable_ndb: If True, transparently enables support for
                                handling MySQL Cluster (NDB).
                                (defaults to False)
diff --git a/oslo_db/tests/sqlalchemy/test_enginefacade.py b/oslo_db/tests/sqlalchemy/test_enginefacade.py
index bdf51042..41758806 100644
--- a/oslo_db/tests/sqlalchemy/test_enginefacade.py
+++ b/oslo_db/tests/sqlalchemy/test_enginefacade.py
@@ -1379,6 +1379,19 @@ class PatchFactoryTest(test_base.BaseTestCase):
         self.assertTrue(engine_args['sqlite_fk'])
         self.assertEqual("FOOBAR", engine_args["mysql_sql_mode"])
         self.assertEqual(38, engine_args["max_overflow"])
+        self.assertNotIn("mysql_wsrep_sync_wait", engine_args)
+
+    def test_new_manager_from_options(self):
+        """test enginefacade's defaults given a default structure from opts"""
+
+        factory = enginefacade._TransactionFactory()
+        cfg.CONF.register_opts(options.database_opts, 'database')
+        factory.configure(**dict(cfg.CONF.database.items()))
+        engine_args = factory._engine_args_for_conf(None)
+
+        self.assertEqual(None, engine_args["mysql_wsrep_sync_wait"])
+        self.assertEqual(True, engine_args["sqlite_synchronous"])
+        self.assertEqual("TRADITIONAL", engine_args["mysql_sql_mode"])
 
 
 class SynchronousReaderWSlaveMockFacadeTest(MockFacadeTest):
diff --git a/oslo_db/tests/sqlalchemy/test_options.py b/oslo_db/tests/sqlalchemy/test_options.py
index 54c4ada1..c045e4d3 100644
--- a/oslo_db/tests/sqlalchemy/test_options.py
+++ b/oslo_db/tests/sqlalchemy/test_options.py
@@ -110,3 +110,4 @@ pool_timeout=7
 
         self.assertTrue(len(conf.database.items()) > 1)
         self.assertEqual('sqlite:///:memory:', conf.database.connection)
+        self.assertEqual(None, self.conf.database.mysql_wsrep_sync_wait)
diff --git a/releasenotes/notes/fix_mysql_wsrsp-0ef98dec5ea3759f.yaml b/releasenotes/notes/fix_mysql_wsrsp-0ef98dec5ea3759f.yaml
new file mode 100644
index 00000000..af8702e8
--- /dev/null
+++ b/releasenotes/notes/fix_mysql_wsrsp-0ef98dec5ea3759f.yaml
@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    The newly added mysql_wsrep_sync_wait parameter now defaults to non-present
+    in the enginefacade's default configuration options, so that it is not
+    configured in a MySQL / MariaDB database by default, unless passed in the
+    options explicitly. Previously, the default value was "0", meaning the
+    wsrep_sync_wait parameter would be set unconditionally on new connections,
+    which would fail for MySQL backends that don't provide for this setting.