From 18fa00a54cab7cc2b9453d42b8ebc038fd3d07bd Mon Sep 17 00:00:00 2001 From: Vlad Lesin Date: Thu, 12 Oct 2023 12:33:03 +0300 Subject: [PATCH] MDEV-32272 lock_release_on_prepare_try() does not release lock if supremum bit is set along with other bits set in lock's bitmap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The error is caused by MDEV-30165 fix with the following commit: d13a57ae8181f2a8fbee86838d5476740e050d50 There is logical error in lock_release_on_prepare_try(): if (supremum_bit) lock_rec_unlock_supremum(*cell, lock); else lock_rec_dequeue_from_page(lock, false); Because there can be other bits set in the lock's bitmap, and the lock type can be suitable for releasing criteria, but the above logic releases only supremum bit of the lock. The fix is to release lock if it suits for releasing criteria and unlock supremum if supremum is locked otherwise. Tere is also the test for the case, which was reported by QA team. I placed it in a separate files, because it requires debug build. Reviewed by: Marko Mäkelä --- .../r/xa_prepare_reset_supremum_lock.result | 13 ++++++++ ...p_release_locks_on_dict_stats_table.result | 17 ++++++++++ .../t/xa_prepare_reset_supremum_lock.test | 23 +++++++++++-- ...xap_release_locks_on_dict_stats_table.test | 33 +++++++++++++++++++ storage/innobase/dict/dict0stats.cc | 11 +++++++ storage/innobase/lock/lock0lock.cc | 10 +++--- 6 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 mysql-test/suite/innodb/r/xap_release_locks_on_dict_stats_table.result create mode 100644 mysql-test/suite/innodb/t/xap_release_locks_on_dict_stats_table.test diff --git a/mysql-test/suite/innodb/r/xa_prepare_reset_supremum_lock.result b/mysql-test/suite/innodb/r/xa_prepare_reset_supremum_lock.result index 475363444e9..bd971adf966 100644 --- a/mysql-test/suite/innodb/r/xa_prepare_reset_supremum_lock.result +++ b/mysql-test/suite/innodb/r/xa_prepare_reset_supremum_lock.result @@ -14,6 +14,19 @@ XA PREPARE '1'; connect con1,localhost,root; SET innodb_lock_wait_timeout=1; INSERT INTO t VALUES(50); +connection default; +XA COMMIT '1'; +XA START '1'; +SELECT * FROM t WHERE a > 20 LOCK IN SHARE MODE; +a +40 +50 +INSERT INTO t VALUES (5); +XA END '1'; +XA PREPARE '1'; +connection con1; +INSERT INTO t VALUES (60); +INSERT INTO t VALUES (30); disconnect con1; connection default; XA COMMIT '1'; diff --git a/mysql-test/suite/innodb/r/xap_release_locks_on_dict_stats_table.result b/mysql-test/suite/innodb/r/xap_release_locks_on_dict_stats_table.result new file mode 100644 index 00000000000..1a849f1c477 --- /dev/null +++ b/mysql-test/suite/innodb/r/xap_release_locks_on_dict_stats_table.result @@ -0,0 +1,17 @@ +call mtr.add_suppression("bytes freed by"); +SET @old_innodb_stats_persistent = @@innodb_stats_persistent; +SET GLOBAL innodb_stats_persistent=1; +CREATE TABLE t ENGINE=InnoDB AS SELECT 1; +SET @old_debug_dbug = @@global.debug_dbug; +XA START 'a'; +INSERT INTO mysql.innodb_index_stats SELECT '','' AS table_name,index_name,LAST_UPDATE,stat_name,0 AS stat_value,sample_size,stat_description FROM mysql.innodb_index_stats WHERE table_name='dummy' FOR UPDATE; +SET GLOBAL debug_dbug = "+d,dict_stats_save_exit_notify"; +INSERT INTO t VALUES (1); +XA END 'a'; +XA PREPARE 'a'; +SET DEBUG_SYNC="now WAIT_FOR dict_stats_save_finished"; +SET @@global.debug_dbug = @old_debug_dbug; +SET DEBUG_SYNC="RESET"; +SET GLOBAL innodb_stats_persistent = @old_innodb_stats_persistent; +XA COMMIT 'a'; +DROP TABLE t; diff --git a/mysql-test/suite/innodb/t/xa_prepare_reset_supremum_lock.test b/mysql-test/suite/innodb/t/xa_prepare_reset_supremum_lock.test index d285f6f4f3a..180e44d05ae 100644 --- a/mysql-test/suite/innodb/t/xa_prepare_reset_supremum_lock.test +++ b/mysql-test/suite/innodb/t/xa_prepare_reset_supremum_lock.test @@ -13,19 +13,36 @@ INSERT INTO t VALUES(20); SET TRANSACTION ISOLATION LEVEL REPEATABLE READ; XA START '1'; SELECT * FROM t WHERE a > 20 FOR UPDATE; +# The following INSERT is necessary because trx_prepare() resets locks +# only if there were modifications in transaction. INSERT INTO t VALUES(40); XA END '1'; XA PREPARE '1'; connect (con1,localhost,root); SET innodb_lock_wait_timeout=1; -# This will be finished with lock wait timeout error if XA PREPARE did not -# reset lock on supremum +# The following INSERT must not be blocked if XA PREPARE released supremum lock INSERT INTO t VALUES(50); + +--connection default +XA COMMIT '1'; + +XA START '1'; +SELECT * FROM t WHERE a > 20 LOCK IN SHARE MODE; +# The following INSERT is necessary because trx_prepare() resets locks +# only if there were modifications in transaction. +INSERT INTO t VALUES (5); +XA END '1'; +XA PREPARE '1'; + +--connection con1 +# The following INSERT must not be blocked if XA PREPARE released supremum lock +INSERT INTO t VALUES (60); +# The following INSERT must not be blocked if XA PREPARE released shared lock +INSERT INTO t VALUES (30); --disconnect con1 --connection default XA COMMIT '1'; DROP TABLE t; - --source include/wait_until_count_sessions.inc diff --git a/mysql-test/suite/innodb/t/xap_release_locks_on_dict_stats_table.test b/mysql-test/suite/innodb/t/xap_release_locks_on_dict_stats_table.test new file mode 100644 index 00000000000..a02a032ef61 --- /dev/null +++ b/mysql-test/suite/innodb/t/xap_release_locks_on_dict_stats_table.test @@ -0,0 +1,33 @@ +--source include/have_innodb.inc +--source include/have_debug.inc +--source include/have_debug_sync.inc + +# Some memory is allocated in dict_stats_save() function for additional sync +# point. The memory is allocated in current_thd->mem_root pool after +# dict_stats_func() arranged new thd and freed after destroy_background_thd() +# attached background thread thd to the current_thd. That's there are +# different thread id's for memory allocation and deallocation, what causes +# the following warnings. This is not an error because the memory is still +# allocated and deallocated by the same thread in pool. +call mtr.add_suppression("bytes freed by"); + +SET @old_innodb_stats_persistent = @@innodb_stats_persistent; +SET GLOBAL innodb_stats_persistent=1; +CREATE TABLE t ENGINE=InnoDB AS SELECT 1; + +SET @old_debug_dbug = @@global.debug_dbug; + +XA START 'a'; +INSERT INTO mysql.innodb_index_stats SELECT '','' AS table_name,index_name,LAST_UPDATE,stat_name,0 AS stat_value,sample_size,stat_description FROM mysql.innodb_index_stats WHERE table_name='dummy' FOR UPDATE; # Note the SELECT is empty +SET GLOBAL debug_dbug = "+d,dict_stats_save_exit_notify"; +INSERT INTO t VALUES (1); +XA END 'a'; +XA PREPARE 'a'; + +# Locking queue validation will crash the server if the bug is not fixed +SET DEBUG_SYNC="now WAIT_FOR dict_stats_save_finished"; +SET @@global.debug_dbug = @old_debug_dbug; +SET DEBUG_SYNC="RESET"; +SET GLOBAL innodb_stats_persistent = @old_innodb_stats_persistent; +XA COMMIT 'a'; +DROP TABLE t; diff --git a/storage/innobase/dict/dict0stats.cc b/storage/innobase/dict/dict0stats.cc index e8dd976f8d2..c553b31ef01 100644 --- a/storage/innobase/dict/dict0stats.cc +++ b/storage/innobase/dict/dict0stats.cc @@ -34,6 +34,8 @@ Created Jan 06, 2010 Vasil Dimov #include "log.h" #include "btr0btr.h" #include "que0que.h" +#include "scope.h" +#include "debug_sync.h" #include #include @@ -3245,6 +3247,15 @@ dict_stats_save( char db_utf8[MAX_DB_UTF8_LEN]; char table_utf8[MAX_TABLE_UTF8_LEN]; +#ifdef ENABLED_DEBUG_SYNC + DBUG_EXECUTE_IF("dict_stats_save_exit_notify", + SCOPE_EXIT([] { + debug_sync_set_action(current_thd, + STRING_WITH_LEN("now SIGNAL dict_stats_save_finished")); + }); + ); +#endif /* ENABLED_DEBUG_SYNC */ + if (high_level_read_only) { return DB_READ_ONLY; } diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index e7638e8f3ba..31e02d2451a 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -4345,17 +4345,19 @@ static bool lock_release_on_prepare_try(trx_t *trx) { ut_ad(!lock->index->table->is_temporary()); bool supremum_bit = lock_rec_get_nth_bit(lock, PAGE_HEAP_NO_SUPREMUM); - if (!supremum_bit && lock->is_rec_granted_exclusive_not_gap()) + bool rec_granted_exclusive_not_gap = + lock->is_rec_granted_exclusive_not_gap(); + if (!supremum_bit && rec_granted_exclusive_not_gap) continue; auto &lock_hash= lock_sys.hash_get(lock->type_mode); auto cell= lock_hash.cell_get(lock->un_member.rec_lock.page_id.fold()); auto latch= lock_sys_t::hash_table::latch(cell); if (latch->try_acquire()) { - if (supremum_bit) - lock_rec_unlock_supremum(*cell, lock); - else + if (!rec_granted_exclusive_not_gap) lock_rec_dequeue_from_page(lock, false); + else if (supremum_bit) + lock_rec_unlock_supremum(*cell, lock); latch->release(); } else