MDEV-25226 Assertion when wsrep_on set OFF with SR transaction

This patch makes the following changes around variable wsrep_on:

1) Variable wsrep_on can no longer be updated from a session that has
an active transaction running. The original behavior allowed cases
like this:

     BEGIN;
     INSERT INTO t1 VALUES (1);
     SET SESSION wsrep_on = OFF;
     INSERT INTO t1 VALUES (2);
     COMMIT;

With regular transactions this would result in no replication
events (not even value 1). With streaming replication it would be
unnecessarily complex to achieve the same behavior. In the above
example, it would be possible for value 1 to be already replicated if
it happened to fill a separate fragment, while value 2 wouldn't.

2) Global variable wsrep_on no longer affects current sessions, only
subsequent ones. This is to avoid a similar case to the above, just
using just by using global wsrep_on instead session wsrep_on:

      --connection conn_1
      BEGIN;
      INSERT INTO t1 VALUES(1);

      --connection conn_2
      SET GLOBAL wsrep_on = OFF;

      --connection conn_1
      INSERT INTO t1 VALUES(2);
      COMMIT;

The above example results in the transaction to be replicated, as
global wsrep_on will only affect the session wsrep_on of new
connections.

Reviewed-by: Jan Lindström <jan.lindstrom@mariadb.com>

10 files changed, 190 insertions, 19 deletions

diff --git a/mysql-test/suite/galera/r/galera_var_wsrep_on_off.result b/mysql-test/suite/galera/r/galera_var_wsrep_on_off.result
index 5323bc9bf60..b3096afd387 100644
--- a/mysql-test/suite/galera/r/galera_var_wsrep_on_off.result
+++ b/mysql-test/suite/galera/r/galera_var_wsrep_on_off.result
@@ -22,3 +22,49 @@ SELECT COUNT(*) = 1 FROM t1 WHERE f1 = 3;
 COUNT(*) = 1
 1
 DROP TABLE t1;
+START TRANSACTION;
+SET SESSION wsrep_on=OFF;
+ERROR 25000: You are not allowed to execute this command in a transaction
+SET GLOBAL wsrep_on=OFF;
+ERROR 25000: You are not allowed to execute this command in a transaction
+COMMIT;
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+START TRANSACTION;
+INSERT INTO t1 VALUES (1);
+connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1;;
+connection node_1a;
+SET GLOBAL wsrep_on = OFF;
+connection node_1;
+SHOW SESSION VARIABLES LIKE 'wsrep_on';
+Variable_name	Value
+wsrep_on	ON
+SHOW GLOBAL VARIABLES LIKE 'wsrep_on';
+Variable_name	Value
+wsrep_on	OFF
+INSERT INTO t1 VALUES (2);
+COMMIT;
+connection node_2;
+SET SESSION wsrep_sync_wait = 15;
+SELECT * FROM t1;
+f1
+1
+2
+connection node_1a;
+SET GLOBAL wsrep_on = ON;
+DROP TABLE t1;
+connection node_1;
+SET GLOBAL wsrep_on = OFF;
+connect node_1b, 127.0.0.1, root, , test, $NODE_MYPORT_1;;
+connection node_1b;
+SHOW SESSION VARIABLES LIKE 'wsrep_on';
+Variable_name	Value
+wsrep_on	OFF
+SHOW GLOBAL VARIABLES LIKE 'wsrep_on';
+Variable_name	Value
+wsrep_on	OFF
+CREATE TABLE t2 (f1 INTEGER);
+DROP TABLE t2;
+SET GLOBAL wsrep_on = ON;
+SHOW SESSION VARIABLES LIKE 'wsrep_on';
+Variable_name	Value
+wsrep_on	ON
diff --git a/mysql-test/suite/galera/t/galera_var_wsrep_on_off.test b/mysql-test/suite/galera/t/galera_var_wsrep_on_off.test
index 783b78792e6..1a48abc25eb 100644
--- a/mysql-test/suite/galera/t/galera_var_wsrep_on_off.test
+++ b/mysql-test/suite/galera/t/galera_var_wsrep_on_off.test
@@ -30,3 +30,60 @@ SELECT COUNT(*) = 1 FROM t1 WHERE f1 = 3;
 
 DROP TABLE t1;
 
+
+#
+# Test that variable wsrep_on cannot be changed while in
+# active transaction.
+#
+
+START TRANSACTION;
+--error ER_CANT_DO_THIS_DURING_AN_TRANSACTION
+SET SESSION wsrep_on=OFF;
+--error ER_CANT_DO_THIS_DURING_AN_TRANSACTION
+SET GLOBAL wsrep_on=OFF;
+COMMIT;
+
+
+#
+# Test that @@global.wsrep_on does not affect the value of
+# @@session.wsrep_on of current sessions
+#
+
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+START TRANSACTION;
+INSERT INTO t1 VALUES (1);
+
+--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1;
+--connection node_1a
+SET GLOBAL wsrep_on = OFF;
+
+--connection node_1
+SHOW SESSION VARIABLES LIKE 'wsrep_on';
+SHOW GLOBAL VARIABLES LIKE 'wsrep_on';
+INSERT INTO t1 VALUES (2);
+COMMIT;
+
+--connection node_2
+SET SESSION wsrep_sync_wait = 15;
+SELECT * FROM t1;
+
+--connection node_1a
+SET GLOBAL wsrep_on = ON;
+DROP TABLE t1;
+
+
+#
+# New connections inherit @@session.wsrep_on from @@global.wsrep_on
+#
+--connection node_1
+SET GLOBAL wsrep_on = OFF;
+
+--connect node_1b, 127.0.0.1, root, , test, $NODE_MYPORT_1;
+--connection node_1b
+SHOW SESSION VARIABLES LIKE 'wsrep_on';
+SHOW GLOBAL VARIABLES LIKE 'wsrep_on';
+CREATE TABLE t2 (f1 INTEGER);
+DROP TABLE t2;
+
+SET GLOBAL wsrep_on = ON;
+SHOW SESSION VARIABLES LIKE 'wsrep_on';
diff --git a/mysql-test/suite/galera_sr/r/MDEV-25226.result b/mysql-test/suite/galera_sr/r/MDEV-25226.result
new file mode 100644
index 00000000000..4699023562d
--- /dev/null
+++ b/mysql-test/suite/galera_sr/r/MDEV-25226.result
@@ -0,0 +1,24 @@
+connection node_2;
+connection node_1;
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+SET SESSION wsrep_trx_fragment_size=1;
+START TRANSACTION;
+INSERT INTO t1 VALUES(1);
+SET SESSION wsrep_on=OFF;
+ERROR 25000: You are not allowed to execute this command in a transaction
+SET GLOBAL wsrep_on=OFF;
+ERROR 25000: You are not allowed to execute this command in a transaction
+INSERT INTO t1 VALUES(2);
+COMMIT;
+connection node_1;
+SELECT * FROM t1;
+f1
+1
+2
+connection node_2;
+SELECT * FROM t1;
+f1
+1
+2
+connection node_1;
+DROP TABLE t1;
diff --git a/mysql-test/suite/galera_sr/t/MDEV-25226.test b/mysql-test/suite/galera_sr/t/MDEV-25226.test
new file mode 100644
index 00000000000..3d19a0b9675
--- /dev/null
+++ b/mysql-test/suite/galera_sr/t/MDEV-25226.test
@@ -0,0 +1,33 @@
+#
+# MDEV-25226 - Test the case the where wsrep_on is set OFF
+# on a transaction that has already replicated a fragment.
+#
+# This would cause: Assertion `transaction_.active() == false ||
+# (transaction_.state() == wsrep::transaction::s_executing ||
+# transaction_.state() == wsrep::transaction::s_prepared ||
+# transaction_.state() == wsrep::transaction::s_aborted ||
+# transaction_.state() == wsrep::transaction::s_must_abort)'
+#
+
+--source include/galera_cluster.inc
+
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+
+SET SESSION wsrep_trx_fragment_size=1;
+START TRANSACTION;
+INSERT INTO t1 VALUES(1);
+--error ER_CANT_DO_THIS_DURING_AN_TRANSACTION
+SET SESSION wsrep_on=OFF;
+--error ER_CANT_DO_THIS_DURING_AN_TRANSACTION
+SET GLOBAL wsrep_on=OFF;
+INSERT INTO t1 VALUES(2);
+COMMIT;
+
+--connection node_1
+SELECT * FROM t1;
+
+--connection node_2
+SELECT * FROM t1;
+
+--connection node_1
+DROP TABLE t1;
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index d0081034c0c..c536cdef504 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -1131,14 +1131,6 @@ PSI_file_key key_file_binlog_state;
 PSI_statement_info stmt_info_new_packet;
 #endif
 
-#ifdef WITH_WSREP
-/** Whether the Galera write-set replication is enabled. A cached copy of
-global_system_variables.wsrep_on && wsrep_provider &&
-  strcmp(wsrep_provider, WSREP_NONE)
-*/
-bool WSREP_ON_;
-#endif /* WITH_WSREP */
-
 #ifndef EMBEDDED_LIBRARY
 void net_before_header_psi(struct st_net *net, void *thd, size_t /* unused: count */)
 {
@@ -5710,10 +5702,7 @@ int mysqld_main(int argc, char **argv)
   }
 
 #ifdef WITH_WSREP
-  WSREP_ON_= (global_system_variables.wsrep_on &&
-          wsrep_provider &&
-          strcmp(wsrep_provider, WSREP_NONE));
-
+  wsrep_set_wsrep_on();
   if (WSREP_ON && wsrep_check_opts()) unireg_abort(1);
 #endif
 
diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc
index c3c4e4a0bf4..0338d7ad054 100644
--- a/sql/wsrep_mysqld.cc
+++ b/sql/wsrep_mysqld.cc
@@ -132,6 +132,18 @@ uint  wsrep_ignore_apply_errors= 0;
  */
 
 /*
+ * Cached variables
+ */
+
+// Whether the Galera write-set replication provider is set
+// wsrep_provider && strcmp(wsrep_provider, WSREP_NONE)
+bool WSREP_PROVIDER_EXISTS_;
+
+// Whether the Galera write-set replication is enabled
+// global_system_variables.wsrep_on && WSREP_PROVIDER_EXISTS_
+bool WSREP_ON_;
+
+/*
  * Other wsrep global variables.
  */
 
diff --git a/sql/wsrep_mysqld.h b/sql/wsrep_mysqld.h
index 02996012156..0b245ea11cb 100644
--- a/sql/wsrep_mysqld.h
+++ b/sql/wsrep_mysqld.h
@@ -20,6 +20,7 @@
 
 #ifdef WITH_WSREP
 extern bool WSREP_ON_;
+extern bool WSREP_PROVIDER_EXISTS_;
 
 #include <mysql/plugin.h>
 #include "mysql/service_wsrep.h"
@@ -221,7 +222,8 @@ extern wsrep_seqno_t wsrep_locked_seqno;
 /* use xxxxxx_NNULL macros when thd pointer is guaranteed to be non-null to
  * avoid compiler warnings (GCC 6 and later) */
 
-#define WSREP_NNULL(thd) (WSREP_ON && thd->variables.wsrep_on)
+#define WSREP_NNULL(thd) \
+  (WSREP_PROVIDER_EXISTS_ && thd->variables.wsrep_on)
 
 #define WSREP(thd) \
   (thd && WSREP_NNULL(thd))
@@ -277,8 +279,7 @@ void WSREP_LOG(void (*fun)(const char* fmt, ...), const char* fmt, ...);
     WSREP_INFO("context: %s:%d", __FILE__, __LINE__); \
   }
 
-#define WSREP_PROVIDER_EXISTS                                                  \
-  (wsrep_provider && strncasecmp(wsrep_provider, WSREP_NONE, FN_REFLEN))
+#define WSREP_PROVIDER_EXISTS (WSREP_PROVIDER_EXISTS_)
 
 static inline bool wsrep_cluster_address_exists()
 {
diff --git a/sql/wsrep_trans_observer.h b/sql/wsrep_trans_observer.h
index dbe710a0256..366cbef50d3 100644
--- a/sql/wsrep_trans_observer.h
+++ b/sql/wsrep_trans_observer.h
@@ -407,7 +407,7 @@ static inline void wsrep_after_apply(THD* thd)
 static inline void wsrep_open(THD* thd)
 {
   DBUG_ENTER("wsrep_open");
-  if (WSREP(thd))
+  if (WSREP_PROVIDER_EXISTS)
   {
     thd->wsrep_cs().open(wsrep::client_id(thd->thread_id));
     thd->wsrep_cs().debug_log_level(wsrep_debug);
diff --git a/sql/wsrep_var.cc b/sql/wsrep_var.cc
index 815a65ff240..01e9d022cfb 100644
--- a/sql/wsrep_var.cc
+++ b/sql/wsrep_var.cc
@@ -88,10 +88,11 @@ static bool refresh_provider_options()
   }
 }
 
-static void wsrep_set_wsrep_on()
+void wsrep_set_wsrep_on()
 {
-  WSREP_ON_= global_system_variables.wsrep_on && wsrep_provider &&
-    strcmp(wsrep_provider, WSREP_NONE);
+  WSREP_PROVIDER_EXISTS_= wsrep_provider &&
+    strncasecmp(wsrep_provider, WSREP_NONE, FN_REFLEN);
+  WSREP_ON_= global_system_variables.wsrep_on && WSREP_PROVIDER_EXISTS_;
 }
 
 /* This is intentionally declared as a weak global symbol, so that
@@ -146,6 +147,13 @@ bool wsrep_on_check(sys_var *self, THD* thd, set_var* var)
             " innodb_lock_schedule_algorithm=FCFS and restart.", MYF(0));
     return true;
   }
+
+  if (thd->in_active_multi_stmt_transaction())
+  {
+    my_error(ER_CANT_DO_THIS_DURING_AN_TRANSACTION, MYF(0));
+    return true;
+  }
+
   return false;
 }
 
diff --git a/sql/wsrep_var.h b/sql/wsrep_var.h
index 481df02f2d5..b1b2932cdfe 100644
--- a/sql/wsrep_var.h
+++ b/sql/wsrep_var.h
@@ -35,6 +35,7 @@ class set_var;
 class THD;
 
 int wsrep_init_vars();
+void wsrep_set_wsrep_on();
 
 #define CHECK_ARGS   (sys_var *self, THD* thd, set_var *var)
 #define UPDATE_ARGS  (sys_var *self, THD* thd, enum_var_type type)