diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2019-03-28 12:27:06 +0200 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2019-03-28 12:39:50 +0200 |
commit | d0116e10a5da52503a89a413e481996ce3f65e63 (patch) | |
tree | e67bbdfa81ee11b205610c3b038e864f3206a0a6 | |
parent | 81d71ee6b21870772c336bff15b71904914f146a (diff) | |
download | mariadb-git-d0116e10a5da52503a89a413e481996ce3f65e63.tar.gz |
Revert MDEV-18464 and MDEV-12009
This reverts commit 21b2fada7ab7f35c898c02d2f918461409cc9c8e
and commit 81d71ee6b21870772c336bff15b71904914f146a.
The MDEV-18464 change introduces a few data race issues. Contrary to
the documentation, the field trx_t::victim is not always being protected
by lock_sys_t::mutex and trx_t::mutex. Most importantly, it seems
that KILL QUERY could wrongly avoid acquiring both mutexes when
invoking lock_trx_handle_wait_low(), in case another thread had
already set trx->victim=true.
We also revert MDEV-12009, because it should depend on the MDEV-18464
fix being present.
-rw-r--r-- | include/mysql/service_wsrep.h | 3 | ||||
-rw-r--r-- | mysql-test/suite/galera/r/galera_kill_applier.result | 4 | ||||
-rw-r--r-- | mysql-test/suite/galera/t/galera_kill_applier.cnf | 10 | ||||
-rw-r--r-- | mysql-test/suite/galera/t/galera_kill_applier.test | 54 | ||||
-rw-r--r-- | sql/sql_parse.cc | 14 | ||||
-rw-r--r-- | sql/sql_plugin_services.ic | 3 | ||||
-rw-r--r-- | sql/wsrep_dummy.cc | 3 | ||||
-rw-r--r-- | sql/wsrep_mysqld.cc | 1 | ||||
-rw-r--r-- | sql/wsrep_thd.cc | 9 | ||||
-rw-r--r-- | sql/wsrep_thd.h | 2 | ||||
-rw-r--r-- | storage/innobase/handler/ha_innodb.cc | 38 | ||||
-rw-r--r-- | storage/innobase/include/trx0trx.h | 19 | ||||
-rw-r--r-- | storage/innobase/lock/lock0lock.cc | 50 | ||||
-rw-r--r-- | storage/innobase/row/row0sel.cc | 4 | ||||
-rw-r--r-- | storage/innobase/trx/trx0roll.cc | 3 | ||||
-rw-r--r-- | storage/innobase/trx/trx0trx.cc | 8 | ||||
-rw-r--r-- | storage/xtradb/handler/ha_innodb.cc | 36 | ||||
-rw-r--r-- | storage/xtradb/include/trx0trx.h | 19 | ||||
-rw-r--r-- | storage/xtradb/lock/lock0lock.cc | 50 | ||||
-rw-r--r-- | storage/xtradb/row/row0sel.cc | 4 | ||||
-rw-r--r-- | storage/xtradb/trx/trx0roll.cc | 21 | ||||
-rw-r--r-- | storage/xtradb/trx/trx0trx.cc | 17 |
22 files changed, 178 insertions, 194 deletions
diff --git a/include/mysql/service_wsrep.h b/include/mysql/service_wsrep.h index 59df7a8b561..267c8cb4e90 100644 --- a/include/mysql/service_wsrep.h +++ b/include/mysql/service_wsrep.h @@ -112,7 +112,6 @@ extern struct wsrep_service_st { int (*wsrep_trx_order_before_func)(MYSQL_THD, MYSQL_THD); void (*wsrep_unlock_rollback_func)(); void (*wsrep_set_data_home_dir_func)(const char *data_dir); - my_bool (*wsrep_thd_is_applier_func)(THD *thd); } *wsrep_service; #ifdef MYSQL_DYNAMIC_PLUGIN @@ -156,7 +155,6 @@ extern struct wsrep_service_st { #define wsrep_trx_order_before(T1,T2) wsrep_service->wsrep_trx_order_before_func(T1,T2) #define wsrep_unlock_rollback() wsrep_service->wsrep_unlock_rollback_func() #define wsrep_set_data_home_dir(A) wsrep_service->wsrep_set_data_home_dir_func(A) -#define wsrep_thd_is_applier(T) wsrep_service->wsrep_thd_is_applier_func(T) #define wsrep_debug get_wsrep_debug() #define wsrep_log_conflicts get_wsrep_log_conflicts() @@ -216,7 +214,6 @@ void wsrep_thd_set_conflict_state(THD *thd, enum wsrep_conflict_state state); bool wsrep_thd_ignore_table(THD *thd); void wsrep_unlock_rollback(); void wsrep_set_data_home_dir(const char *data_dir); -my_bool wsrep_thd_is_applier(THD *thd); #endif diff --git a/mysql-test/suite/galera/r/galera_kill_applier.result b/mysql-test/suite/galera/r/galera_kill_applier.result index 89f8ead0b65..fe4911639ed 100644 --- a/mysql-test/suite/galera/r/galera_kill_applier.result +++ b/mysql-test/suite/galera/r/galera_kill_applier.result @@ -1,8 +1,4 @@ -CREATE USER foo@localhost; -GRANT SELECT on test.* TO foo@localhost; -# Open connection to the 1st node using 'test_user1' user. Got one of the listed errors Got one of the listed errors Got one of the listed errors Got one of the listed errors -DROP USER foo@localhost; diff --git a/mysql-test/suite/galera/t/galera_kill_applier.cnf b/mysql-test/suite/galera/t/galera_kill_applier.cnf deleted file mode 100644 index f2563e66f2e..00000000000 --- a/mysql-test/suite/galera/t/galera_kill_applier.cnf +++ /dev/null @@ -1,10 +0,0 @@ -!include ../galera_2nodes.cnf - -[mysqld.1] -wsrep_provider_options='base_port=@mysqld.1.#galera_port;pc.ignore_sb=true' -auto_increment_offset=1 - -[mysqld.2] -wsrep_provider_options='base_port=@mysqld.2.#galera_port;pc.ignore_sb=true' -auto_increment_offset=2 - diff --git a/mysql-test/suite/galera/t/galera_kill_applier.test b/mysql-test/suite/galera/t/galera_kill_applier.test index 5e4a587fe57..e14a8b9af23 100644 --- a/mysql-test/suite/galera/t/galera_kill_applier.test +++ b/mysql-test/suite/galera/t/galera_kill_applier.test @@ -1,25 +1,14 @@ # # This test checks that applier threads are immune to KILL QUERY and KILL STATEMENT -# when USER is not SUPER # --source include/galera_cluster.inc +--source include/have_innodb.inc --connection node_1 -CREATE USER foo@localhost; -GRANT SELECT on test.* TO foo@localhost; - --let $applier_thread = `SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user' AND STATE != 'wsrep aborter idle' OR STATE IS NULL LIMIT 1` ---let $aborter_thread = `SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user' AND STATE = 'wsrep aborter idle' LIMIT 1` - ---echo # Open connection to the 1st node using 'test_user1' user. ---let $port_1= \$NODE_MYPORT_1 ---connect(foo_node_1,localhost,foo,,test,$port_1,) - ---connection foo_node_1 - --disable_query_log --error ER_KILL_DENIED_ERROR,ER_KILL_DENIED_ERROR --eval KILL $applier_thread @@ -27,48 +16,11 @@ GRANT SELECT on test.* TO foo@localhost; --error ER_KILL_DENIED_ERROR,ER_KILL_DENIED_ERROR --eval KILL QUERY $applier_thread ---error ER_KILL_DENIED_ERROR,ER_KILL_DENIED_ERROR ---eval KILL $aborter_thread - ---error ER_KILL_DENIED_ERROR,ER_KILL_DENIED_ERROR ---eval KILL QUERY $aborter_thread ---enable_query_log - -# -# SUPER can kill applier threads -# ---connection node_2 - ---let $applier_thread = `SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user' AND STATE != 'wsrep aborter idle' OR STATE IS NULL LIMIT 1` - ---disable_query_log ---eval KILL $applier_thread ---enable_query_log - --let $aborter_thread = `SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user' AND STATE = 'wsrep aborter idle' LIMIT 1` ---disable_query_log +--error ER_KILL_DENIED_ERROR,ER_KILL_DENIED_ERROR --eval KILL $aborter_thread ---enable_query_log - ---source include/restart_mysqld.inc ---connection node_2 ---let $applier_thread = `SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user' AND STATE != 'wsrep aborter idle' OR STATE IS NULL LIMIT 1` - ---disable_query_log ---eval KILL QUERY $applier_thread ---enable_query_log - ---let $aborter_thread = `SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'system user' AND STATE = 'wsrep aborter idle' LIMIT 1` - ---disable_query_log +--error ER_KILL_DENIED_ERROR,ER_KILL_DENIED_ERROR --eval KILL QUERY $aborter_thread --enable_query_log - ---source include/restart_mysqld.inc - ---connection node_1 ---disconnect foo_node_1 -DROP USER foo@localhost; - diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 920f7ac0329..1f060305d4f 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -8292,19 +8292,11 @@ kill_one_thread(THD *thd, longlong id, killed_state kill_signal, killed_type typ It's ok to also kill DELAYED threads with KILL_CONNECTION instead of KILL_SYSTEM_THREAD; The difference is that KILL_CONNECTION may be faster and do a harder kill than KILL_SYSTEM_THREAD; - - Note that if thread is wsrep Brute Force or applier thread we - allow killing it only when we're SUPER. */ - if ((thd->security_ctx->master_access & SUPER_ACL) || - (thd->security_ctx->user_matches(tmp->security_ctx) -#ifdef WITH_WSREP - && - !tmp->wsrep_applier && - !wsrep_thd_is_BF(tmp, false) -#endif - )) + if (((thd->security_ctx->master_access & SUPER_ACL) || + thd->security_ctx->user_matches(tmp->security_ctx)) && + !wsrep_thd_is_BF(tmp, false)) { tmp->awake(kill_signal); error=0; diff --git a/sql/sql_plugin_services.ic b/sql/sql_plugin_services.ic index d70c76701fd..427d8937c57 100644 --- a/sql/sql_plugin_services.ic +++ b/sql/sql_plugin_services.ic @@ -181,8 +181,7 @@ static struct wsrep_service_st wsrep_handler = { wsrep_trx_is_aborting, wsrep_trx_order_before, wsrep_unlock_rollback, - wsrep_set_data_home_dir, - wsrep_thd_is_applier, + wsrep_set_data_home_dir }; static struct thd_specifics_service_st thd_specifics_handler= diff --git a/sql/wsrep_dummy.cc b/sql/wsrep_dummy.cc index e2a4dcf3bad..795e2d19252 100644 --- a/sql/wsrep_dummy.cc +++ b/sql/wsrep_dummy.cc @@ -20,9 +20,6 @@ my_bool wsrep_thd_is_BF(THD *, my_bool) { return 0; } -my_bool wsrep_thd_is_applier(THD *) -{ return 0; } - int wsrep_trx_order_before(THD *, THD *) { return 0; } diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc index 82415691b3f..ee8509e3fa2 100644 --- a/sql/wsrep_mysqld.cc +++ b/sql/wsrep_mysqld.cc @@ -2470,6 +2470,7 @@ extern "C" void wsrep_thd_set_exec_mode(THD *thd, enum wsrep_exec_mode mode) thd->wsrep_exec_mode= mode; } + extern "C" void wsrep_thd_set_query_state( THD *thd, enum wsrep_query_state state) { diff --git a/sql/wsrep_thd.cc b/sql/wsrep_thd.cc index 91f0e1e8d8a..551e710cfeb 100644 --- a/sql/wsrep_thd.cc +++ b/sql/wsrep_thd.cc @@ -596,15 +596,6 @@ my_bool wsrep_thd_is_BF(THD *thd, my_bool sync) return status; } -my_bool wsrep_thd_is_applier(THD *thd) -{ - my_bool ret = FALSE; - if (thd) { - ret = thd->wsrep_applier; - } - return ret; -} - extern "C" my_bool wsrep_thd_is_BF_or_commit(void *thd_ptr, my_bool sync) { diff --git a/sql/wsrep_thd.h b/sql/wsrep_thd.h index f5fcb50280c..5900668f3fb 100644 --- a/sql/wsrep_thd.h +++ b/sql/wsrep_thd.h @@ -37,7 +37,6 @@ int wsrep_abort_thd(void *bf_thd_ptr, void *victim_thd_ptr, */ extern void wsrep_thd_set_PA_safe(void *thd_ptr, my_bool safe); extern my_bool wsrep_thd_is_BF(THD *thd, my_bool sync); -extern my_bool wsrep_thd_is_applier(THD *thd); extern my_bool wsrep_thd_is_wsrep(void *thd_ptr); enum wsrep_conflict_state wsrep_thd_conflict_state(void *thd_ptr, my_bool sync); @@ -48,7 +47,6 @@ extern "C" int wsrep_thd_in_locking_session(void *thd_ptr); #else /* WITH_WSREP */ #define wsrep_thd_is_BF(T, S) (0) -#define wsrep_thd_is_applier(T) (0) #define wsrep_abort_thd(X,Y,Z) do { } while(0) #define wsrep_create_appliers(T) do { } while(0) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index c55eaf59879..cd605b6b791 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -4929,6 +4929,8 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels) /* if victim has been signaled by BF thread and/or aborting is already progressing, following query aborting is not necessary any more. + Also, BF thread should own trx mutex for the victim, which would + conflict with trx_mutex_enter() below */ DBUG_VOID_RETURN; } @@ -4937,8 +4939,34 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels) if (trx_t* trx = thd_to_trx(thd)) { ut_ad(trx->mysql_thd == thd); + switch (trx->abort_type) { +#ifdef WITH_WSREP + case TRX_WSREP_ABORT: + break; +#endif + case TRX_SERVER_ABORT: + if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) { + lock_mutex_enter(); + } + /* fall through */ + case TRX_REPLICATION_ABORT: + trx_mutex_enter(trx); + } /* Cancel a pending lock request if there are any */ lock_trx_handle_wait(trx); + switch (trx->abort_type) { +#ifdef WITH_WSREP + case TRX_WSREP_ABORT: + break; +#endif + case TRX_SERVER_ABORT: + if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) { + lock_mutex_exit(); + } + /* fall through */ + case TRX_REPLICATION_ABORT: + trx_mutex_exit(trx); + } } DBUG_VOID_RETURN; @@ -18655,12 +18683,6 @@ wsrep_innobase_kill_one_trx( wsrep_thd_ws_handle(thd)->trx_id); wsrep_thd_LOCK(thd); - - /* We mark this as victim transaction, which is already marked - as BF victim. Both trx mutex and lock_sys mutex is held until - this victim has aborted. */ - victim_trx->victim = true; - DBUG_EXECUTE_IF("sync.wsrep_after_BF_victim_lock", { const char act[]= @@ -18844,7 +18866,7 @@ wsrep_abort_transaction( my_bool signal) { DBUG_ENTER("wsrep_innobase_abort_thd"); - + trx_t* victim_trx = thd_to_trx(victim_thd); trx_t* bf_trx = (bf_thd) ? thd_to_trx(bf_thd) : NULL; @@ -18856,10 +18878,12 @@ wsrep_abort_transaction( if (victim_trx) { lock_mutex_enter(); trx_mutex_enter(victim_trx); + victim_trx->abort_type = TRX_WSREP_ABORT; int rcode = wsrep_innobase_kill_one_trx(bf_thd, bf_trx, victim_trx, signal); trx_mutex_exit(victim_trx); lock_mutex_exit(); + victim_trx->abort_type = TRX_SERVER_ABORT; wsrep_srv_conc_cancel_wait(victim_trx); DBUG_RETURN(rcode); } else { diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h index b40c6ae4667..fe16b8272b8 100644 --- a/storage/innobase/include/trx0trx.h +++ b/storage/innobase/include/trx0trx.h @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2015, 2019, MariaDB Corporation. +Copyright (c) 2015, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -623,6 +623,7 @@ struct trx_lock_t { lock_sys->mutex. Otherwise, this may only be modified by the thread that is serving the running transaction. */ + mem_heap_t* lock_heap; /*!< memory heap for trx_locks; protected by lock_sys->mutex */ @@ -694,6 +695,14 @@ lock_rec_convert_impl_to_expl()) will access transactions associated to other connections. The locks of transactions are protected by lock_sys->mutex and sometimes by trx->mutex. */ +enum trx_abort_t { + TRX_SERVER_ABORT = 0, +#ifdef WITH_WSREP + TRX_WSREP_ABORT, +#endif + TRX_REPLICATION_ABORT +}; + struct trx_t{ ulint magic_n; @@ -871,12 +880,8 @@ struct trx_t{ /*------------------------------*/ THD* mysql_thd; /*!< MySQL thread handle corresponding to this trx, or NULL */ - bool victim; /*!< This transaction is - selected as victim for abort - either by replication or - high priority wsrep thread. This - field is protected by trx and - lock sys mutex. */ + trx_abort_t abort_type; /*!< Transaction abort type*/ + const char* mysql_log_file_name; /*!< if MySQL binlog is used, this field contains a pointer to the latest file diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 33563a03f1a..f06fcd6c4d8 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2014, 2019, MariaDB Corporation. +Copyright (c) 2014, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1793,8 +1793,10 @@ wsrep_kill_victim( } } + lock->trx->abort_type = TRX_WSREP_ABORT; wsrep_innobase_kill_one_trx(trx->mysql_thd, (const trx_t*) trx, lock->trx, TRUE); + lock->trx->abort_type = TRX_SERVER_ABORT; } } } @@ -4780,11 +4782,12 @@ lock_report_waiters_to_mysql( if (w_trx->id != victim_trx_id) { /* If thd_report_wait_for() decides to kill the transaction, then we will get a call back into - innobase_kill_query.*/ - trx_mutex_enter(w_trx); - w_trx->victim = true; + innobase_kill_query. We mark this by setting + current_lock_mutex_owner, so we can avoid trying + to recursively take lock_sys->mutex. */ + w_trx->abort_type = TRX_REPLICATION_ABORT; thd_report_wait_for(mysql_thd, w_trx->mysql_thd); - trx_mutex_exit(w_trx); + w_trx->abort_type = TRX_SERVER_ABORT; } ++i; } @@ -7964,23 +7967,6 @@ lock_trx_release_locks( lock_mutex_exit(); } -inline dberr_t lock_trx_handle_wait_low(trx_t* trx) -{ - ut_ad(lock_mutex_own()); - ut_ad(trx_mutex_own(trx)); - - if (trx->lock.was_chosen_as_deadlock_victim) { - return DB_DEADLOCK; - } - if (!trx->lock.wait_lock) { - /* The lock was probably granted before we got here. */ - return DB_SUCCESS; - } - - lock_cancel_waiting_and_release(trx->lock.wait_lock); - return DB_LOCK_WAIT; -} - /*********************************************************************//** Check whether the transaction has already been rolled back because it was selected as a deadlock victim, or if it has to wait then cancel @@ -7992,19 +7978,19 @@ lock_trx_handle_wait( /*=================*/ trx_t* trx) /*!< in/out: trx lock state */ { - if (!trx->victim) { - lock_mutex_enter(); - trx_mutex_enter(trx); - } - - dberr_t err = lock_trx_handle_wait_low(trx); + ut_ad(lock_mutex_own()); + ut_ad(trx_mutex_own(trx)); - if (!trx->victim) { - lock_mutex_exit(); - trx_mutex_exit(trx); + if (trx->lock.was_chosen_as_deadlock_victim) { + return DB_DEADLOCK; + } + if (!trx->lock.wait_lock) { + /* The lock was probably granted before we got here. */ + return DB_SUCCESS; } - return err; + lock_cancel_waiting_and_release(trx->lock.wait_lock); + return DB_LOCK_WAIT; } /*********************************************************************//** diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index 855266686e6..06bf4cc30c0 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -4746,7 +4746,11 @@ no_gap_lock: a deadlock and the transaction had to wait then release the lock it is waiting on. */ + lock_mutex_enter(); + trx_mutex_enter(trx); err = lock_trx_handle_wait(trx); + lock_mutex_exit(); + trx_mutex_exit(trx); switch (err) { case DB_SUCCESS: diff --git a/storage/innobase/trx/trx0roll.cc b/storage/innobase/trx/trx0roll.cc index 127e834335d..3fd71aff23a 100644 --- a/storage/innobase/trx/trx0roll.cc +++ b/storage/innobase/trx/trx0roll.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2017, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2016, 2019, MariaDB Corporation. +Copyright (c) 2016, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -370,7 +370,6 @@ trx_rollback_to_savepoint_for_mysql_low( trx_mark_sql_stat_end(trx); trx->op_info = ""; - trx->victim = false; return(err); } diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 98ac08a00e7..f36aabba8b4 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2015, 2019, MariaDB Corporation. +Copyright (c) 2015, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1339,7 +1339,11 @@ trx_commit_in_memory( ut_ad(!trx->in_ro_trx_list); ut_ad(!trx->in_rw_trx_list); - trx->victim = false; +#ifdef WITH_WSREP + if (trx->mysql_thd && wsrep_on(trx->mysql_thd)) { + trx->lock.was_chosen_as_deadlock_victim = FALSE; + } +#endif trx->dict_operation = TRX_DICT_OP_NONE; trx->error_state = DB_SUCCESS; diff --git a/storage/xtradb/handler/ha_innodb.cc b/storage/xtradb/handler/ha_innodb.cc index 55c49711f19..a0df23b60d4 100644 --- a/storage/xtradb/handler/ha_innodb.cc +++ b/storage/xtradb/handler/ha_innodb.cc @@ -5534,6 +5534,8 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels) /* if victim has been signaled by BF thread and/or aborting is already progressing, following query aborting is not necessary any more. + Also, BF thread should own trx mutex for the victim, which would + conflict with trx_mutex_enter() below */ DBUG_VOID_RETURN; } @@ -5541,8 +5543,34 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels) if (trx_t* trx = thd_to_trx(thd)) { ut_ad(trx->mysql_thd == thd); + switch (trx->abort_type) { +#ifdef WITH_WSREP + case TRX_WSREP_ABORT: + break; +#endif + case TRX_SERVER_ABORT: + if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) { + lock_mutex_enter(); + } + /* fall through */ + case TRX_REPLICATION_ABORT: + trx_mutex_enter(trx); + } /* Cancel a pending lock request if there are any */ lock_trx_handle_wait(trx); + switch (trx->abort_type) { +#ifdef WITH_WSREP + case TRX_WSREP_ABORT: + break; +#endif + case TRX_SERVER_ABORT: + if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) { + lock_mutex_exit(); + } + /* fall through */ + case TRX_REPLICATION_ABORT: + trx_mutex_exit(trx); + } } DBUG_VOID_RETURN; @@ -19695,12 +19723,6 @@ wsrep_innobase_kill_one_trx( (thd && wsrep_thd_query(thd)) ? wsrep_thd_query(thd) : "void"); wsrep_thd_LOCK(thd); - - /* We mark this as victim transaction, which is already marked - as BF victim. Both trx mutex and lock_sys mutex is held until - this victim has aborted. */ - victim_trx->victim = true; - DBUG_EXECUTE_IF("sync.wsrep_after_BF_victim_lock", { const char act[]= @@ -19889,10 +19911,12 @@ wsrep_abort_transaction(handlerton* hton, THD *bf_thd, THD *victim_thd, if (victim_trx) { lock_mutex_enter(); trx_mutex_enter(victim_trx); + victim_trx->abort_type = TRX_WSREP_ABORT; int rcode = wsrep_innobase_kill_one_trx(bf_thd, bf_trx, victim_trx, signal); trx_mutex_exit(victim_trx); lock_mutex_exit(); + victim_trx->abort_type = TRX_SERVER_ABORT; wsrep_srv_conc_cancel_wait(victim_trx); DBUG_RETURN(rcode); } else { diff --git a/storage/xtradb/include/trx0trx.h b/storage/xtradb/include/trx0trx.h index df01284af05..77afde4c35c 100644 --- a/storage/xtradb/include/trx0trx.h +++ b/storage/xtradb/include/trx0trx.h @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2015, 2019, MariaDB Corporation. +Copyright (c) 2015, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -672,6 +672,7 @@ struct trx_lock_t { lock_sys->mutex. Otherwise, this may only be modified by the thread that is serving the running transaction. */ + mem_heap_t* lock_heap; /*!< memory heap for trx_locks; protected by lock_sys->mutex */ @@ -743,6 +744,14 @@ lock_rec_convert_impl_to_expl()) will access transactions associated to other connections. The locks of transactions are protected by lock_sys->mutex and sometimes by trx->mutex. */ +enum trx_abort_t { + TRX_SERVER_ABORT = 0, +#ifdef WITH_WSREP + TRX_WSREP_ABORT, +#endif + TRX_REPLICATION_ABORT +}; + struct trx_t{ ulint magic_n; @@ -921,12 +930,8 @@ struct trx_t{ /*------------------------------*/ THD* mysql_thd; /*!< MySQL thread handle corresponding to this trx, or NULL */ - bool victim; /*!< This transaction is - selected as victim for abort - either by replication or - high priority wsrep thread. This - field is protected by trx and - lock sys mutex. */ + trx_abort_t abort_type; /*!< Transaction abort type */ + const char* mysql_log_file_name; /*!< if MySQL binlog is used, this field contains a pointer to the latest file diff --git a/storage/xtradb/lock/lock0lock.cc b/storage/xtradb/lock/lock0lock.cc index 2999c339457..9daa2cc906f 100644 --- a/storage/xtradb/lock/lock0lock.cc +++ b/storage/xtradb/lock/lock0lock.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2014, 2019, MariaDB Corporation. +Copyright (c) 2014, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1804,8 +1804,10 @@ wsrep_kill_victim( } } + lock->trx->abort_type = TRX_WSREP_ABORT; wsrep_innobase_kill_one_trx(trx->mysql_thd, (const trx_t*) trx, lock->trx, TRUE); + lock->trx->abort_type = TRX_SERVER_ABORT; } } } @@ -4819,11 +4821,12 @@ lock_report_waiters_to_mysql( if (w_trx->id != victim_trx_id) { /* If thd_report_wait_for() decides to kill the transaction, then we will get a call back into - innobase_kill_query.*/ - trx_mutex_enter(w_trx); - w_trx->victim = true; + innobase_kill_query. We mark this by setting + current_lock_mutex_owner, so we can avoid trying + to recursively take lock_sys->mutex. */ + w_trx->abort_type = TRX_REPLICATION_ABORT; thd_report_wait_for(mysql_thd, w_trx->mysql_thd); - trx_mutex_exit(w_trx); + w_trx->abort_type = TRX_SERVER_ABORT; } ++i; } @@ -8074,23 +8077,6 @@ lock_trx_release_locks( lock_mutex_exit(); } -inline dberr_t lock_trx_handle_wait_low(trx_t* trx) -{ - ut_ad(lock_mutex_own()); - ut_ad(trx_mutex_own(trx)); - - if (trx->lock.was_chosen_as_deadlock_victim) { - return DB_DEADLOCK; - } - if (!trx->lock.wait_lock) { - /* The lock was probably granted before we got here. */ - return DB_SUCCESS; - } - - lock_cancel_waiting_and_release(trx->lock.wait_lock); - return DB_LOCK_WAIT; -} - /*********************************************************************//** Check whether the transaction has already been rolled back because it was selected as a deadlock victim, or if it has to wait then cancel @@ -8102,19 +8088,19 @@ lock_trx_handle_wait( /*=================*/ trx_t* trx) /*!< in/out: trx lock state */ { - if (!trx->victim) { - lock_mutex_enter(); - trx_mutex_enter(trx); - } - - dberr_t err = lock_trx_handle_wait_low(trx); + ut_ad(lock_mutex_own()); + ut_ad(trx_mutex_own(trx)); - if (!trx->victim) { - lock_mutex_exit(); - trx_mutex_exit(trx); + if (trx->lock.was_chosen_as_deadlock_victim) { + return DB_DEADLOCK; + } + if (!trx->lock.wait_lock) { + /* The lock was probably granted before we got here. */ + return DB_SUCCESS; } - return err; + lock_cancel_waiting_and_release(trx->lock.wait_lock); + return DB_LOCK_WAIT; } /*********************************************************************//** diff --git a/storage/xtradb/row/row0sel.cc b/storage/xtradb/row/row0sel.cc index 87bc2aa2875..b6b5d107885 100644 --- a/storage/xtradb/row/row0sel.cc +++ b/storage/xtradb/row/row0sel.cc @@ -4755,7 +4755,11 @@ no_gap_lock: a deadlock and the transaction had to wait then release the lock it is waiting on. */ + lock_mutex_enter(); + trx_mutex_enter(trx); err = lock_trx_handle_wait(trx); + lock_mutex_exit(); + trx_mutex_exit(trx); switch (err) { case DB_SUCCESS: diff --git a/storage/xtradb/trx/trx0roll.cc b/storage/xtradb/trx/trx0roll.cc index 127e834335d..56b7120fa34 100644 --- a/storage/xtradb/trx/trx0roll.cc +++ b/storage/xtradb/trx/trx0roll.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2017, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2016, 2019, MariaDB Corporation. +Copyright (c) 2016, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -33,6 +33,8 @@ Created 3/26/1996 Heikki Tuuri #include "trx0roll.ic" #endif +#include <mysql/service_wsrep.h> + #include "fsp0fsp.h" #include "mach0data.h" #include "trx0rseg.h" @@ -49,6 +51,9 @@ Created 3/26/1996 Heikki Tuuri #include "pars0pars.h" #include "srv0mon.h" #include "trx0sys.h" +#ifdef WITH_WSREP +#include "ha_prototypes.h" +#endif /* WITH_WSREP */ /** This many pages must be undone before a truncate is tried within rollback */ @@ -370,7 +375,13 @@ trx_rollback_to_savepoint_for_mysql_low( trx_mark_sql_stat_end(trx); trx->op_info = ""; - trx->victim = false; + +#ifdef WITH_WSREP + if (wsrep_on(trx->mysql_thd) && + trx->lock.was_chosen_as_deadlock_victim) { + trx->lock.was_chosen_as_deadlock_victim = FALSE; + } +#endif return(err); } @@ -1068,6 +1079,12 @@ trx_roll_try_truncate( if (trx->update_undo) { trx_undo_truncate_end(trx, trx->update_undo, limit); } + +#ifdef WITH_WSREP_OUT + if (wsrep_on(trx->mysql_thd)) { + trx->lock.was_chosen_as_deadlock_victim = FALSE; + } +#endif /* WITH_WSREP */ } /***********************************************************************//** diff --git a/storage/xtradb/trx/trx0trx.cc b/storage/xtradb/trx/trx0trx.cc index 8ca247fadc8..17cba81daf3 100644 --- a/storage/xtradb/trx/trx0trx.cc +++ b/storage/xtradb/trx/trx0trx.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2015, 2019, MariaDB Corporation. +Copyright (c) 2015, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1563,7 +1563,11 @@ trx_commit_in_memory( ut_ad(!trx->in_ro_trx_list); ut_ad(!trx->in_rw_trx_list); - trx->victim = false; +#ifdef WITH_WSREP + if (trx->mysql_thd && wsrep_on(trx->mysql_thd)) { + trx->lock.was_chosen_as_deadlock_victim = FALSE; + } +#endif trx->dict_operation = TRX_DICT_OP_NONE; trx->error_state = DB_SUCCESS; @@ -2664,6 +2668,10 @@ trx_start_if_not_started_low( { switch (trx->state) { case TRX_STATE_NOT_STARTED: +#ifdef WITH_WSREP + ut_d(trx->start_file = __FILE__); + ut_d(trx->start_line = __LINE__); +#endif /* WITH_WSREP */ trx_start_low(trx); /* fall through */ case TRX_STATE_ACTIVE: @@ -2697,6 +2705,11 @@ trx_start_for_ddl_low( trx->will_lock = 1; trx->ddl = true; + +#ifdef WITH_WSREP + ut_d(trx->start_file = __FILE__); + ut_d(trx->start_line = __LINE__); +#endif /* WITH_WSREP */ trx_start_low(trx); return; |