commit 4021bcc136fe673e9c32792c9e9cc27cc9c9379d Author: Sachin Setiya Date: Tue Sep 26 21:31:21 2017 +0530 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. diff --git a/mysql-test/suite/galera/r/galera_mdev_13787.result b/mysql-test/suite/galera/r/galera_mdev_13787.result new file mode 100644 index 0000000..e3133f6 --- /dev/null +++ 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 new file mode 100644 index 0000000..27ec1e3 --- /dev/null +++ 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 new file mode 100644 index 0000000..940cffb --- /dev/null +++ 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 index 33689a9..061f877 100644 --- 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 index 4733c33..6a9831f 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -4110,7 +4110,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 index 8c63157..62bbd57 100644 --- 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 index 3f5489f..cedf083 100644 --- 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 index a447027..4e3de68 100644 --- 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 index 85a4b0b..3d70c93 100644 --- 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 && trx && 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 index caf7a3e..be64b63 100644 --- a/storage/xtradb/handler/ha_innodb.cc +++ b/storage/xtradb/handler/ha_innodb.cc @@ -4621,7 +4621,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 index 923c463..5f8e7b1 100644 --- 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 index 40ab9d9..40511b6 100644 --- 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 index a447027..4e3de68 100644 --- 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 index 91dbc52..c37d33a 100644 --- 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 && trx && wsrep_on_trx(trx) && !(parent && que_node_get_type(parent) == QUE_NODE_UPDATE && ((upd_node_t*)parent)->cascade_node == node) && foreign