More fixes to variable wsrep_on

* Disallow setting wsrep_on = 1 if wsrep_provider is unset. Also, move
  wsrep_on_basic from sys_vars to wsrep suite: this test now requires
  to run with wsrep_provider set
* Disallow setting @@session.wsrep_on = 1 when @@global.wsrep_on = 0
* Handle the case where a new connection turns @@global.wsrep_on from
  off to on. In this case we would miss a call to wsrep_open, causing
  unexpected states in wsrep::client_state (causing assertions).
* Disable wsrep.MDEV-22443 because it is no longer possible to enable
  wsrep_on, if server is started with wsrep_provider='none'

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

3 files changed, 82 insertions, 14 deletions

diff --git a/sql/service_wsrep.cc b/sql/service_wsrep.cc
index 80f164855b2..14f136ca480 100644
--- a/sql/service_wsrep.cc
+++ b/sql/service_wsrep.cc
@@ -120,15 +120,23 @@ extern "C" my_bool wsrep_get_debug()
   return wsrep_debug;
 }
 
+/*
+  Test if this connection is a true local (user) connection and not
+  a replication or wsrep applier thread.
+
+  Note that this is only usable for galera (as there are other kinds
+  of system threads, and only if WSREP_NNULL() is tested by the caller.
+ */
 extern "C" my_bool wsrep_thd_is_local(const THD *thd)
 {
   /*
-    async replication IO and background threads have nothing to replicate in the cluster,
-    marking them as non-local here to prevent write set population and replication
+    async replication IO and background threads have nothing to
+    replicate in the cluster, marking them as non-local here to
+    prevent write set population and replication
 
-    async replication SQL thread, applies client transactions from mariadb master
-    and will be replicated into cluster
-   */
+    async replication SQL thread, applies client transactions from
+    mariadb master and will be replicated into cluster
+  */
   return (
           thd->system_thread != SYSTEM_THREAD_SLAVE_BACKGROUND &&
           thd->system_thread != SYSTEM_THREAD_SLAVE_IO &&
diff --git a/sql/wsrep_trans_observer.h b/sql/wsrep_trans_observer.h
index 0a7cbf52a23..bb9bd54b02f 100644
--- a/sql/wsrep_trans_observer.h
+++ b/sql/wsrep_trans_observer.h
@@ -433,6 +433,16 @@ static inline void wsrep_close(THD* thd)
   DBUG_VOID_RETURN;
 }
 
+static inline void wsrep_cleanup(THD* thd)
+{
+  DBUG_ENTER("wsrep_cleanup");
+  if (thd->wsrep_cs().state() != wsrep::client_state::s_none)
+  {
+    thd->wsrep_cs().cleanup();
+  }
+  DBUG_VOID_RETURN;
+}
+
 static inline void
 wsrep_wait_rollback_complete_and_acquire_ownership(THD *thd)
 {
diff --git a/sql/wsrep_var.cc b/sql/wsrep_var.cc
index 01e9d022cfb..e4cfd0d89c9 100644
--- a/sql/wsrep_var.cc
+++ b/sql/wsrep_var.cc
@@ -25,6 +25,7 @@
 #include <my_dir.h>
 #include <cstdio>
 #include <cstdlib>
+#include "wsrep_trans_observer.h"
 
 ulong   wsrep_reject_queries;
 
@@ -103,7 +104,8 @@ struct handlerton* innodb_hton_ptr __attribute__((weak));
 
 bool wsrep_on_update (sys_var *self, THD* thd, enum_var_type var_type)
 {
-  if (var_type == OPT_GLOBAL) {
+  if (var_type == OPT_GLOBAL)
+  {
     my_bool saved_wsrep_on= global_system_variables.wsrep_on;
 
     thd->variables.wsrep_on= global_system_variables.wsrep_on;
@@ -111,15 +113,15 @@ bool wsrep_on_update (sys_var *self, THD* thd, enum_var_type var_type)
     // If wsrep has not been inited we need to do it now
     if (global_system_variables.wsrep_on && wsrep_provider && !wsrep_inited)
     {
-      char* tmp= strdup(wsrep_provider); // wsrep_init() rewrites provider
-                                         //when fails
-
+      // wsrep_init() rewrites provide if it fails
+      char* tmp= strdup(wsrep_provider);
       mysql_mutex_unlock(&LOCK_global_system_variables);
 
       if (wsrep_init())
       {
         my_error(ER_CANT_OPEN_LIBRARY, MYF(0), tmp, my_error, "wsrep_init failed");
         //rcode= true;
+        saved_wsrep_on= false;
       }
 
       free(tmp);
@@ -131,6 +133,16 @@ bool wsrep_on_update (sys_var *self, THD* thd, enum_var_type var_type)
 
   wsrep_set_wsrep_on();
 
+  if (var_type == OPT_GLOBAL)
+  {
+    if (thd->variables.wsrep_on &&
+        thd->wsrep_cs().state() == wsrep::client_state::s_none)
+    {
+      wsrep_open(thd);
+      wsrep_before_command(thd);
+    }
+  }
+
   return false;
 }
 
@@ -141,11 +153,31 @@ bool wsrep_on_check(sys_var *self, THD* thd, set_var* var)
   if (check_has_super(self, thd, var))
     return true;
 
-  if (new_wsrep_on && innodb_hton_ptr && innodb_lock_schedule_algorithm != 0) {
-    my_message(ER_WRONG_ARGUMENTS, " WSREP (galera) can't be enabled "
-            "if innodb_lock_schedule_algorithm=VATS. Please configure"
-            " innodb_lock_schedule_algorithm=FCFS and restart.", MYF(0));
-    return true;
+  if (new_wsrep_on)
+  {
+    if (innodb_hton_ptr && innodb_lock_schedule_algorithm != 0)
+    {
+      my_message(ER_WRONG_ARGUMENTS, " WSREP (galera) can't be enabled "
+                 "if innodb_lock_schedule_algorithm=VATS. Please configure"
+                 " innodb_lock_schedule_algorithm=FCFS and restart.", MYF(0));
+      return true;
+    }
+
+    if (!WSREP_PROVIDER_EXISTS)
+    {
+      my_message(ER_WRONG_ARGUMENTS, "WSREP (galera) can't be enabled "
+                 "if the wsrep_provider is unset or set to 'none'", MYF(0));
+      return true;
+    }
+
+    if (var->type == OPT_SESSION &&
+        !global_system_variables.wsrep_on)
+    {
+      my_message(ER_WRONG_ARGUMENTS,
+                 "Can't enable @@session.wsrep_on, "
+                 "while @@global.wsrep_on is disabled", MYF(0));
+      return true;
+    }
   }
 
   if (thd->in_active_multi_stmt_transaction())
@@ -154,6 +186,24 @@ bool wsrep_on_check(sys_var *self, THD* thd, set_var* var)
     return true;
   }
 
+  if (var->type == OPT_GLOBAL)
+  {
+    /*
+      The global value is about to change. Cleanup
+      the transaction state and close the client
+      state. wsrep_on_update() will take care of
+      reopening it should wsrep_on be re-enabled.
+     */
+    if (global_system_variables.wsrep_on && !new_wsrep_on)
+    {
+      wsrep_commit_empty(thd, true);
+      wsrep_after_statement(thd);
+      wsrep_after_command_ignore_result(thd);
+      wsrep_close(thd);
+      wsrep_cleanup(thd);
+    }
+  }
+
   return false;
 }