diff --git a/mysql-test/main/xa.result b/mysql-test/main/xa.result index 46bfa6e962a..f77c0afdec5 100644 --- a/mysql-test/main/xa.result +++ b/mysql-test/main/xa.result @@ -296,20 +296,21 @@ DROP TABLE t1; # ASSERTION THD->TRANSACTION.XID_STATE.XID.IS_NULL() # FAILED # -DROP TABLE IF EXISTS t1, t2; CREATE TABLE t1 (a INT) ENGINE=InnoDB; CREATE TABLE t2 (a INT) ENGINE=InnoDB; -START TRANSACTION; -INSERT INTO t1 VALUES (1); +INSERT INTO t2 VALUES (1); +COMMIT; +BEGIN; +INSERT INTO t2 VALUES (2); +UPDATE t2 SET a=a+1; connect con2,localhost,root; XA START 'xid1'; +INSERT INTO t1 VALUES (1); # Sending: -INSERT INTO t2 SELECT a FROM t1; +DELETE FROM t2; connection default; -# Waiting until INSERT ... is blocked DELETE FROM t1; connection con2; -# Reaping: INSERT INTO t2 SELECT a FROM t1 ERROR 40001: Deadlock found when trying to get lock; try restarting transaction XA COMMIT 'xid1'; ERROR XA102: XA_RBDEADLOCK: Transaction branch was rolled back: deadlock was detected diff --git a/mysql-test/main/xa.test b/mysql-test/main/xa.test index b5ef5a118b1..176ef6aa760 100644 --- a/mysql-test/main/xa.test +++ b/mysql-test/main/xa.test @@ -393,33 +393,30 @@ DROP TABLE t1; --echo # FAILED --echo # ---disable_warnings -DROP TABLE IF EXISTS t1, t2; ---enable_warnings - CREATE TABLE t1 (a INT) ENGINE=InnoDB; CREATE TABLE t2 (a INT) ENGINE=InnoDB; -START TRANSACTION; -INSERT INTO t1 VALUES (1); +INSERT INTO t2 VALUES (1); COMMIT; +BEGIN; +INSERT INTO t2 VALUES (2); +UPDATE t2 SET a=a+1; --connect (con2,localhost,root) XA START 'xid1'; +INSERT INTO t1 VALUES (1); --echo # Sending: ---send INSERT INTO t2 SELECT a FROM t1 +--send DELETE FROM t2 --connection default let $wait_condition= SELECT COUNT(*) = 1 FROM information_schema.processlist - WHERE state = "Sending data" - AND info = "INSERT INTO t2 SELECT a FROM t1"; ---echo # Waiting until INSERT ... is blocked + WHERE state = "Updating" + AND info = "DELETE FROM t2"; --source include/wait_condition.inc --sleep 0.1 -DELETE FROM t1; +--send DELETE FROM t1 --connection con2 ---echo # Reaping: INSERT INTO t2 SELECT a FROM t1 --error ER_LOCK_DEADLOCK --reap --error ER_XA_RBDEADLOCK @@ -427,6 +424,7 @@ XA COMMIT 'xid1'; connection default; +reap; COMMIT; connection con2; diff --git a/mysql-test/suite/innodb/r/innodb_information_schema.result b/mysql-test/suite/innodb/r/innodb_information_schema.result index c1625f2bc3c..766d5f47c2d 100644 --- a/mysql-test/suite/innodb/r/innodb_information_schema.result +++ b/mysql-test/suite/innodb/r/innodb_information_schema.result @@ -45,7 +45,7 @@ trx_last_foreign_key_error varchar(256) YES NULL trx_is_read_only int(1) NO 0 trx_autocommit_non_locking int(1) NO 0 trx_state trx_weight trx_tables_in_use trx_tables_locked trx_rows_locked trx_rows_modified trx_concurrency_tickets trx_isolation_level trx_unique_checks trx_foreign_key_checks -RUNNING 4 0 1 7 1 0 REPEATABLE READ 1 1 +RUNNING 3 0 1 5 1 0 REPEATABLE READ 1 1 trx_isolation_level trx_unique_checks trx_foreign_key_checks SERIALIZABLE 0 0 trx_state trx_isolation_level trx_last_foreign_key_error diff --git a/mysql-test/suite/innodb/r/monitor.result b/mysql-test/suite/innodb/r/monitor.result index 2700479e7f7..a40bfdac0d0 100644 --- a/mysql-test/suite/innodb/r/monitor.result +++ b/mysql-test/suite/innodb/r/monitor.result @@ -662,7 +662,21 @@ SELECT NAME, COUNT > 0 FROM INFORMATION_SCHEMA.INNODB_METRICS WHERE NAME LIKE 'buffer_page_written_index_leaf'; NAME COUNT > 0 buffer_page_written_index_leaf 1 +DROP TABLE t1; +CREATE TABLE t1(id INT PRIMARY KEY, a INT, b CHAR(1), UNIQUE KEY u(a,b)) +ENGINE=InnoDB; +SET @start = (SELECT COUNT FROM INFORMATION_SCHEMA.INNODB_METRICS WHERE NAME += 'lock_rec_lock_created'); +BEGIN; +INSERT INTO t1 VALUES(1,1,'a'),(2,9999,'b'),(3,10000,'c'),(4,4,'d'); +DELETE FROM t1 WHERE a = 9999 AND b='b'; +COMMIT; +SET @end = (SELECT COUNT FROM INFORMATION_SCHEMA.INNODB_METRICS WHERE NAME += 'lock_rec_lock_created'); +SELECT @end - @start; +@end - @start +0 +DROP TABLE t1; SET GLOBAL innodb_monitor_enable=default; SET GLOBAL innodb_monitor_disable=default; SET GLOBAL innodb_monitor_reset_all=default; -DROP TABLE t1; diff --git a/mysql-test/suite/innodb/t/monitor.test b/mysql-test/suite/innodb/t/monitor.test index dfae93694bf..3535c9c85ad 100644 --- a/mysql-test/suite/innodb/t/monitor.test +++ b/mysql-test/suite/innodb/t/monitor.test @@ -422,10 +422,27 @@ UNLOCK TABLES; SELECT NAME, COUNT > 0 FROM INFORMATION_SCHEMA.INNODB_METRICS WHERE NAME LIKE 'buffer_page_written_index_leaf'; +DROP TABLE t1; + +CREATE TABLE t1(id INT PRIMARY KEY, a INT, b CHAR(1), UNIQUE KEY u(a,b)) +ENGINE=InnoDB; + +SET @start = (SELECT COUNT FROM INFORMATION_SCHEMA.INNODB_METRICS WHERE NAME += 'lock_rec_lock_created'); + +BEGIN; +INSERT INTO t1 VALUES(1,1,'a'),(2,9999,'b'),(3,10000,'c'),(4,4,'d'); +DELETE FROM t1 WHERE a = 9999 AND b='b'; +COMMIT; + +SET @end = (SELECT COUNT FROM INFORMATION_SCHEMA.INNODB_METRICS WHERE NAME += 'lock_rec_lock_created'); +SELECT @end - @start; + +DROP TABLE t1; + --disable_warnings SET GLOBAL innodb_monitor_enable=default; SET GLOBAL innodb_monitor_disable=default; SET GLOBAL innodb_monitor_reset_all=default; --enable_warnings - -DROP TABLE t1; diff --git a/storage/innobase/include/lock0lock.h b/storage/innobase/include/lock0lock.h index 2159f56d018..e4b8947ac3a 100644 --- a/storage/innobase/include/lock0lock.h +++ b/storage/innobase/include/lock0lock.h @@ -789,19 +789,21 @@ const lock_t* lock_trx_has_sys_table_locks( /*=========================*/ const trx_t* trx) /*!< in: transaction to check */ - MY_ATTRIBUTE((warn_unused_result)); + MY_ATTRIBUTE((nonnull, warn_unused_result)); -/*******************************************************************//** -Check if the transaction holds an exclusive lock on a record. -@return whether the locks are held */ +/** Check if the transaction holds an explicit exclusive lock on a record. +@param[in] trx transaction +@param[in] table table +@param[in] block leaf page +@param[in] heap_no heap number identifying the record +@return whether an explicit X-lock is held */ bool -lock_trx_has_rec_x_lock( -/*====================*/ +lock_trx_has_expl_x_lock( const trx_t* trx, /*!< in: transaction to check */ const dict_table_t* table, /*!< in: table to check */ const buf_block_t* block, /*!< in: buffer block of the record */ ulint heap_no)/*!< in: record heap number */ - MY_ATTRIBUTE((warn_unused_result)); + MY_ATTRIBUTE((nonnull, warn_unused_result)); #endif /* UNIV_DEBUG */ /** diff --git a/storage/innobase/include/trx0sys.h b/storage/innobase/include/trx0sys.h index ea01d698b3b..69374ab3fba 100644 --- a/storage/innobase/include/trx0sys.h +++ b/storage/innobase/include/trx0sys.h @@ -616,7 +616,7 @@ public: @retval pointer to trx */ - trx_t *find(trx_t *caller_trx, trx_id_t trx_id, bool do_ref_count= false) + trx_t *find(trx_t *caller_trx, trx_id_t trx_id, bool do_ref_count) { /* In MariaDB 10.3, purge will reset DB_TRX_ID to 0 @@ -624,9 +624,10 @@ public: always have a nonzero trx_t::id; there the value 0 is reserved for transactions that did not write or lock anything yet. + + The caller should already have handled trx_id==0 specially. */ - if (!trx_id) - return NULL; + ut_ad(trx_id); if (caller_trx && caller_trx->id == trx_id) { if (do_ref_count) @@ -1044,13 +1045,13 @@ public: bool is_registered(trx_t *caller_trx, trx_id_t id) { - return rw_trx_hash.find(caller_trx, id); + return id && find(caller_trx, id, false); } - trx_t *find(trx_t *caller_trx, trx_id_t id) + trx_t *find(trx_t *caller_trx, trx_id_t id, bool do_ref_count= true) { - return rw_trx_hash.find(caller_trx, id, true); + return rw_trx_hash.find(caller_trx, id, do_ref_count); } diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 4987d60dd5a..cd65a9ac8e0 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -4937,8 +4937,12 @@ lock_rec_queue_validate( /* Unlike the non-debug code, this invariant can only succeed if the check and assertion are covered by the lock mutex. */ - const trx_t *impl_trx = trx_sys.rw_trx_hash.find(current_trx(), - lock_clust_rec_some_has_impl(rec, index, offsets)); + const trx_id_t impl_trx_id = lock_clust_rec_some_has_impl( + rec, index, offsets); + + const trx_t *impl_trx = impl_trx_id + ? trx_sys.find(current_trx(), impl_trx_id, false) + : 0; ut_ad(lock_mutex_own()); /* impl_trx cannot be committed until lock_mutex_exit() @@ -5547,18 +5551,31 @@ static void lock_rec_other_trx_holds_expl(trx_t *caller_trx, trx_t *trx, #endif /* UNIV_DEBUG */ -/*********************************************************************//** -If a transaction has an implicit x-lock on a record, but no explicit x-lock -set on the record, sets one for it. */ +/** If an implicit x-lock exists on a record, convert it to an explicit one. + +Often, this is called by a transaction that is about to enter a lock wait +due to the lock conflict. Two explicit locks would be created: first the +exclusive lock on behalf of the lock-holder transaction in this function, +and then a wait request on behalf of caller_trx, in the calling function. + +This may also be called by the same transaction that is already holding +an implicit exclusive lock on the record. In this case, no explicit lock +should be created. + +@param[in,out] caller_trx current transaction +@param[in] block index tree leaf page +@param[in] rec record on the leaf page +@param[in] index the index of the record +@param[in] offsets rec_get_offsets(rec,index) +@return whether caller_trx already holds an exclusive lock on rec */ static -void +bool lock_rec_convert_impl_to_expl( -/*==========================*/ - trx_t* caller_trx,/*!id)) { + return true; + } + trx = trx_sys.find(caller_trx, trx_id); } else { ut_ad(!dict_index_is_online_ddl(index)); trx = lock_sec_rec_some_has_impl(caller_trx, rec, index, offsets); + if (trx == caller_trx) { + trx->release_reference(); + return true; + } ut_d(lock_rec_other_trx_holds_expl(caller_trx, trx, rec, block)); @@ -5597,6 +5625,8 @@ lock_rec_convert_impl_to_expl( lock_rec_convert_impl_to_expl_for_trx( block, rec, index, trx, heap_no); } + + return false; } /*********************************************************************//** @@ -5641,8 +5671,11 @@ lock_clust_rec_modify_check_and_lock( /* If a transaction has no explicit x-lock set on the record, set one for it */ - lock_rec_convert_impl_to_expl(thr_get_trx(thr), block, rec, index, - offsets); + if (lock_rec_convert_impl_to_expl(thr_get_trx(thr), block, rec, index, + offsets)) { + /* We already hold an implicit exclusive lock. */ + return DB_SUCCESS; + } err = lock_rec_lock(TRUE, LOCK_X | LOCK_REC_NOT_GAP, block, heap_no, index, thr); @@ -5786,10 +5819,11 @@ lock_sec_rec_read_check_and_lock( database recovery is running. */ if (!page_rec_is_supremum(rec) - && page_get_max_trx_id(block->frame) >= trx_sys.get_min_trx_id()) { - - lock_rec_convert_impl_to_expl(thr_get_trx(thr), block, rec, - index, offsets); + && page_get_max_trx_id(block->frame) >= trx_sys.get_min_trx_id() + && lock_rec_convert_impl_to_expl(thr_get_trx(thr), block, rec, + index, offsets)) { + /* We already hold an implicit exclusive lock. */ + return DB_SUCCESS; } err = lock_rec_lock(FALSE, ulint(mode) | gap_mode, @@ -5850,10 +5884,11 @@ lock_clust_rec_read_check_and_lock( heap_no = page_rec_get_heap_no(rec); - if (heap_no != PAGE_HEAP_NO_SUPREMUM) { - - lock_rec_convert_impl_to_expl(thr_get_trx(thr), block, rec, - index, offsets); + if (heap_no != PAGE_HEAP_NO_SUPREMUM + && lock_rec_convert_impl_to_expl(thr_get_trx(thr), block, rec, + index, offsets)) { + /* We already hold an implicit exclusive lock. */ + return DB_SUCCESS; } err = lock_rec_lock(FALSE, ulint(mode) | gap_mode, @@ -6560,12 +6595,14 @@ lock_trx_has_sys_table_locks( return(strongest_lock); } -/*******************************************************************//** -Check if the transaction holds an exclusive lock on a record. -@return whether the locks are held */ +/** Check if the transaction holds an explicit exclusive lock on a record. +@param[in] trx transaction +@param[in] table table +@param[in] block leaf page +@param[in] heap_no heap number identifying the record +@return whether an explicit X-lock is held */ bool -lock_trx_has_rec_x_lock( -/*====================*/ +lock_trx_has_expl_x_lock( const trx_t* trx, /*!< in: transaction to check */ const dict_table_t* table, /*!< in: table to check */ const buf_block_t* block, /*!< in: buffer block of the record */ @@ -6574,11 +6611,9 @@ lock_trx_has_rec_x_lock( ut_ad(heap_no > PAGE_HEAP_NO_SUPREMUM); lock_mutex_enter(); - ut_a(lock_table_has(trx, table, LOCK_IX) - || table->is_temporary()); - ut_a(lock_rec_has_expl(LOCK_X | LOCK_REC_NOT_GAP, - block, heap_no, trx) - || table->is_temporary()); + ut_ad(lock_table_has(trx, table, LOCK_IX)); + ut_ad(lock_rec_has_expl(LOCK_X | LOCK_REC_NOT_GAP, block, heap_no, + trx)); lock_mutex_exit(); return(true); } diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc index 39206e66d75..447132dcf86 100644 --- a/storage/innobase/row/row0upd.cc +++ b/storage/innobase/row/row0upd.cc @@ -3093,9 +3093,7 @@ row_upd_clust_step( ulint mode; - DEBUG_SYNC_C_IF_THD( - thr_get_trx(thr)->mysql_thd, - "innodb_row_upd_clust_step_enter"); + DEBUG_SYNC_C_IF_THD(trx->mysql_thd, "innodb_row_upd_clust_step_enter"); if (dict_index_is_online_ddl(index)) { ut_ad(node->table->id != DICT_INDEXES_ID); @@ -3157,10 +3155,11 @@ row_upd_clust_step( } } - ut_ad(index->table->no_rollback() - || lock_trx_has_rec_x_lock(thr_get_trx(thr), index->table, - btr_pcur_get_block(pcur), - page_rec_get_heap_no(rec))); + ut_ad(index->table->no_rollback() || index->table->is_temporary() + || row_get_rec_trx_id(rec, index, offsets) == trx->id + || lock_trx_has_expl_x_lock(trx, index->table, + btr_pcur_get_block(pcur), + page_rec_get_heap_no(rec))); /* NOTE: the following function calls will also commit mtr */ diff --git a/storage/innobase/row/row0vers.cc b/storage/innobase/row/row0vers.cc index bfaa2721746..81f9c795a7c 100644 --- a/storage/innobase/row/row0vers.cc +++ b/storage/innobase/row/row0vers.cc @@ -126,14 +126,22 @@ row_vers_impl_x_locked_low( DBUG_RETURN(0); } - trx_t* trx = trx_sys.find(caller_trx, trx_id); + trx_t* trx; - if (trx == 0) { - /* The transaction that modified or inserted clust_rec is no - longer active, or it is corrupt: no implicit lock on rec */ - lock_check_trx_id_sanity(trx_id, clust_rec, clust_index, clust_offsets); - mem_heap_free(heap); - DBUG_RETURN(0); + if (trx_id == caller_trx->id) { + trx = caller_trx; + trx->reference(); + } else { + trx = trx_sys.find(caller_trx, trx_id); + if (trx == 0) { + /* The transaction that modified or inserted + clust_rec is no longer active, or it is + corrupt: no implicit lock on rec */ + lock_check_trx_id_sanity(trx_id, clust_rec, + clust_index, clust_offsets); + mem_heap_free(heap); + DBUG_RETURN(0); + } } comp = page_rec_is_comp(rec); diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 11d83c96a92..c13ccdf53ed 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -761,7 +761,7 @@ trx_lists_init_at_db_start() for (undo = UT_LIST_GET_FIRST(rseg->undo_list); undo != NULL; undo = UT_LIST_GET_NEXT(undo_list, undo)) { - trx_t *trx = trx_sys.rw_trx_hash.find(0, undo->trx_id); + trx_t *trx = trx_sys.find(0, undo->trx_id, false); if (!trx) { trx_resurrect(undo, rseg, start_time, &rows_to_undo, false);