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.
MariaDB · Dec 4, 2019 · aab6cef · aab6cef
1 parent 0b8b11b
commit aab6cef
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 23 deletions.
diff --git a/include/mysql/service_wsrep.h b/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 */

diff --git a/include/thr_lock.h b/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,

diff --git a/mysql-test/suite/galera_sr/disabled.def b/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
-
diff --git a/sql/service_wsrep.cc b/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

diff --git a/sql/wsrep_dummy.cc b/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 *)

diff --git a/sql/wsrep_mysqld.cc b/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
     {

diff --git a/sql/wsrep_thd.cc b/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());
 

diff --git a/sql/wsrep_thd.h b/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)
 {