MDEV-13787 Crash in persistent stats wsrep_on (thd=0x0)

Problem:- This crash happens because of thd = NULL , and while checking for wsrep_on , we no longer check for thd != NULL (MDEV-7955). So this problem is regression of MDEV-7955. However this patch not only solves this regression , It solves all regression caused by MDEV-7955 patch. To get all possible cases when thd can be null , assert(thd)/ assert(trx->mysql_thd) is place just before all wsrep_on and innodb test suite is run. And the assert which caused failure are removed with a physical check for thd != NULL. Rest assert are removed. Hopefully this method will remove all current/potential regression of MDEV-7955.
MariaDB · Sep 27, 2017 · 0627929 · 0627929
1 parent e3dee83
commit 0627929
Show file tree

Hide file tree

Showing 14 changed files with 56 additions and 19 deletions.
diff --git a/mysql-test/suite/galera/r/galera_mdev_13787.result b/mysql-test/suite/galera/r/galera_mdev_13787.result
@@ -0,0 +1,3 @@
+create table t(a int);
+insert into t select 1;
+DROP TABLE t;
diff --git a/mysql-test/suite/galera/t/galera_mdev_13787.opt b/mysql-test/suite/galera/t/galera_mdev_13787.opt
@@ -0,0 +1 @@
+--innodb-stats-persistent=1
diff --git a/mysql-test/suite/galera/t/galera_mdev_13787.test b/mysql-test/suite/galera/t/galera_mdev_13787.test
@@ -0,0 +1,6 @@
+--source include/galera_cluster.inc
+--source include/have_innodb.inc
+--connection node_1
+create table t(a int);
+insert into t select 1;
+DROP TABLE t;
diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc
@@ -2293,7 +2293,7 @@ static int wsrep_create_sp(THD *thd, uchar** buf, size_t* buf_len)
 }
 
 
-extern int wsrep_on(THD *thd)
+extern inline int wsrep_on(THD *thd)
 {
   return (int)(WSREP(thd));
 }

diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
@@ -4122,7 +4122,7 @@ innobase_commit_low(
 		trx_commit_for_mysql(trx);
 	}
 #ifdef WITH_WSREP
-	if (wsrep_on(thd)) { thd_proc_info(thd, tmp); }
+	if (thd && wsrep_on(thd)) { thd_proc_info(thd, tmp); }
 #endif /* WITH_WSREP */
 }
 

diff --git a/storage/innobase/include/lock0lock.h b/storage/innobase/include/lock0lock.h
@@ -1024,6 +1024,11 @@ std::string
 lock_get_info(
 	const lock_t*);
 
+/*******************************************************************//**
+@return whether wsrep_on is true on trx->mysql_thd*/
+bool
+wsrep_on_trx(const trx_t* trx);
+
 #endif /* WITH_WSREP */
 #ifndef UNIV_NONINL
 #include "lock0lock.ic"

diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
@@ -1825,7 +1825,7 @@ lock_rec_other_has_conflicting(
 
 #ifdef WITH_WSREP
 		if (lock_rec_has_to_wait(TRUE, trx, mode, lock, is_supremum)) {
-			if (wsrep_on(trx->mysql_thd)) {
+			if (wsrep_on_trx(trx)) {
 				trx_mutex_enter(lock->trx);
 				wsrep_kill_victim(trx, lock);
 				trx_mutex_exit(lock->trx);
@@ -2151,7 +2151,7 @@ lock_rec_create(
 
 #ifdef WITH_WSREP
 	if (c_lock                      &&
-	    wsrep_on(trx->mysql_thd)    &&
+	    wsrep_on_trx(trx)           &&
 	    wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
 		lock_t *hash	= (lock_t *)c_lock->hash;
 		lock_t *prev	= NULL;
@@ -8213,3 +8213,11 @@ lock_get_info(
 
 	return info;
 }
+
+#ifdef WITH_WSREP
+bool inline
+wsrep_on_trx(const trx_t* trx)
+{
+    return trx->mysql_thd && wsrep_on(trx->mysql_thd);
+}
+#endif /* WITH_WSREP */
diff --git a/storage/innobase/lock/lock0wait.cc b/storage/innobase/lock/lock0wait.cc
@@ -197,7 +197,7 @@ wsrep_is_BF_lock_timeout(
 /*====================*/
     trx_t* trx) /* in: trx to check for lock priority */
 {
-       if (wsrep_on(trx->mysql_thd) &&
+       if (wsrep_on_trx(trx) &&
            wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
                fprintf(stderr, "WSREP: BF lock wait long\n");
                 srv_print_innodb_monitor       = TRUE;
@@ -402,7 +402,7 @@ lock_wait_suspend_thread(
 	if (lock_wait_timeout < 100000000
 	    && wait_time > (double) lock_wait_timeout) {
 #ifdef WITH_WSREP
-                if (!wsrep_on(trx->mysql_thd) ||
+                if (!wsrep_on_trx(trx) ||
                     (!wsrep_is_BF_lock_timeout(trx) &&
                      trx->error_state != DB_DEADLOCK)) {
 #endif /* WITH_WSREP */

diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc
@@ -1983,7 +1983,7 @@ row_upd_sec_index_entry(
 			}
 #ifdef WITH_WSREP
 			if (err == DB_SUCCESS && !referenced                  &&
-			    wsrep_on(trx->mysql_thd)                          &&
+			    wsrep_on_trx(trx)                                 &&
 			    !wsrep_thd_is_BF(trx->mysql_thd, FALSE)           &&
 			    !(parent && que_node_get_type(parent) ==
 				QUE_NODE_UPDATE                               &&
@@ -2270,7 +2270,7 @@ row_upd_clust_rec_by_insert(
 			}
 		}
 #ifdef WITH_WSREP
-		if (!referenced && wsrep_on(trx->mysql_thd)                  &&
+		if (!referenced && wsrep_on_trx(trx) &&
 		    !(parent && que_node_get_type(parent) == QUE_NODE_UPDATE &&
 		      ((upd_node_t*)parent)->cascade_node == node)           &&
 		    foreign
@@ -2536,8 +2536,7 @@ row_upd_del_mark_clust_rec(
 	}
 #ifdef WITH_WSREP
 	trx_t* trx = thr_get_trx(thr) ;
-
-	if (err == DB_SUCCESS && !referenced && trx && wsrep_on(trx->mysql_thd) &&
+	if (err == DB_SUCCESS && !referenced && wsrep_on_trx(trx) &&
 	    !(parent && que_node_get_type(parent) == QUE_NODE_UPDATE &&
 	      ((upd_node_t*)parent)->cascade_node == node)           &&
 	    foreign

diff --git a/storage/xtradb/handler/ha_innodb.cc b/storage/xtradb/handler/ha_innodb.cc
@@ -4633,7 +4633,7 @@ innobase_commit_low(
 		trx_commit_for_mysql(trx);
 	}
 #ifdef WITH_WSREP
-	if (wsrep_on(thd)) { thd_proc_info(thd, tmp); }
+	if (thd && wsrep_on(thd)) { thd_proc_info(thd, tmp); }
 #endif /* WITH_WSREP */
 }
 

diff --git a/storage/xtradb/include/lock0lock.h b/storage/xtradb/include/lock0lock.h
@@ -1028,6 +1028,14 @@ Get lock mode and table/index name
 std::string
 lock_get_info(
 	const lock_t*);
+#ifdef WITH_WSREP
+
+/*******************************************************************//**
+@return whether wsrep_on is true on trx->mysql_thd*/
+bool
+wsrep_on_trx(const trx_t* trx);
+
+#endif /* WITH_WSREP */
 
 #ifndef UNIV_NONINL
 #include "lock0lock.ic"

diff --git a/storage/xtradb/lock/lock0lock.cc b/storage/xtradb/lock/lock0lock.cc
@@ -1835,7 +1835,7 @@ lock_rec_other_has_conflicting(
 
 #ifdef WITH_WSREP
 		if (lock_rec_has_to_wait(TRUE, trx, mode, lock, is_supremum)) {
-			if (wsrep_on(trx->mysql_thd)) {
+			if (wsrep_on_trx(trx)) {
 				trx_mutex_enter(lock->trx);
 				wsrep_kill_victim(trx, lock);
 				trx_mutex_exit(lock->trx);
@@ -2290,7 +2290,7 @@ lock_rec_create(
 
 #ifdef WITH_WSREP
 	if (c_lock                      &&
-	    wsrep_on(trx->mysql_thd)    &&
+	    wsrep_on_trx(trx)           &&
 	    wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
 		lock_t *hash	= (lock_t *)c_lock->hash;
 		lock_t *prev	= NULL;
@@ -8336,3 +8336,11 @@ lock_get_info(
 
 	return info;
 }
+
+#ifdef WITH_WSREP
+bool inline
+wsrep_on_trx(const trx_t* trx)
+{
+    return trx->mysql_thd && wsrep_on(trx->mysql_thd);
+}
+#endif /* WITH_WSREP */
diff --git a/storage/xtradb/lock/lock0wait.cc b/storage/xtradb/lock/lock0wait.cc
@@ -197,7 +197,7 @@ wsrep_is_BF_lock_timeout(
 /*====================*/
     trx_t* trx) /* in: trx to check for lock priority */
 {
-       if (wsrep_on(trx->mysql_thd) &&
+       if (wsrep_on_trx(trx) &&
            wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
                fprintf(stderr, "WSREP: BF lock wait long\n");
                 srv_print_innodb_monitor       = TRUE;
@@ -402,7 +402,7 @@ lock_wait_suspend_thread(
 	if (lock_wait_timeout < 100000000
 	    && wait_time > (double) lock_wait_timeout) {
 #ifdef WITH_WSREP
-                if (!wsrep_on(trx->mysql_thd) ||
+                if (!wsrep_on_trx(trx) ||
                     (!wsrep_is_BF_lock_timeout(trx) &&
                      trx->error_state != DB_DEADLOCK)) {
 #endif /* WITH_WSREP */

diff --git a/storage/xtradb/row/row0upd.cc b/storage/xtradb/row/row0upd.cc
@@ -1989,7 +1989,7 @@ row_upd_sec_index_entry(
 			}
 #ifdef WITH_WSREP
 			if (err == DB_SUCCESS && !referenced                  &&
-			    wsrep_on(trx->mysql_thd)                          &&
+			    wsrep_on_trx(trx)                                 &&
 			    !wsrep_thd_is_BF(trx->mysql_thd, FALSE)           &&
 			    !(parent && que_node_get_type(parent) ==
 				QUE_NODE_UPDATE                               &&
@@ -2279,7 +2279,7 @@ row_upd_clust_rec_by_insert(
 			}
 		}
 #ifdef WITH_WSREP
-		if (!referenced && wsrep_on(trx->mysql_thd)                  &&
+		if (!referenced && wsrep_on_trx(trx) &&
 		    !(parent && que_node_get_type(parent) == QUE_NODE_UPDATE &&
 		      ((upd_node_t*)parent)->cascade_node == node)           &&
 		    foreign
@@ -2548,8 +2548,7 @@ row_upd_del_mark_clust_rec(
 	}
 #ifdef WITH_WSREP
 	trx_t* trx = thr_get_trx(thr) ;
-
-	if (err == DB_SUCCESS && !referenced && trx && wsrep_on(trx->mysql_thd) &&
+	if (err == DB_SUCCESS && !referenced && wsrep_on_trx(trx) &&
 	    !(parent && que_node_get_type(parent) == QUE_NODE_UPDATE &&
 	      ((upd_node_t*)parent)->cascade_node == node)           &&
 	    foreign