From b82abc71639661c642b9d753393be0f85e09c101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 15 Jan 2025 16:55:01 +0200 Subject: [PATCH] MDEV-35701 trx_t::autoinc_locks causes unnecessary dynamic memory allocation trx_t::autoinc_locks: Use small_vector in order to avoid any dynamic memory allocation in the most common case (a statement is holding AUTO_INCREMENT locks on at most 4 tables or partitions). lock_cancel_waiting_and_release(): Instead of removing elements from the middle, simply assign nullptr, like lock_table_remove_autoinc_lock(). The added test innodb.auto_increment_lock_mode covers the dynamic memory allocation as well as nondeterministically (occasionally) covers the out-of-order lock release in lock_table_remove_autoinc_lock(). Reviewed by: Debarun Banerjee --- .../innodb/r/auto_increment_lock_mode.result | 43 +++++ .../innodb/r/innodb-autoinc-56228.result | 35 ++-- .../t/auto_increment_lock_mode.combinations | 6 + .../innodb/t/auto_increment_lock_mode.test | 61 +++++++ .../suite/innodb/t/innodb-autoinc-56228.test | 36 ++-- storage/innobase/dict/drop.cc | 2 +- storage/innobase/handler/ha_innodb.cc | 2 +- storage/innobase/include/small_vector.h | 65 +++++-- storage/innobase/include/trx0trx.h | 14 +- storage/innobase/lock/lock0lock.cc | 158 +++++++----------- storage/innobase/mtr/mtr0mtr.cc | 4 +- storage/innobase/trx/trx0trx.cc | 28 +--- 12 files changed, 272 insertions(+), 182 deletions(-) create mode 100644 mysql-test/suite/innodb/r/auto_increment_lock_mode.result create mode 100644 mysql-test/suite/innodb/t/auto_increment_lock_mode.combinations create mode 100644 mysql-test/suite/innodb/t/auto_increment_lock_mode.test diff --git a/mysql-test/suite/innodb/r/auto_increment_lock_mode.result b/mysql-test/suite/innodb/r/auto_increment_lock_mode.result new file mode 100644 index 00000000000..b19d5cf40fc --- /dev/null +++ b/mysql-test/suite/innodb/r/auto_increment_lock_mode.result @@ -0,0 +1,43 @@ +CREATE TABLE t1(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t2(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t3(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t4(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t5(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t6(a SERIAL, b INT) ENGINE=InnoDB; +CREATE FUNCTION p1() RETURNS INT +BEGIN +INSERT INTO t1() VALUES(); +INSERT INTO t2() VALUES(); +INSERT INTO t3() VALUES(); +INSERT INTO t4() VALUES(); +INSERT INTO t5() VALUES(); +RETURN 1; +END$$ +INSERT INTO t6(b) SELECT p1(); +UPDATE t1,t2,t3,t4,t5 SET t1.a=2,t2.a=2,t3.a=2,t4.a=2,t5.a=2; +SELECT * FROM t1; +a +2 +connect con1,localhost,root,,; +connect con2,localhost,root,,; +connect con3,localhost,root,,; +connection con1; +INSERT INTO t6(b) SELECT SLEEP(p1()); +connection con2; +INSERT INTO t6(b) SELECT SLEEP(p1()); +connection con3; +UPDATE t1,t2,t3,t4,t5 SET t1.a=0,t2.a=0,t3.a=0,t4.a=0,t5.a=0 +WHERE t1.a=2 AND t2.a=2 AND t3.a=2 AND t4.a=2 AND t5.a=2; +connection default; +KILL QUERY $ID1; +KILL QUERY $ID2; +KILL QUERY $ID3; +connection con1; +disconnect con1; +connection con2; +disconnect con2; +connection con3; +disconnect con3; +connection default; +DROP FUNCTION p1; +DROP TABLE t1,t2,t3,t4,t5,t6; diff --git a/mysql-test/suite/innodb/r/innodb-autoinc-56228.result b/mysql-test/suite/innodb/r/innodb-autoinc-56228.result index 6a3fd85ebd3..f8616772151 100644 --- a/mysql-test/suite/innodb/r/innodb-autoinc-56228.result +++ b/mysql-test/suite/innodb/r/innodb-autoinc-56228.result @@ -1,15 +1,6 @@ -DROP TABLE IF EXISTS t1_56228; -Warnings: -Note 1051 Unknown table 'test.t1_56228' -DROP TABLE IF EXISTS t2_56228; -Warnings: -Note 1051 Unknown table 'test.t2_56228' -DROP FUNCTION IF EXISTS bug56228; -Warnings: -Note 1305 FUNCTION test.bug56228 does not exist -CREATE TEMPORARY TABLE t1_56228( +CREATE TABLE t1_56228( c1 iNT AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; -CREATE TEMPORARY TABLE t2_56228( +CREATE TABLE t2_56228( c1 iNT AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; CREATE FUNCTION bug56228() RETURNS INT DETERMINISTIC BEGIN @@ -17,14 +8,18 @@ INSERT INTO t1_56228 VALUES(NULL); INSERT INTO t2_56228 VALUES(NULL); INSERT INTO t1_56228 VALUES(NULL); INSERT INTO t2_56228 VALUES(NULL); -DROP TEMPORARY TABLE t1_56228; +DROP TABLE t1_56228; RETURN 42; END // -SELECT bug56228(); -bug56228() -42 -DROP FUNCTION bug56228; -DROP TEMPORARY TABLE t2_56228; -DROP TEMPORARY TABLE IF EXISTS t1_56228; -Warnings: -Note 1051 Unknown table 'test.t1_56228' +ERROR HY000: Explicit or implicit commit is not allowed in stored function or trigger +CREATE PROCEDURE bug56228() +BEGIN +INSERT INTO t1_56228 VALUES(NULL); +INSERT INTO t2_56228 VALUES(NULL); +INSERT INTO t1_56228 VALUES(NULL); +INSERT INTO t2_56228 VALUES(NULL); +DROP TABLE t1_56228; +END // +CALL bug56228(); +DROP PROCEDURE bug56228; +DROP TABLE t2_56228; diff --git a/mysql-test/suite/innodb/t/auto_increment_lock_mode.combinations b/mysql-test/suite/innodb/t/auto_increment_lock_mode.combinations new file mode 100644 index 00000000000..efaee252a4e --- /dev/null +++ b/mysql-test/suite/innodb/t/auto_increment_lock_mode.combinations @@ -0,0 +1,6 @@ +[old] +--innodb-autoinc-lock-mode=0 +[new] +--innodb-autoinc-lock-mode=1 +[none] +--innodb-autoinc-lock-mode=2 diff --git a/mysql-test/suite/innodb/t/auto_increment_lock_mode.test b/mysql-test/suite/innodb/t/auto_increment_lock_mode.test new file mode 100644 index 00000000000..27497960e72 --- /dev/null +++ b/mysql-test/suite/innodb/t/auto_increment_lock_mode.test @@ -0,0 +1,61 @@ +--source include/have_innodb.inc + +CREATE TABLE t1(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t2(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t3(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t4(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t5(a TINYINT UNSIGNED AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; +CREATE TABLE t6(a SERIAL, b INT) ENGINE=InnoDB; + +DELIMITER $$; +CREATE FUNCTION p1() RETURNS INT +BEGIN + INSERT INTO t1() VALUES(); + INSERT INTO t2() VALUES(); + INSERT INTO t3() VALUES(); + INSERT INTO t4() VALUES(); + INSERT INTO t5() VALUES(); + RETURN 1; +END$$ +DELIMITER ;$$ + +INSERT INTO t6(b) SELECT p1(); + +UPDATE t1,t2,t3,t4,t5 SET t1.a=2,t2.a=2,t3.a=2,t4.a=2,t5.a=2; +SELECT * FROM t1; +--source include/count_sessions.inc + +--connect(con1,localhost,root,,) +let $ID1= `SELECT @id := CONNECTION_ID()`; +--connect(con2,localhost,root,,) +let $ID2= `SELECT @id := CONNECTION_ID()`; +--connect(con3,localhost,root,,) +let $ID3= `SELECT @id := CONNECTION_ID()`; +--connection con1 +send INSERT INTO t6(b) SELECT SLEEP(p1()); +--connection con2 +send INSERT INTO t6(b) SELECT SLEEP(p1()); +--connection con3 +send UPDATE t1,t2,t3,t4,t5 SET t1.a=0,t2.a=0,t3.a=0,t4.a=0,t5.a=0 +WHERE t1.a=2 AND t2.a=2 AND t3.a=2 AND t4.a=2 AND t5.a=2; +--connection default +evalp KILL QUERY $ID1; +evalp KILL QUERY $ID2; +evalp KILL QUERY $ID3; +--connection con1 +--error 0,ER_QUERY_INTERRUPTED,ER_AUTOINC_READ_FAILED +--reap +--disconnect con1 +--connection con2 +--error 0,ER_QUERY_INTERRUPTED,ER_AUTOINC_READ_FAILED +--reap +--disconnect con2 +--connection con3 +--error 0,ER_QUERY_INTERRUPTED,ER_AUTOINC_READ_FAILED +--reap +--disconnect con3 +--connection default + +DROP FUNCTION p1; +DROP TABLE t1,t2,t3,t4,t5,t6; +--source include/wait_until_count_sessions.inc diff --git a/mysql-test/suite/innodb/t/innodb-autoinc-56228.test b/mysql-test/suite/innodb/t/innodb-autoinc-56228.test index a02f7b4383a..e1d8741c730 100644 --- a/mysql-test/suite/innodb/t/innodb-autoinc-56228.test +++ b/mysql-test/suite/innodb/t/innodb-autoinc-56228.test @@ -2,33 +2,45 @@ ## # Bug #56228: dropping tables from within an active statement crashes server # -DROP TABLE IF EXISTS t1_56228; -DROP TABLE IF EXISTS t2_56228; -DROP FUNCTION IF EXISTS bug56228; -CREATE TEMPORARY TABLE t1_56228( +# This test used to use TEMPORARY TABLE, which before MySQL 5.7 or +# MariaDB Server 10.2 were covered by InnoDB locks. +# In MariaDB Server 10.6, the locking and logging was corrected for Atomic DDL. +# Hence, even if we tweaked create_table_info_t::innobase_table_flags() +# so that TEMPORARY TABLE are created as persistent tables, +# the DROP TEMPORARY TABLE statement inside the function would +# fail due to HA_ERR_LOCK_WAIT_TIMEOUT, instead of breaking locks +# like it used to do before MDEV-26603 and possibly other changes. +CREATE TABLE t1_56228( c1 iNT AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; -CREATE TEMPORARY TABLE t2_56228( +CREATE TABLE t2_56228( c1 iNT AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB; DELIMITER //; +--error ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG CREATE FUNCTION bug56228() RETURNS INT DETERMINISTIC BEGIN INSERT INTO t1_56228 VALUES(NULL); INSERT INTO t2_56228 VALUES(NULL); INSERT INTO t1_56228 VALUES(NULL); INSERT INTO t2_56228 VALUES(NULL); - DROP TEMPORARY TABLE t1_56228; + DROP TABLE t1_56228; RETURN 42; END // +CREATE PROCEDURE bug56228() +BEGIN + INSERT INTO t1_56228 VALUES(NULL); + INSERT INTO t2_56228 VALUES(NULL); + INSERT INTO t1_56228 VALUES(NULL); + INSERT INTO t2_56228 VALUES(NULL); + DROP TABLE t1_56228; +END // + DELIMITER ;// ---disable_ps_protocol -SELECT bug56228(); ---enable_ps2_protocol +CALL bug56228(); -DROP FUNCTION bug56228; -DROP TEMPORARY TABLE t2_56228; -DROP TEMPORARY TABLE IF EXISTS t1_56228; +DROP PROCEDURE bug56228; +DROP TABLE t2_56228; diff --git a/storage/innobase/dict/drop.cc b/storage/innobase/dict/drop.cc index f46b4b04752..80bf8c97ead 100644 --- a/storage/innobase/dict/drop.cc +++ b/storage/innobase/dict/drop.cc @@ -247,7 +247,7 @@ void trx_t::commit(std::vector &deleted) mutex_lock(); lock_release_on_drop(this); ut_ad(UT_LIST_GET_LEN(lock.trx_locks) == 0); - ut_ad(ib_vector_is_empty(autoinc_locks)); + ut_ad(autoinc_locks.empty()); mem_heap_empty(lock.lock_heap); lock.table_locks.clear(); /* commit_persist() already reset this. */ diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 4fedd79790b..31c4f525adf 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -5097,7 +5097,7 @@ static void innobase_kill_query(handlerton*, THD *thd, enum thd_kill_levels) may be executed as part of a multi-transaction DDL operation, such as rollback_inplace_alter_table() or ha_innobase::delete_table(). */; trx->error_state= DB_INTERRUPTED; - lock_sys_t::cancel(trx, lock); + lock_sys.cancel(trx, lock); } lock_sys.deadlock_check(); } diff --git a/storage/innobase/include/small_vector.h b/storage/innobase/include/small_vector.h index d28a36184b8..2acdc49f668 100644 --- a/storage/innobase/include/small_vector.h +++ b/storage/innobase/include/small_vector.h @@ -31,14 +31,14 @@ protected: small_vector_base()= delete; small_vector_base(void *small, size_t small_size) : BeginX(small), Capacity(Size_T(small_size)) {} - ATTRIBUTE_COLD void grow_by_1(void *small, size_t element_size); + ATTRIBUTE_COLD void grow_by_1(void *small, size_t element_size) noexcept; public: - size_t size() const { return Size; } - size_t capacity() const { return Capacity; } - bool empty() const { return !Size; } - void clear() { Size= 0; } + size_t size() const noexcept { return Size; } + size_t capacity() const noexcept { return Capacity; } + bool empty() const noexcept { return !Size; } + void clear() noexcept { Size= 0; } protected: - void set_size(size_t N) { Size= Size_T(N); } + void set_size(size_t N) noexcept { Size= Size_T(N); } }; template @@ -49,7 +49,7 @@ class small_vector : public small_vector_base using small_vector_base::set_size; - void grow_if_needed() + void grow_if_needed() noexcept { if (unlikely(size() >= capacity())) grow_by_1(small, sizeof *small); @@ -60,38 +60,67 @@ public: { TRASH_ALLOC(small, sizeof small); } - ~small_vector() + ~small_vector() noexcept { if (small != begin()) my_free(begin()); MEM_MAKE_ADDRESSABLE(small, sizeof small); } + void fake_defined() const noexcept + { + ut_ad(empty()); + MEM_MAKE_DEFINED(small, sizeof small); + } + void make_undefined() const noexcept { MEM_UNDEFINED(small, sizeof small); } + + bool is_small() const noexcept { return small == BeginX; } + + void deep_clear() noexcept + { + if (!is_small()) + { + my_free(BeginX); + BeginX= small; + Capacity= N; + } + ut_ad(capacity() == N); + set_size(0); + } + using iterator= T *; using const_iterator= const T *; using reverse_iterator= std::reverse_iterator; using reference= T &; using const_reference= const T&; - iterator begin() { return static_cast(BeginX); } - const_iterator begin() const { return static_cast(BeginX); } - iterator end() { return begin() + size(); } - const_iterator end() const { return begin() + size(); } + iterator begin() noexcept { return static_cast(BeginX); } + const_iterator begin() const noexcept + { return static_cast(BeginX); } + iterator end() noexcept { return begin() + size(); } + const_iterator end() const noexcept { return begin() + size(); } - reverse_iterator rbegin() { return reverse_iterator(end()); } - reverse_iterator rend() { return reverse_iterator(begin()); } + reverse_iterator rbegin() noexcept { return reverse_iterator(end()); } + reverse_iterator rend() noexcept { return reverse_iterator(begin()); } - reference operator[](size_t i) { assert(i < size()); return begin()[i]; } - const_reference operator[](size_t i) const + reference operator[](size_t i) noexcept + { assert(i < size()); return begin()[i]; } + const_reference operator[](size_t i) const noexcept { return const_cast(*this)[i]; } - void erase(const_iterator S, const_iterator E) + void erase(const_iterator S, const_iterator E) noexcept { set_size(std::move(const_cast(E), end(), const_cast(S)) - begin()); } - void emplace_back(T &&arg) + void emplace_back(T &&arg) noexcept + { + grow_if_needed(); + ::new (end()) T(arg); + set_size(size() + 1); + } + void emplace_back(T &arg) noexcept { grow_if_needed(); ::new (end()) T(arg); diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h index 9a1c5edd43f..d79aafb1ced 100644 --- a/storage/innobase/include/trx0trx.h +++ b/storage/innobase/include/trx0trx.h @@ -32,10 +32,10 @@ Created 3/26/1996 Heikki Tuuri #include "que0types.h" #include "mem0mem.h" #include "trx0xa.h" -#include "ut0vec.h" #include "fts0fts.h" #include "read0types.h" #include "ilist.h" +#include "small_vector.h" #include @@ -857,12 +857,10 @@ public: ulint n_autoinc_rows; /*!< no. of AUTO-INC rows required for an SQL statement. This is useful for multi-row INSERTs */ - ib_vector_t* autoinc_locks; /* AUTOINC locks held by this - transaction. Note that these are - also in the lock list trx_locks. This - vector needs to be freed explicitly - when the trx instance is destroyed. - Protected by lock_sys.latch. */ + typedef small_vector autoinc_lock_vector; + /** AUTO_INCREMENT locks held by this transaction; a subset of trx_locks, + protected by lock_sys.latch. */ + autoinc_lock_vector autoinc_locks; /*------------------------------*/ bool read_only; /*!< true if transaction is flagged as a READ-ONLY transaction. @@ -1059,7 +1057,7 @@ public: ut_ad(!lock.wait_lock); ut_ad(UT_LIST_GET_LEN(lock.trx_locks) == 0); ut_ad(lock.table_locks.empty()); - ut_ad(!autoinc_locks || ib_vector_is_empty(autoinc_locks)); + ut_ad(autoinc_locks.empty()); ut_ad(UT_LIST_GET_LEN(lock.evicted_tables) == 0); ut_ad(!dict_operation); ut_ad(!apply_online_log); diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 5a6857678fc..5e6ed1502af 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -2302,7 +2302,7 @@ static void lock_grant(lock_t *lock) dict_table_t *table= lock->un_member.tab_lock.table; ut_ad(!table->autoinc_trx); table->autoinc_trx= trx; - ib_vector_push(trx->autoinc_locks, &lock); + trx->autoinc_locks.emplace_back(lock); } DBUG_PRINT("ib_lock", ("wait for trx " TRX_ID_FMT " ends", trx->id)); @@ -3646,7 +3646,7 @@ lock_t *lock_table_create(dict_table_t *table, unsigned type_mode, trx_t *trx, ut_ad(!table->autoinc_trx); table->autoinc_trx = trx; - ib_vector_push(trx->autoinc_locks, &lock); + trx->autoinc_locks.emplace_back(lock); goto allocated; } @@ -3695,79 +3695,45 @@ allocated: return(lock); } -/*************************************************************//** -Pops autoinc lock requests from the transaction's autoinc_locks. We -handle the case where there are gaps in the array and they need to -be popped off the stack. */ -UNIV_INLINE -void -lock_table_pop_autoinc_locks( -/*=========================*/ - trx_t* trx) /*!< in/out: transaction that owns the AUTOINC locks */ +/** Release a granted AUTO_INCREMENT lock. +@param lock AUTO_INCREMENT lock +@param trx transaction that owns the lock */ +static void lock_table_remove_autoinc_lock(lock_t *lock, trx_t *trx) noexcept { - ut_ad(!ib_vector_is_empty(trx->autoinc_locks)); + ut_ad(lock->type_mode == (LOCK_AUTO_INC | LOCK_TABLE)); + lock_sys.assert_locked(*lock->un_member.tab_lock.table); + ut_ad(trx->mutex_is_owner()); - /* Skip any gaps, gaps are NULL lock entries in the - trx->autoinc_locks vector. */ + auto begin= trx->autoinc_locks.begin(), end= trx->autoinc_locks.end(), i=end; + ut_ad(begin != end); + if (*--i == lock) + { + /* Normally, the last acquired lock is released first, in order to + avoid unnecessary traversal of trx->autoinc_locks, which + only stores granted locks. */ - do { - ib_vector_pop(trx->autoinc_locks); + /* We remove the last lock, as well as any nullptr entries + immediately preceding it, which might have been created by the + "else" branch below, or by lock_cancel_waiting_and_release(). */ + while (begin != i && !i[-1]) i--; + trx->autoinc_locks.erase(i, end); + } + else + { + ut_a(*i); + /* Clear the lock when it is not the last one. */ + while (begin != i) + { + if (*--i == lock) + { + *i= nullptr; + return; + } + } - if (ib_vector_is_empty(trx->autoinc_locks)) { - return; - } - - } while (*(lock_t**) ib_vector_get_last(trx->autoinc_locks) == NULL); -} - -/*************************************************************//** -Removes an autoinc lock request from the transaction's autoinc_locks. */ -UNIV_INLINE -void -lock_table_remove_autoinc_lock( -/*===========================*/ - lock_t* lock, /*!< in: table lock */ - trx_t* trx) /*!< in/out: transaction that owns the lock */ -{ - ut_ad(lock->type_mode == (LOCK_AUTO_INC | LOCK_TABLE)); - lock_sys.assert_locked(*lock->un_member.tab_lock.table); - ut_ad(trx->mutex_is_owner()); - - auto s = ib_vector_size(trx->autoinc_locks); - ut_ad(s); - - /* With stored functions and procedures the user may drop - a table within the same "statement". This special case has - to be handled by deleting only those AUTOINC locks that were - held by the table being dropped. */ - - lock_t* autoinc_lock = *static_cast( - ib_vector_get(trx->autoinc_locks, --s)); - - /* This is the default fast case. */ - - if (autoinc_lock == lock) { - lock_table_pop_autoinc_locks(trx); - } else { - /* The last element should never be NULL */ - ut_a(autoinc_lock != NULL); - - /* Handle freeing the locks from within the stack. */ - - while (s) { - autoinc_lock = *static_cast( - ib_vector_get(trx->autoinc_locks, --s)); - - if (autoinc_lock == lock) { - void* null_var = NULL; - ib_vector_set(trx->autoinc_locks, s, &null_var); - return; - } - } - - /* Must find the autoinc lock. */ - ut_error; - } + /* The lock must exist. */ + ut_error; + } } /*************************************************************//** @@ -3799,14 +3765,7 @@ lock_table_remove_low( ut_ad((table->autoinc_trx == trx) == !lock->is_waiting()); if (table->autoinc_trx == trx) { - table->autoinc_trx = NULL; - /* The locks must be freed in the reverse order from - the one in which they were acquired. This is to avoid - traversing the AUTOINC lock vector unnecessarily. - - We only store locks that were granted in the - trx->autoinc_locks vector (see lock_table_create() - and lock_grant()). */ + table->autoinc_trx = nullptr; lock_table_remove_autoinc_lock(lock, trx); } @@ -6443,44 +6402,31 @@ lock_clust_rec_read_check_and_lock_alt( return(err); } -/*******************************************************************//** -Check if a transaction holds any autoinc locks. -@return TRUE if the transaction holds any AUTOINC locks. */ -static -ibool -lock_trx_holds_autoinc_locks( -/*=========================*/ - const trx_t* trx) /*!< in: transaction */ -{ - ut_a(trx->autoinc_locks != NULL); - - return(!ib_vector_is_empty(trx->autoinc_locks)); -} - /** Release all AUTO_INCREMENT locks of the transaction. */ static void lock_release_autoinc_locks(trx_t *trx) { { + auto begin= trx->autoinc_locks.begin(), end= trx->autoinc_locks.end(); + ut_ad(begin != end); LockMutexGuard g{SRW_LOCK_CALL}; mysql_mutex_lock(&lock_sys.wait_mutex); trx->mutex_lock(); - auto autoinc_locks= trx->autoinc_locks; - ut_a(autoinc_locks); /* We release the locks in the reverse order. This is to avoid searching the vector for the element to delete at the lower level. See (lock_table_remove_low()) for details. */ - while (ulint size= ib_vector_size(autoinc_locks)) + do { - lock_t *lock= *static_cast - (ib_vector_get(autoinc_locks, size - 1)); + lock_t *lock= *--end; ut_ad(lock->type_mode == (LOCK_AUTO_INC | LOCK_TABLE)); lock_table_dequeue(lock, true); lock_trx_table_locks_remove(lock); } + while (begin != end); } mysql_mutex_unlock(&lock_sys.wait_mutex); trx->mutex_unlock(); + trx->autoinc_locks.clear(); } /** Cancel a waiting lock request and release possibly waiting transactions */ @@ -6502,8 +6448,18 @@ void lock_cancel_waiting_and_release(lock_t *lock) { if (lock->type_mode == (LOCK_AUTO_INC | LOCK_TABLE)) { - ut_ad(trx->autoinc_locks); - ib_vector_remove(trx->autoinc_locks, lock); + /* This is similar to lock_table_remove_autoinc_lock() */ + auto begin= trx->autoinc_locks.begin(), end= trx->autoinc_locks.end(); + ut_ad(begin != end); + if (*--end == lock) + trx->autoinc_locks.erase(end, end + 1); + else + while (begin != end) + if (*--end == lock) + { + *end= nullptr; + break; + } } lock_table_dequeue(lock, true); /* Remove the lock from table lock vector too. */ @@ -6703,7 +6659,7 @@ lock_unlock_table_autoinc( ut_ad(trx_state == TRX_STATE_ACTIVE || trx_state == TRX_STATE_PREPARED || trx_state == TRX_STATE_NOT_STARTED); - if (lock_trx_holds_autoinc_locks(trx)) + if (!trx->autoinc_locks.empty()) lock_release_autoinc_locks(trx); } diff --git a/storage/innobase/mtr/mtr0mtr.cc b/storage/innobase/mtr/mtr0mtr.cc index bd0ef0eee1b..d8f694d80f0 100644 --- a/storage/innobase/mtr/mtr0mtr.cc +++ b/storage/innobase/mtr/mtr0mtr.cc @@ -1268,14 +1268,14 @@ void mtr_t::free(const fil_space_t &space, uint32_t offset) m_log.close(log_write(id, nullptr)); } -void small_vector_base::grow_by_1(void *small, size_t element_size) +void small_vector_base::grow_by_1(void *small, size_t element_size) noexcept { const size_t cap= Capacity*= 2, s= cap * element_size; void *new_begin; if (BeginX == small) { new_begin= my_malloc(PSI_NOT_INSTRUMENTED, s, MYF(0)); - memcpy(new_begin, BeginX, size() * element_size); + memcpy(new_begin, BeginX, s / 2); TRASH_FREE(small, size() * element_size); } else diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 7935cdb6bc6..ade8dea929a 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -170,6 +170,8 @@ struct TrxFactory { allocated object. trx_t objects are allocated by ut_zalloc_nokey() in Pool::Pool() which would not call the constructors of the trx_t members. */ + new(&trx->autoinc_locks) trx_t::autoinc_lock_vector(); + new(&trx->mod_tables) trx_mod_tables_t(); new(&trx->lock.table_locks) lock_list(); @@ -232,6 +234,8 @@ struct TrxFactory { trx->mutex_destroy(); + trx->autoinc_locks.~small_vector(); + trx->mod_tables.~trx_mod_tables_t(); ut_ad(!trx->read_view.is_open()); @@ -334,21 +338,12 @@ trx_t *trx_create() trx->assert_freed(); - mem_heap_t* heap; - ib_alloc_t* alloc; - /* We just got trx from pool, it should be non locking */ ut_ad(!trx->will_lock); ut_ad(!trx->rw_trx_hash_pins); DBUG_LOG("trx", "Create: " << trx); - heap = mem_heap_create(sizeof(ib_vector_t) + sizeof(void*) * 8); - - alloc = ib_heap_allocator_create(heap); - - trx->autoinc_locks = ib_vector_create(alloc, sizeof(void**), 4); - ut_ad(trx->mod_tables.empty()); ut_ad(trx->lock.n_rec_locks == 0); ut_ad(trx->lock.set_nth_bit_calls == 0); @@ -364,6 +359,7 @@ trx_t *trx_create() /** Free the memory to trx_pools */ void trx_t::free() { + autoinc_locks.fake_defined(); #ifdef HAVE_MEM_CHECK if (xid.is_null()) MEM_MAKE_DEFINED(&xid, sizeof xid); @@ -372,6 +368,7 @@ void trx_t::free() sizeof xid.data - (xid.gtrid_length + xid.bqual_length)); #endif MEM_CHECK_DEFINED(this, sizeof *this); + autoinc_locks.make_undefined(); ut_ad(!n_mysql_tables_in_use); ut_ad(!mysql_log_file_name); @@ -390,14 +387,7 @@ void trx_t::free() trx_sys.rw_trx_hash.put_pins(this); mysql_thd= nullptr; - // FIXME: We need to avoid this heap free/alloc for each commit. - if (autoinc_locks) - { - ut_ad(ib_vector_is_empty(autoinc_locks)); - /* We allocated a dedicated heap for the vector. */ - ib_vector_free(autoinc_locks); - autoinc_locks= NULL; - } + autoinc_locks.deep_clear(); MEM_NOACCESS(&skip_lock_inheritance_and_n_ref, sizeof skip_lock_inheritance_and_n_ref); @@ -495,7 +485,7 @@ inline void trx_t::release_locks() lock_release(this); ut_ad(!lock.n_rec_locks); ut_ad(UT_LIST_GET_LEN(lock.trx_locks) == 0); - ut_ad(ib_vector_is_empty(autoinc_locks)); + ut_ad(autoinc_locks.empty()); mem_heap_empty(lock.lock_heap); } @@ -923,7 +913,7 @@ trx_start_low( trx->wsrep = wsrep_on(trx->mysql_thd); #endif /* WITH_WSREP */ - ut_a(ib_vector_is_empty(trx->autoinc_locks)); + ut_a(trx->autoinc_locks.empty()); ut_a(trx->lock.table_locks.empty()); /* No other thread can access this trx object through rw_trx_hash,