MDEV-20848 Fixes for MTR test galera_sr.GCF-1060 (#1421)

This patch contains two fixes:

* wsrep_handle_mdl_conflict(): handle the case where SR transaction
  is in aborting state. Previously, a BF-BF conflict was reported, and
  the process would abort.
* wsrep_thd_bf_abort(): do not restore thread vars after calling
  wsrep_bf_abort(). Thread vars are already restored in wsrep-lib if
  necessary. This also removes the assumption that the caller of
  wsrep_thd_bf_abort() is the given bf_thd, which is not the case.

Also in this patch:

* Remove unnecessary check for active victim transaction in
  wsrep_thd_bf_abort(): the exact same check is performed later in
  wsrep_bf_abort().
* Make wsrep_thd_bf_abort() and wsrep_log_thd() const-correct.
* Change signature of wsrep_abort_thd() to take THD pointers instead
  of void pointers.

include/mysql/service_wsrep.h

@@ -74,7 +74,7 @@ extern struct wsrep_service_st {
   const char*                 (*wsrep_thd_client_mode_str_func)(const MYSQL_THD thd);
   const char*                 (*wsrep_thd_transaction_state_str_func)(const MYSQL_THD thd);
   query_id_t                  (*wsrep_thd_transaction_id_func)(const MYSQL_THD thd);
-  my_bool                     (*wsrep_thd_bf_abort_func)(const MYSQL_THD bf_thd,
+  my_bool                     (*wsrep_thd_bf_abort_func)(MYSQL_THD bf_thd,
                                                          MYSQL_THD victim_thd,
                                                          my_bool signal);
   my_bool                     (*wsrep_thd_order_before_func)(const MYSQL_THD left, const MYSQL_THD right);
@@ -190,7 +190,7 @@ extern "C" void wsrep_handle_SR_rollback(MYSQL_THD BF_thd, MYSQL_THD victim_thd)
 /* Return thd retry counter */
 extern "C" int wsrep_thd_retry_counter(const MYSQL_THD thd);
 /* BF abort victim_thd */
-extern "C" my_bool wsrep_thd_bf_abort(const MYSQL_THD bf_thd,
+extern "C" my_bool wsrep_thd_bf_abort(MYSQL_THD bf_thd,
                                       MYSQL_THD victim_thd,
                                       my_bool signal);
 /* Return true if left thd is ordered before right thd */

include/thr_lock.h

@@ -169,7 +169,7 @@ void thr_set_lock_wait_callback(void (*before_wait)(void),
 
 #ifdef WITH_WSREP
   typedef my_bool (* wsrep_thd_is_brute_force_fun)(const MYSQL_THD, my_bool);
-  typedef my_bool(* wsrep_abort_thd_fun)(const MYSQL_THD, MYSQL_THD, my_bool);
+  typedef my_bool(* wsrep_abort_thd_fun)(MYSQL_THD, MYSQL_THD, my_bool);
   typedef my_bool (* wsrep_on_fun)(const MYSQL_THD);
   void wsrep_thr_lock_init(
     wsrep_thd_is_brute_force_fun bf_fun, wsrep_abort_thd_fun abort_fun,

mysql-test/suite/galera_sr/disabled.def

@@ -1,6 +1,4 @@
 galera_sr_table_contents : missing file
 GCF-437 : test relies on InnoDB redo log size limitation
-GCF-1060 : MDEV-20848 Galera test failure on galera_sr.GCF_1060
 galera_sr_ddl_master : MDEV-20780 Galera test failure on galera_sr.galera_sr_ddl_master
 GCF-1043A : MDEV-21170 Galera test failure on galera_sr.GCF-1043A
-

sql/service_wsrep.cc

@@ -196,18 +196,10 @@ extern "C" void wsrep_handle_SR_rollback(THD *bf_thd,
   }
 }
 
-extern "C" my_bool wsrep_thd_bf_abort(const THD *bf_thd, THD *victim_thd,
+extern "C" my_bool wsrep_thd_bf_abort(THD *bf_thd, THD *victim_thd,
                                       my_bool signal)
 {
-  /* Note: do not store/reset globals before wsrep_bf_abort() call
-     to avoid losing BF thd context. */
-  if (WSREP(victim_thd) && !victim_thd->wsrep_trx().active())
-  {
-    WSREP_DEBUG("BF abort for non active transaction");
-    wsrep_start_transaction(victim_thd, victim_thd->wsrep_next_trx_id());
-  }
   my_bool ret= wsrep_bf_abort(bf_thd, victim_thd);
-  wsrep_store_threadvars((THD*)bf_thd);
   /*
     Send awake signal if victim was BF aborted or does not
     have wsrep on. Note that this should never interrupt RSU

sql/wsrep_dummy.cc

@@ -115,7 +115,7 @@ const char* wsrep_thd_transaction_state_str(const THD*)
 query_id_t wsrep_thd_transaction_id(const THD *)
 { return 0; }
 
-my_bool wsrep_thd_bf_abort(const THD *, THD *, my_bool)
+my_bool wsrep_thd_bf_abort(THD *, THD *, my_bool)
 { return 0; }
 
 my_bool wsrep_thd_order_before(const THD*, const THD *)

sql/wsrep_mysqld.cc

@@ -2101,12 +2101,18 @@ void wsrep_handle_mdl_conflict(MDL_context *requestor_ctx,
     if (wsrep_thd_is_toi(granted_thd) ||
         wsrep_thd_is_applying(granted_thd))
     {
-      if (wsrep_thd_is_SR(granted_thd) && !wsrep_thd_is_SR(request_thd))
+      if (wsrep_thd_is_aborting(granted_thd))
+      {
+        WSREP_DEBUG("BF thread waiting for SR in aborting state");
+        ticket->wsrep_report(wsrep_debug);
+        mysql_mutex_unlock(&granted_thd->LOCK_thd_data);
+      }
+      else if (wsrep_thd_is_SR(granted_thd) && !wsrep_thd_is_SR(request_thd))
       {
         WSREP_MDL_LOG(INFO, "MDL conflict, DDL vs SR", 
                       schema, schema_len, request_thd, granted_thd);
         mysql_mutex_unlock(&granted_thd->LOCK_thd_data);
-        wsrep_abort_thd((void*)request_thd, (void*)granted_thd, 1);
+        wsrep_abort_thd(request_thd, granted_thd, 1);
       }
       else
       {
@@ -2130,7 +2136,7 @@ void wsrep_handle_mdl_conflict(MDL_context *requestor_ctx,
                   wsrep_thd_transaction_state_str(granted_thd));
       ticket->wsrep_report(wsrep_debug);
       mysql_mutex_unlock(&granted_thd->LOCK_thd_data);
-      wsrep_abort_thd((void*)request_thd, (void*)granted_thd, 1);
+      wsrep_abort_thd(request_thd, granted_thd, 1);
     }
     else
     {

sql/wsrep_thd.cc

@@ -345,7 +345,7 @@ void wsrep_fire_rollbacker(THD *thd)
 }
 
 
-int wsrep_abort_thd(void *bf_thd_ptr, void *victim_thd_ptr, my_bool signal)
+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;
@@ -373,7 +373,7 @@ int wsrep_abort_thd(void *bf_thd_ptr, void *victim_thd_ptr, my_bool signal)
 
 bool wsrep_bf_abort(const THD* bf_thd, THD* victim_thd)
 {
-  WSREP_LOG_THD((THD*)bf_thd, "BF aborter before");
+  WSREP_LOG_THD(bf_thd, "BF aborter before");
   WSREP_LOG_THD(victim_thd, "victim before");
   wsrep::seqno bf_seqno(bf_thd->wsrep_trx().ws_meta().seqno());
 

sql/wsrep_thd.h

@@ -88,8 +88,8 @@ void wsrep_create_appliers(long threads);
 void wsrep_create_rollbacker();
 
 bool wsrep_bf_abort(const THD*, THD*);
-int  wsrep_abort_thd(void *bf_thd_ptr, void *victim_thd_ptr,
-                                my_bool signal);
+int  wsrep_abort_thd(THD *bf_thd_ptr, THD *victim_thd_ptr, my_bool signal);
+
 extern void  wsrep_thd_set_PA_safe(void *thd_ptr, my_bool safe);
 
 /*
@@ -262,7 +262,7 @@ static inline void wsrep_override_error(THD* thd,
    @param message Optional message
    @param function Function where the call was made from
  */
-static inline void wsrep_log_thd(THD *thd,
+static inline void wsrep_log_thd(const THD *thd,
                                  const char *message,
                                  const char *function)
 {