From 368529a1782e6de5a47c2c71d72b8b1a3b3cd08f Mon Sep 17 00:00:00 2001 From: Vlad Lesin Date: Thu, 5 Aug 2021 20:18:37 +0300 Subject: MDEV-26206 gap lock is not set if implicit lock exists Move check if table is locked from implicit to explicit lock conversion function to the higher level. This can increase performace by avoiding per-record check and doing per-row check instead. This is experimental commit, if testing is passed and perfomance influence is good, the commit can be squashed with the previous one. --- storage/innobase/include/lock0lock.h | 14 ++++++++++++++ storage/innobase/lock/lock0lock.cc | 23 +++++++++++++++++++---- storage/innobase/row/row0ins.cc | 6 +++++- storage/innobase/row/row0upd.cc | 6 +++++- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/storage/innobase/include/lock0lock.h b/storage/innobase/include/lock0lock.h index 9b83cf6ee1a..74b9cf50956 100644 --- a/storage/innobase/include/lock0lock.h +++ b/storage/innobase/include/lock0lock.h @@ -1040,6 +1040,20 @@ lock_rtr_move_rec_list( moved */ ulint num_move); /*!< in: num of rec to move */ +/*********************************************************************//** +Checks if a transaction has the specified table lock, or stronger. This +function should only be called by the thread that owns the transaction. +The function is the same as lock_table_has(), but lock_table_has() is +supposed to be private for lock0lock.cc, and if we try to make it public and +move it in lock0lock.ic, we also need to move a half of lock0priv.ic. So +making some wrap is the less evil. +@param[in] trx transaction +@param[in] table table +@param[in] in_mode lock_mode +@return lock or NULL */ +const lock_t *lock_table_locked(const trx_t *trx, const dict_table_t *table, + lock_mode in_mode); + #include "lock0lock.ic" #endif diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 9d527e95795..1078b5dc728 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -63,6 +63,23 @@ extern "C" int thd_need_wait_reports(const MYSQL_THD thd); extern "C" int thd_need_ordering_with(const MYSQL_THD thd, const MYSQL_THD other_thd); #endif +/*********************************************************************//** +Checks if a transaction has the specified table lock, or stronger. This +function should only be called by the thread that owns the transaction. +The function is the same as lock_table_has(), but lock_table_has() is +supposed to be private for lock0lock.cc, and if we try to make it public and +move it in lock0lock.ic, we also need to move a half of lock0priv.ic. So +making some wrap is the less evil. +@param[in] trx transaction +@param[in] table table +@param[in] in_mode lock_mode +@return lock or NULL */ +const lock_t *lock_table_locked(const trx_t *trx, const dict_table_t *table, + lock_mode in_mode) +{ + return lock_table_has(trx, table, in_mode); +} + /** Functor for accessing the embedded node within a table lock. */ struct TableLockGetNode { @@ -5206,8 +5223,7 @@ lock_sec_rec_read_check_and_lock( database recovery is running. */ trx_t *trx = thr_get_trx(thr); - if (!lock_table_has(trx, index->table, LOCK_X) - && !page_rec_is_supremum(rec) + 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(trx, id, rec, index, offsets) @@ -5293,8 +5309,7 @@ lock_clust_rec_read_check_and_lock( heap_no = page_rec_get_heap_no(rec); trx_t *trx = thr_get_trx(thr); - if (!lock_table_has(trx, index->table, LOCK_X) - && heap_no != PAGE_HEAP_NO_SUPREMUM + if (heap_no != PAGE_HEAP_NO_SUPREMUM && lock_rec_convert_impl_to_expl(trx, id, rec, index, offsets) && gap_mode == LOCK_REC_NOT_GAP ) { diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc index 4312e95d110..ffa819aede6 100644 --- a/storage/innobase/row/row0ins.cc +++ b/storage/innobase/row/row0ins.cc @@ -3189,7 +3189,9 @@ row_ins_clust_index_entry( const bool skip_locking = wsrep_thd_skip_locking(thr_get_trx(thr)->mysql_thd); ulint flags = index->table->no_rollback() ? BTR_NO_ROLLBACK - : (index->table->is_temporary() || skip_locking) + : (index->table->is_temporary() + || lock_table_locked(thr_get_trx(thr), index->table, LOCK_X) + || skip_locking) ? BTR_NO_LOCKING_FLAG : 0; #ifdef UNIV_DEBUG if (skip_locking && strcmp(wsrep_get_sr_table_name(), @@ -3204,6 +3206,7 @@ row_ins_clust_index_entry( #else ulint flags = index->table->no_rollback() ? BTR_NO_ROLLBACK : index->table->is_temporary() + || lock_table_locked(thr_get_trx(thr), index->table, LOCK_X) ? BTR_NO_LOCKING_FLAG : 0; #endif /* WITH_WSREP */ const ulint orig_n_fields = entry->n_fields; @@ -3290,6 +3293,7 @@ row_ins_sec_index_entry( log_free_check(); ulint flags = index->table->is_temporary() + || lock_table_locked(thr_get_trx(thr), index->table, LOCK_X) ? BTR_NO_LOCKING_FLAG : 0; diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc index ee88ee7a4a6..d90a028123e 100644 --- a/storage/innobase/row/row0upd.cc +++ b/storage/innobase/row/row0upd.cc @@ -1939,6 +1939,9 @@ row_upd_sec_index_entry( break; } + if (lock_table_locked(thr_get_trx(thr), index->table, LOCK_X)) + flags |= BTR_NO_LOCKING_FLAG; + bool uncommitted = !index->is_committed(); if (uncommitted) { @@ -2722,7 +2725,8 @@ row_upd_clust_step( mtr.start(); - if (node->table->is_temporary()) { + if (node->table->is_temporary() + || lock_table_locked(trx, index->table, LOCK_X)) { /* Disable locking, because temporary tables are private to the connection (no concurrent access). */ flags = node->table->no_rollback() -- cgit v1.2.1