MDEV-22203: WSREP_ON is unnecessarily expensive to evaluate

Replaced WSREP_ON macro by single global variable WSREP_ON
that is then updated at server statup and on wsrep_on and
wsrep_provider update functions.

mysql-test/suite/galera/t/MW-86-wait1.test

@@ -5,6 +5,7 @@
 #
 --source include/galera_cluster.inc
 --source include/have_binlog_format_row.inc
+--source include/have_debug.inc
 --source include/have_debug_sync.inc
 SET @orig_debug=@@debug_dbug;
 

sql/handler.cc

@@ -2131,6 +2131,11 @@ static my_bool xarecover_handlerton(THD *unused, plugin_ref plugin,
          the prepare.
       */
       my_xid wsrep_limit __attribute__((unused))= 0;
+
+      /* Note that we could call this for binlog also that
+         will not have WSREP(thd) but global wsrep on might
+         be true.
+      */
       if (WSREP_ON)
         wsrep_limit= wsrep_order_and_check_continuity(info->list, got);
 

sql/log.cc

@@ -1826,7 +1826,7 @@ binlog_commit_flush_stmt_cache(THD *thd, bool all,
 #ifdef WITH_WSREP
   if (thd->wsrep_mysql_replicated > 0)
   {
-    DBUG_ASSERT(WSREP_ON);
+    DBUG_ASSERT(WSREP(thd));
     WSREP_DEBUG("avoiding binlog_commit_flush_trx_cache: %d",
                 thd->wsrep_mysql_replicated);
     return 0;
@@ -6739,14 +6739,15 @@ int MYSQL_BIN_LOG::rotate(bool force_rotate, bool* check_purge)
   int error= 0;
   DBUG_ENTER("MYSQL_BIN_LOG::rotate");
 
-  if (wsrep_to_isolation)
+#ifdef WITH_WSREP
+  if (WSREP_ON && wsrep_to_isolation)
   {
-    DBUG_ASSERT(WSREP_ON);
     *check_purge= false;
-    WSREP_DEBUG("avoiding binlog rotate due to TO isolation: %d", 
+    WSREP_DEBUG("avoiding binlog rotate due to TO isolation: %d",
                 wsrep_to_isolation);
     DBUG_RETURN(0);
   }
+#endif /* WITH_WSREP */
 
   //todo: fix the macro def and restore safe_mutex_assert_owner(&LOCK_log);
   *check_purge= false;

sql/log_event.cc

@@ -4342,9 +4342,14 @@ Query_log_event::Query_log_event(THD* thd_arg, const char* query_arg, size_t que
   /*
     If Query_log_event will contain non trans keyword (not BEGIN, COMMIT,
     SAVEPOINT or ROLLBACK) we disable PA for this transaction.
+    Note that here WSREP(thd) might not be true e.g. when wsrep_shcema
+    is created we create tables with thd->variables.wsrep_on=false
+    to avoid replicating wsrep_schema tables to other nodes.
    */
   if (WSREP_ON && !is_trans_keyword())
+  {
     thd->wsrep_PA_safe= false;
+  }
 #endif /* WITH_WSREP */
 
   memset(&user, 0, sizeof(user));
@@ -5967,7 +5972,7 @@ Query_log_event::do_shall_skip(rpl_group_info *rgi)
     }
   }
 #ifdef WITH_WSREP
-  else if (WSREP_ON && wsrep_mysql_replication_bundle && opt_slave_domain_parallel_threads == 0 &&
+  else if (WSREP(thd) && wsrep_mysql_replication_bundle && opt_slave_domain_parallel_threads == 0 &&
            thd->wsrep_mysql_replicated > 0 &&
            (is_begin() || is_commit()))
   {
@@ -5981,7 +5986,7 @@ Query_log_event::do_shall_skip(rpl_group_info *rgi)
       thd->wsrep_mysql_replicated = 0;
     }
   }
-#endif
+#endif /* WITH_WSREP */
   DBUG_RETURN(Log_event::do_shall_skip(rgi));
 }
 
@@ -9075,7 +9080,7 @@ Xid_log_event::do_shall_skip(rpl_group_info *rgi)
     DBUG_RETURN(Log_event::EVENT_SKIP_COUNT);
   }
 #ifdef WITH_WSREP
-  else if (wsrep_mysql_replication_bundle && WSREP_ON &&
+  else if (wsrep_mysql_replication_bundle && WSREP(thd) &&
            opt_slave_domain_parallel_threads == 0)
   {
     if (++thd->wsrep_mysql_replicated < (int)wsrep_mysql_replication_bundle)

sql/mdl.cc

@@ -1215,12 +1215,12 @@ void MDL_lock::Ticket_list::add_ticket(MDL_ticket *ticket)
       wsrep_thd_is_BF(ticket->get_ctx()->get_thd(), false))
   {
     Ticket_iterator itw(ticket->get_lock()->m_waiting);
-
-    DBUG_ASSERT(WSREP_ON);
     MDL_ticket *waiting;
     MDL_ticket *prev=NULL;
     bool added= false;
 
+    DBUG_ASSERT(WSREP(ticket->get_ctx()->get_thd()));
+
     while ((waiting= itw++) && !added)
     {
       if (!wsrep_thd_is_BF(waiting->get_ctx()->get_thd(), true))

sql/mysqld.cc

@@ -1136,6 +1136,8 @@ PSI_file_key key_file_binlog_state;
 PSI_statement_info stmt_info_new_packet;
 #endif
 
+my_bool WSREP_ON= false;
+
 #ifndef EMBEDDED_LIBRARY
 void net_before_header_psi(struct st_net *net, void *thd, size_t /* unused: count */)
 {
@@ -1874,6 +1876,9 @@ extern "C" void unireg_abort(int exit_code)
   disable_log_notes= 1;
 
 #ifdef WITH_WSREP
+  // Note that we do not have thd here, thus can't use
+  // WSREP(thd)
+
   if (WSREP_ON &&
       Wsrep_server_state::is_inited() &&
       Wsrep_server_state::instance().state() != wsrep::server_state::s_disconnected)
@@ -1889,6 +1894,7 @@ extern "C" void unireg_abort(int exit_code)
     sleep(1); /* so give some time to exit for those which can */
     WSREP_INFO("Some threads may fail to exit.");
   }
+
   if (WSREP_ON)
   {
     /* In bootstrap mode we deinitialize wsrep here. */
@@ -4856,7 +4862,6 @@ static int init_default_storage_engine_impl(const char *opt_name,
   return 0;
 }
 
-
 static int
 init_gtid_pos_auto_engines(void)
 {
@@ -4883,7 +4888,6 @@ init_gtid_pos_auto_engines(void)
   return 0;
 }
 
-
 static int init_server_components()
 {
   DBUG_ENTER("init_server_components");
@@ -5714,6 +5718,14 @@ int mysqld_main(int argc, char **argv)
       set_user(mysqld_user, user_info);
   }
 
+#ifdef WITH_WSREP
+  WSREP_ON= (global_system_variables.wsrep_on &&
+          wsrep_provider &&
+          strcmp(wsrep_provider, WSREP_NONE));
+#else
+  WSREP_ON= false;
+#endif
+
   if (WSREP_ON && wsrep_check_opts()) unireg_abort(1);
 
   /* 

sql/rpl_record.cc

@@ -329,22 +329,6 @@ unpack_row(rpl_group_info *rgi,
                              (int) (pack_ptr - old_pack_ptr)));
         if (!pack_ptr)
         {
-#ifdef WITH_WSREP
-	  if (WSREP_ON)
-          {
-            /*
-              Debug message to troubleshoot bug:
-              https://mariadb.atlassian.net/browse/MDEV-4404
-              Galera Node throws "Could not read field" error and drops out of cluster
-            */
-            WSREP_WARN("ROW event unpack field: %s  metadata: 0x%x;"
-                       " conv_table %p conv_field %p table %s"
-                       " row_end: %p",
-                       f->field_name.str, metadata, conv_table, conv_field,
-                       (table_found) ? "found" : "not found", row_end
-            );
-	  }
-#endif /* WITH_WSREP */
           rgi->rli->report(ERROR_LEVEL, ER_SLAVE_CORRUPT_EVENT,
                       rgi->gtid_info(),
                       "Could not read field '%s' of table '%s.%s'",

sql/slave.cc

@@ -3959,7 +3959,8 @@ apply_event_and_update_pos_apply(Log_event* ev, THD* thd, rpl_group_info *rgi,
     exec_res= ev->apply_event(rgi);
 
 #ifdef WITH_WSREP
-  if (WSREP_ON) {
+  if (WSREP(thd)) {
+
     if (exec_res) {
       mysql_mutex_lock(&thd->LOCK_thd_data);
       switch(thd->wsrep_trx().state()) {
@@ -5609,7 +5610,7 @@ pthread_handler_t handle_slave_sql(void *arg)
     if (exec_relay_log_event(thd, rli, serial_rgi))
     {
 #ifdef WITH_WSREP
-      if (WSREP_ON)
+      if (WSREP(thd))
       {
         mysql_mutex_lock(&thd->LOCK_thd_data);
 
@@ -5627,8 +5628,10 @@ pthread_handler_t handle_slave_sql(void *arg)
       if (!sql_slave_killed(serial_rgi))
       {
         slave_output_error_info(serial_rgi, thd);
-        if (WSREP_ON && rli->last_error().number == ER_UNKNOWN_COM_ERROR)
+        if (WSREP(thd) && rli->last_error().number == ER_UNKNOWN_COM_ERROR)
+        {
           wsrep_node_dropped= TRUE;
+        }
       }
       goto err;
     }
@@ -5765,7 +5768,7 @@ err_during_init:
     If slave stopped due to node going non primary, we set global flag to
     trigger automatic restart of slave when node joins back to cluster.
   */
-  if (WSREP_ON && wsrep_node_dropped && wsrep_restart_slave)
+  if (WSREP(thd) && wsrep_node_dropped && wsrep_restart_slave)
   {
     if (wsrep_ready_get())
     {

sql/sql_base.cc

@@ -4480,7 +4480,7 @@ restart:
   }
 
 #ifdef WITH_WSREP
-  if (WSREP_ON                                         &&
+  if (WSREP(thd)                                       &&
       wsrep_replicate_myisam                           &&
       (*start)                                         &&
       (*start)->table                                  &&

sql/sql_class.h

@@ -3296,7 +3296,7 @@ public:
   void awake_no_mutex(killed_state state_to_set);
   void awake(killed_state state_to_set)
   {
-    bool wsrep_on_local= WSREP_ON;
+    bool wsrep_on_local= WSREP_NNULL(this);
     /*
       mutex locking order (LOCK_thd_data - LOCK_thd_kill)) requires
       to grab LOCK_thd_data here

sql/sql_parse.cc

@@ -1753,7 +1753,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
   {
     mysqld_stmt_bulk_execute(thd, packet, packet_length);
 #ifdef WITH_WSREP
-    if (WSREP_ON)
+    if (WSREP(thd))
     {
         (void)wsrep_after_statement(thd);
     }
@@ -1764,7 +1764,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
   {
     mysqld_stmt_execute(thd, packet, packet_length);
 #ifdef WITH_WSREP
-    if (WSREP_ON)
+    if (WSREP(thd))
     {
         (void)wsrep_after_statement(thd);
     }
@@ -1822,7 +1822,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
       break;
 
 #ifdef WITH_WSREP
-    if (WSREP_ON)
+    if (WSREP(thd))
     {
       if (wsrep_mysql_parse(thd, thd->query(), thd->query_length(),
                             &parser_state,
@@ -1924,7 +1924,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
       parser_state.reset(beginning_of_next_stmt, length);
 
 #ifdef WITH_WSREP
-      if (WSREP_ON)
+      if (WSREP(thd))
       {
         if (wsrep_mysql_parse(thd, beginning_of_next_stmt,
                               length, &parser_state,

sql/sql_reload.cc

@@ -153,6 +153,8 @@ bool reload_acl_and_cache(THD *thd, unsigned long long options,
       if (mysql_bin_log.rotate_and_purge(true, drop_gtid_domain))
         *write_to_binlog= -1;
 
+      /* Note that WSREP(thd) might not be true here e.g. during
+      SST. */
       if (WSREP_ON)
       {
         /* Wait for last binlog checkpoint event to be logged. */

sql/sql_repl.cc

@@ -3940,7 +3940,7 @@ int reset_master(THD* thd, rpl_gtid *init_state, uint32 init_state_len,
   }
 
 #ifdef WITH_WSREP
-  if (WSREP_ON)
+  if (WSREP(thd))
   {
     /* RESET MASTER will initialize GTID sequence, and that would happen locally
        in this node, so better reject it

sql/sql_table.cc

@@ -5620,9 +5620,11 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table,
   DBUG_ENTER("mysql_create_like_table");
 
 #ifdef WITH_WSREP
-  if (WSREP_ON && !thd->wsrep_applier &&
+  if (WSREP(thd) && !thd->wsrep_applier &&
       wsrep_create_like_table(thd, table, src_table, create_info))
+  {
     DBUG_RETURN(res);
+  }
 #endif
 
   /*

sql/wsrep_mysqld.cc

@@ -732,6 +732,7 @@ void wsrep_init_globals()
 {
   wsrep_init_sidno(Wsrep_server_state::instance().connected_gtid().id());
   wsrep_init_schema();
+
   if (WSREP_ON)
   {
     Wsrep_server_state::instance().initialized();
@@ -769,6 +770,11 @@ int wsrep_init()
 
   global_system_variables.wsrep_on= 1;
 
+  if (wsrep_provider && strcmp(wsrep_provider, WSREP_NONE))
+    WSREP_ON= true;
+  else
+    WSREP_ON= false;
+
   if (wsrep_gtid_mode && opt_bin_log && !opt_log_slave_updates)
   {
     WSREP_ERROR("Option --log-slave-updates is required if "

sql/wsrep_mysqld.h

@@ -18,6 +18,8 @@
 
 #include <wsrep.h>
 
+extern my_bool WSREP_ON;
+
 #ifdef WITH_WSREP
 
 #include <mysql/plugin.h>
@@ -213,15 +215,11 @@ extern void wsrep_prepend_PATH (const char* path);
 
 /* Other global variables */
 extern wsrep_seqno_t wsrep_locked_seqno;
-#define WSREP_ON                         \
-  ((global_system_variables.wsrep_on) && \
-   wsrep_provider                     && \
-   strcmp(wsrep_provider, WSREP_NONE))
 
 /* 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_ON && thd->variables.wsrep_on)
 
 #define WSREP(thd) \
   (thd && WSREP_NNULL(thd))
@@ -484,7 +482,6 @@ enum wsrep::streaming_context::fragment_unit wsrep_fragment_unit(ulong unit);
 
 #define WSREP(T)  (0)
 #define WSREP_NNULL(T) (0)
-#define WSREP_ON  (0)
 #define WSREP_EMULATE_BINLOG(thd) (0)
 #define WSREP_EMULATE_BINLOG_NNULL(thd) (0)
 #define WSREP_BINLOG_FORMAT(my_format) ((ulong)my_format)

sql/wsrep_sst.cc

@@ -367,6 +367,7 @@ void wsrep_sst_received (THD*                thd,
       my_pthread_setspecific_ptr(THR_THD, NULL);
     }
 
+    /* During sst WSREP(thd) is not yet set for joiner. */
     if (WSREP_ON)
     {
       int const rcode(seqno < 0 ? seqno : 0);

sql/wsrep_thd.cc

@@ -320,10 +320,15 @@ int wsrep_abort_thd(THD *bf_thd_ptr, THD *victim_thd_ptr, my_bool signal)
   DBUG_ENTER("wsrep_abort_thd");
   THD *victim_thd= (THD *) victim_thd_ptr;
   THD *bf_thd= (THD *) bf_thd_ptr;
+
   mysql_mutex_lock(&victim_thd->LOCK_thd_data);
-  if ( (WSREP(bf_thd) ||
+
-         ( (WSREP_ON || bf_thd->variables.wsrep_OSU_method == WSREP_OSU_RSU) &&
+  /* Note that when you use RSU node is desynced from cluster, thus WSREP(thd)
-           wsrep_thd_is_toi(bf_thd)) )                         &&
+  might not be true.
+  */
+  if ((WSREP(bf_thd) ||
+       ((WSREP_ON || bf_thd->variables.wsrep_OSU_method == WSREP_OSU_RSU) &&
+	 wsrep_thd_is_toi(bf_thd))) &&
        victim_thd &&
        !wsrep_thd_is_aborting(victim_thd))
   {
@@ -337,6 +342,7 @@ int wsrep_abort_thd(THD *bf_thd_ptr, THD *victim_thd_ptr, my_bool signal)
   {
     WSREP_DEBUG("wsrep_abort_thd not effective: %p %p", bf_thd, victim_thd);
   }
+
   mysql_mutex_unlock(&victim_thd->LOCK_thd_data);
   DBUG_RETURN(1);
 }

sql/wsrep_var.cc

@@ -88,6 +88,15 @@ static bool refresh_provider_options()
   }
 }
 
+static void wsrep_set_wsrep_on(void)
+{
+  if (global_system_variables.wsrep_on && wsrep_provider &&
+      strcmp(wsrep_provider, WSREP_NONE))
+    WSREP_ON= true;
+  else
+    WSREP_ON= false;
+}
+
 /* This is intentionally declared as a weak global symbol, so that
 linking will succeed even if the server is built with a dynamically
 linked InnoDB. */
@@ -122,6 +131,8 @@ bool wsrep_on_update (sys_var *self, THD* thd, enum_var_type var_type)
     thd->variables.wsrep_on= global_system_variables.wsrep_on= saved_wsrep_on;
   }
 
+  wsrep_set_wsrep_on();
+
   return false;
 }
 
@@ -378,6 +389,7 @@ bool wsrep_provider_update (sys_var *self, THD* thd, enum_var_type type)
   if (!rcode)
     refresh_provider_options();
 
+  wsrep_set_wsrep_on();
   mysql_mutex_lock(&LOCK_global_system_variables);
 
   return rcode;
@@ -397,6 +409,7 @@ void wsrep_provider_init (const char* value)
 
   if (wsrep_provider) my_free((void *)wsrep_provider);
   wsrep_provider= my_strdup(value, MYF(0));
+  wsrep_set_wsrep_on();
 }
 
 bool wsrep_provider_options_check(sys_var *self, THD* thd, set_var* var)
@@ -875,6 +888,8 @@ static void export_wsrep_status_to_mysql(THD* thd)
 
 int wsrep_show_status (THD *thd, SHOW_VAR *var, char *buff)
 {
+  /* Note that we should allow show status like 'wsrep%' even
+  when WSREP(thd) is false. */
   if (WSREP_ON)
   {
     export_wsrep_status_to_mysql(thd);