From 08377b68b1ce45c881d76cae8787a3c8dbecc86b Mon Sep 17 00:00:00 2001 From: Mohammad Tafzeel Shams Date: Thu, 27 Aug 2026 07:11:56 +0530 Subject: [PATCH] MDEV-35154 : dict_sys_t::load_table() is holding exclusive dict_sys.latch for unnecessarily long time Issue: dict_load_table_one() was invoked with the exclusive dict_sys.latch held and never released it. The whole load ran inside that one critical section: reading the SYS_TABLES record, opening the .ibd file, and reading SYS_COLUMNS, SYS_VIRTUAL, SYS_INDEXES, SYS_FIELDS, the clustered index root page, and SYS_FOREIGN, as well as loading every table related by FOREIGN KEY constraints. Almost all of that is I/O. Because dict_sys.latch is the single latch that guards every dictionary lookup, one cold table open blocked every other session from opening any table, including tables that were already cached. A slow enough load could trigger the fatal semaphore wait check. Fix: Split the load into three phases. A short latched phase creates the table object and publishes it as an incomplete "stub"; the I/O runs with no latch held; a final latched phase links the FOREIGN KEY constraints. The stub's progress is held in the top two bits of dict_table_t:: n_ref_count (LOADING_DEF, LOADING_FK, LOAD_FAILED), analogous to how buf_page_t::state() combines a small lifecycle state with the buffer-fix count in one atomic. A thread that finds a loading table waits by pinning it (dict_table_t::try_pin_for_wait(), so it cannot be freed while unlatched) and acquiring dict_table_t::lock_latch in shared mode, the same latch the loader holds in exclusive mode for the duration of the load. dict_sys_t::wait_for_load() reports whether the load it waited for actually succeeded, so a caller that only needed to wait for the table it already found can use it directly afterwards instead of repeating the lookup; a caller that only wanted to confirm a related table had finished linking its constraints can likewise skip straight to treating it as already linked. A failed load can be torn down safely even though other threads may have already taken such a pin to wait on it: try_pin_for_wait() refuses once LOAD_FAILED is set, and the teardown path drains any pins taken earlier before freeing the stub. A table remains hidden (LOADING_FK) until every table related to it by FOREIGN KEY constraints has been loaded as well, and dict_sys_t:: load_table() makes them all visible in one step. The intermediate state is visible to constraint linking via dict_sys_t::find_table_fk(), so that two threads loading tables that reference each other can still link the constraint between them. Tables that InnoDB internal SQL can refer to are loaded without ever releasing the latch. The internal SQL parser is not reentrant and is serialized only by the exclusive dict_sys.latch, and it opens tables in the middle of parsing; releasing the latch there let another thread run the parser concurrently and corrupt its state. Changes: - dict_table_t : hold the progress of loading in the top bits of n_ref_count (std::atomic): LOADING_DEF, LOADING_FK, LOAD_FAILED, loading(). Add the loader-only helpers start_loading(), advance_to_loading_fk(), load_finish(), mark_load_failed(), and try_pin_for_wait() for waiters, all built on the existing lock_latch. - dict_load_table_one() : publish the table as a LOADING_DEF stub in table_non_LRU and release the latch before loading the tablespace, the columns and the indexes; reacquire it, move the table to table_LRU and advance it to LOADING_FK before loading the foreign key constraints. - dict_sys_t::load_table() : wait for a concurrent load of the same table via wait_for_load(); allocate the foreign key table names on a local heap, because the latch is released while draining them; in the drain loop, wait for a table whose definition is still being loaded by another thread and skip one that is only waiting for its own related tables; make all tables loaded by this invocation visible in one step via load_finish(). - dict_sys_t::wait_for_load() : pins the observed table via try_pin_for_wait() (returning false immediately if the load already failed), releases dict_sys.latch, blocks on the table's own lock_latch, reacquires dict_sys.latch, and only then checks loading() and releases its pin, returning whether the load succeeded. Because the latch is reacquired before the pin is released, a successful wait leaves the table valid to use directly, with no need to repeat the lookup that found it. Takes an excl parameter: whether the caller holds dict_sys.latch in exclusive mode (lock()/unlock()) or shared mode (freeze()/unfreeze()), so it knows which pair to use to release and reacquire it. - dict_load_table_one_discard() : used wherever a stub must be torn down (retry after DB_SUCCESS_LOCKED_REC, column or virtual-column load failure, a corrupted index or missing FK index). Calls mark_load_failed(), which marks the stub LOAD_FAILED and releases lock_latch to wake any already-pinned waiters, then drains the reference count to zero before removing the stub. - dict_sys_t::add() : take lock_latch in exclusive mode on the loader's behalf before a loading stub becomes reachable via find_table_any(). - dict_get_and_save_data_dir_path() : skip re-acquiring lock_latch when the table is loading(), because the loading thread already holds it in exclusive mode and is the only thread that can reach the table at that point. - dict_load_hold_latch() : whether a table may be referenced by InnoDB internal SQL, and therefore must be loaded without releasing the latch: InnoDB system tables, FULLTEXT INDEX auxiliary tables and the persistent statistics tables. The FULLTEXT prefix match is case-sensitive, matching how these auxiliary table names are actually generated. - dict_load_table_on_id() : copy the table name and release the SYS_TABLES page latch before calling load_table(), which may now block. Restore the cursor position only if the scan has to continue. - dict_load_foreign(), dict_load_foreigns() : add the fk_heap parameter and allocate the names appended to fk_tables on it. - dict_sys_t::find_table_any() : an unconditional lookup by table name or table_id_t, returning a table even while it is still being loaded. Only the name (or id) and hash pointers may be read on such a table. - dict_sys_t::find_table() : wait for a concurrent load of the table it finds, via wait_for_load(), both in the by-name and the by-id variant, retrying the lookup if the load failed, since the stub may have been freed, or a fresh load of the same name or id may now be in progress. Takes an excl parameter so that the caller can state which dict_sys.latch mode it holds: dict_acquire_mdl_shared(), dict_table_open_on_id(), dict_table_open_on_name(), row_purge_remove_clust_if_poss_low(), trx_t::evict_table(), UndorecApplier::apply_undo_rec(), trx_purge_table_open(), prepare_inplace_alter_table_dict(), lock_release(), and create_table_info_t::create_foreign_keys() all pass it. - dict_sys_t::find_table_fk() : like find_table(), but LOADING_FK tables are returned, so that constraints can be linked into them while holding the exclusive latch. - dict_table_can_be_evicted() : a table that is being loaded may only be freed by the thread that is loading it. - dict_foreign_add_to_cache() : resolve both sides with find_table_fk(). - dict_table_t::load_failed_refs_cleared() : whether every reference taken before a failed load has been released. dict_load_table_one_discard() uses this to drain a failed stub before freeing it, instead of comparing get_ref_count() to 0. - create_table_info_t::create_foreign_keys() : take a temporary reference on a referenced table as soon as it is resolved, released on every exit path by a scope guard; call dict_sys.prevent_eviction() only once the constraint is actually committed to the dictionary cache, replacing the temporary reference. - create_table_info_t::create_table() : acquire a reference to the created table around the foreign key handling, and use a local heap for the names of the foreign key related tables. - assertion fix : the load path may now run without dict_sys.latch, but only in the thread that is loading the table, and a cached table is no longer necessarily fully loaded. dict_sys.locked() is relaxed to "dict_sys.locked() || table->is_loader()" in dict_load_columns(), dict_load_virtual_col(), dict_load_fields(), dict_load_indexes(), dict_index_add_to_cache(), dict_index_find_cols(), dict_index_build_internal_clust(), dict_index_build_internal_non_clust() and dict_index_build_internal_fts(). Checks of dict_table_t::cached are relaxed and reordered after the atomic loading in dict_table_add_system_columns(), dict_sys_t::add(), dict_table_rename_in_cache() and hash_insert(). - innodb.dict_load_concurrent A load is parked at dict_load_table_one_no_latch while holding no latch; another table can be loaded meanwhile, and a second opener of the same table waits. A second case checks that a table is not made visible while a table related to it by a FOREIGN KEY constraint is still being loaded by another thread. --- .../innodb/r/dict_load_concurrent.result | 83 +++++ .../suite/innodb/t/dict_load_concurrent.test | 140 ++++++++ storage/innobase/btr/btr0sea.cc | 9 + storage/innobase/dict/dict0dict.cc | 125 ++++++- storage/innobase/dict/dict0load.cc | 327 +++++++++++++++--- storage/innobase/fts/fts0fts.cc | 2 + storage/innobase/handler/ha_innodb.cc | 61 +++- storage/innobase/handler/handler0alter.cc | 13 +- storage/innobase/handler/i_s.cc | 2 + storage/innobase/include/dict0dict.h | 85 ++++- storage/innobase/include/dict0dict.inl | 30 +- storage/innobase/include/dict0load.h | 5 + storage/innobase/include/dict0mem.h | 106 +++++- storage/innobase/lock/lock0lock.cc | 2 +- storage/innobase/row/row0merge.cc | 1 + storage/innobase/row/row0mysql.cc | 8 +- storage/innobase/row/row0purge.cc | 3 +- storage/innobase/trx/trx0purge.cc | 2 +- storage/innobase/trx/trx0trx.cc | 3 +- storage/innobase/trx/trx0undo.cc | 2 +- 20 files changed, 917 insertions(+), 92 deletions(-) create mode 100644 mysql-test/suite/innodb/r/dict_load_concurrent.result create mode 100644 mysql-test/suite/innodb/t/dict_load_concurrent.test diff --git a/mysql-test/suite/innodb/r/dict_load_concurrent.result b/mysql-test/suite/innodb/r/dict_load_concurrent.result new file mode 100644 index 0000000000000..0e88c5ae8da85 --- /dev/null +++ b/mysql-test/suite/innodb/r/dict_load_concurrent.result @@ -0,0 +1,83 @@ +# +# MDEV-35154 dict_sys_t::load_table() is holding exclusive dict_sys.latch for unnecessarily long time +# +CREATE TABLE t1(a INT PRIMARY KEY) STATS_PERSISTENT=0 ENGINE=InnoDB; +CREATE TABLE t2(a INT PRIMARY KEY) STATS_PERSISTENT=0 ENGINE=InnoDB; +INSERT INTO t1 VALUES(1); +INSERT INTO t2 VALUES(2); +SET @save_evict= @@GLOBAL.innodb_evict_tables_on_commit_debug; +SET GLOBAL innodb_evict_tables_on_commit_debug=ON; +XA START 'x'; +INSERT INTO t1 VALUES(2); +INSERT INTO t2 VALUES(3); +XA END 'x'; +XA PREPARE 'x'; +connect con1,localhost,root,,; +FLUSH TABLES; +connection default; +XA COMMIT 'x'; +SET GLOBAL innodb_evict_tables_on_commit_debug=@save_evict; +connection con1; +SET DEBUG_SYNC = 'dict_load_table_one_no_latch SIGNAL loading WAIT_FOR go TIMEOUT 30'; +SELECT * FROM t1; +connection default; +SET DEBUG_SYNC = 'now WAIT_FOR loading TIMEOUT 30'; +SELECT * FROM t2; +a +2 +3 +connect con2,localhost,root,,; +SELECT * FROM t1; +connection default; +SET DEBUG_SYNC = 'now SIGNAL go'; +connection con1; +a +1 +2 +connection con2; +a +1 +2 +connection default; +SET DEBUG_SYNC = RESET; +DROP TABLE t1, t2; +CREATE TABLE parent(a INT PRIMARY KEY) STATS_PERSISTENT=0 ENGINE=InnoDB; +CREATE TABLE child(a INT PRIMARY KEY, +FOREIGN KEY(a) REFERENCES parent(a)) +STATS_PERSISTENT=0 ENGINE=InnoDB; +INSERT INTO parent VALUES(1); +INSERT INTO child VALUES(1); +SET GLOBAL innodb_evict_tables_on_commit_debug=ON; +XA START 'y'; +INSERT INTO parent VALUES(2); +INSERT INTO child VALUES(2); +XA END 'y'; +XA PREPARE 'y'; +connection con1; +FLUSH TABLES; +connection default; +XA COMMIT 'y'; +SET GLOBAL innodb_evict_tables_on_commit_debug=@save_evict; +connection con1; +SET DEBUG_SYNC = 'dict_load_table_one_no_latch SIGNAL child_loading WAIT_FOR child_go TIMEOUT 30'; +SELECT * FROM child; +connection con2; +SET DEBUG_SYNC = 'now WAIT_FOR child_loading TIMEOUT 30'; +SELECT * FROM parent; +connection default; +SET DEBUG_SYNC = 'now SIGNAL child_go'; +connection con1; +a +1 +2 +connection con2; +a +1 +2 +connection default; +DELETE FROM parent WHERE a=1; +ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fails (`test`.`child`, CONSTRAINT `child_ibfk_1` FOREIGN KEY (`a`) REFERENCES `parent` (`a`)) +disconnect con1; +disconnect con2; +SET DEBUG_SYNC = RESET; +DROP TABLE child, parent; diff --git a/mysql-test/suite/innodb/t/dict_load_concurrent.test b/mysql-test/suite/innodb/t/dict_load_concurrent.test new file mode 100644 index 0000000000000..9cdaf120b77c3 --- /dev/null +++ b/mysql-test/suite/innodb/t/dict_load_concurrent.test @@ -0,0 +1,140 @@ +--source include/have_innodb.inc +--source include/have_debug.inc +--source include/have_debug_sync.inc +--source include/count_sessions.inc + +--echo # +--echo # MDEV-35154 dict_sys_t::load_table() is holding exclusive dict_sys.latch for unnecessarily long time +--echo # + +# Loading a table definition into the dictionary cache must not hold +# the exclusive dict_sys.latch during the I/O phase. A table whose +# definition is being loaded is published as an incomplete stub; +# other openers of the same table wait for the load to complete, +# while any other table can be opened (and loaded) concurrently. + +# STATS_PERSISTENT=0 keeps the background statistics thread from +# reloading the evicted table definitions behind our back. +CREATE TABLE t1(a INT PRIMARY KEY) STATS_PERSISTENT=0 ENGINE=InnoDB; +CREATE TABLE t2(a INT PRIMARY KEY) STATS_PERSISTENT=0 ENGINE=InnoDB; +INSERT INTO t1 VALUES(1); +INSERT INTO t2 VALUES(2); + +# Evict t1 and t2 from the dictionary cache: at XA COMMIT, the +# modified tables are evicted if no table handles are open, which +# FLUSH TABLES between XA PREPARE and XA COMMIT guarantees. The +# FLUSH must be issued from another connection, because a session +# with a prepared XA transaction refuses other statements. The +# prepared transaction's own table lock protects the tables from +# LRU eviction until the commit. +SET @save_evict= @@GLOBAL.innodb_evict_tables_on_commit_debug; +SET GLOBAL innodb_evict_tables_on_commit_debug=ON; +XA START 'x'; +INSERT INTO t1 VALUES(2); +INSERT INTO t2 VALUES(3); +XA END 'x'; +XA PREPARE 'x'; + +connect con1,localhost,root,,; +FLUSH TABLES; + +connection default; +XA COMMIT 'x'; +SET GLOBAL innodb_evict_tables_on_commit_debug=@save_evict; + +connection con1; +SET DEBUG_SYNC = 'dict_load_table_one_no_latch SIGNAL loading WAIT_FOR go TIMEOUT 30'; +send SELECT * FROM t1; + +connection default; +SET DEBUG_SYNC = 'now WAIT_FOR loading TIMEOUT 30'; + +# con1 is parked inside the load of t1 while holding no dict_sys.latch. +# Loading another table must not block. (Before this change, the +# following statement would hang until the load of t1 completed.) +SELECT * FROM t2; + +connect con2,localhost,root,,; +send SELECT * FROM t1; + +connection default; +# con2 must wait for the load of t1 (it observes the loading stub); +# con1 is parked at the DEBUG_SYNC point (state 'debug sync point: +# ...'), so exactly one session is waiting in 'Opening tables'. +let $wait_condition= + SELECT COUNT(*) = 1 FROM information_schema.processlist + WHERE state = 'Opening tables' AND info = 'SELECT * FROM t1'; +--source include/wait_condition.inc + +SET DEBUG_SYNC = 'now SIGNAL go'; + +connection con1; +reap; +connection con2; +reap; +connection default; +SET DEBUG_SYNC = RESET; +DROP TABLE t1, t2; + +# +# A table must not become visible while a table that is related to +# it by a FOREIGN KEY constraint is still being loaded by another +# thread: the constraint would be missing from referenced_set. +# +CREATE TABLE parent(a INT PRIMARY KEY) STATS_PERSISTENT=0 ENGINE=InnoDB; +CREATE TABLE child(a INT PRIMARY KEY, + FOREIGN KEY(a) REFERENCES parent(a)) +STATS_PERSISTENT=0 ENGINE=InnoDB; +INSERT INTO parent VALUES(1); +INSERT INTO child VALUES(1); + +SET GLOBAL innodb_evict_tables_on_commit_debug=ON; +XA START 'y'; +INSERT INTO parent VALUES(2); +INSERT INTO child VALUES(2); +XA END 'y'; +XA PREPARE 'y'; + +connection con1; +FLUSH TABLES; + +connection default; +XA COMMIT 'y'; +SET GLOBAL innodb_evict_tables_on_commit_debug=@save_evict; + +# Park inside the load of the child table. +connection con1; +SET DEBUG_SYNC = 'dict_load_table_one_no_latch SIGNAL child_loading WAIT_FOR child_go TIMEOUT 30'; +send SELECT * FROM child; + +connection con2; +SET DEBUG_SYNC = 'now WAIT_FOR child_loading TIMEOUT 30'; +# Loading the parent finds the constraint in SYS_FOREIGN, but the child +# is still an incomplete stub, so the constraint cannot be linked yet. +# The parent must not become visible before the child has been loaded. +send SELECT * FROM parent; + +connection default; +let $wait_condition= + SELECT COUNT(*) = 1 FROM information_schema.processlist + WHERE state = 'Opening tables' AND info = 'SELECT * FROM parent'; +--source include/wait_condition.inc + +SET DEBUG_SYNC = 'now SIGNAL child_go'; + +connection con1; +reap; +connection con2; +reap; + +connection default; +# The constraint was linked into parent->referenced_set before the +# parent became visible. +--error ER_ROW_IS_REFERENCED_2 +DELETE FROM parent WHERE a=1; + +disconnect con1; +disconnect con2; +SET DEBUG_SYNC = RESET; +DROP TABLE child, parent; +--source include/wait_until_count_sessions.inc diff --git a/storage/innobase/btr/btr0sea.cc b/storage/innobase/btr/btr0sea.cc index ee2fdde991e67..8681562e68463 100644 --- a/storage/innobase/btr/btr0sea.cc +++ b/storage/innobase/btr/btr0sea.cc @@ -245,6 +245,15 @@ ATTRIBUTE_COLD bool btr_search_disable() for (table = UT_LIST_GET_FIRST(dict_sys.table_non_LRU); table; table = UT_LIST_GET_NEXT(table_LRU, table)) { + if (table->loading()) { + /* The table definition is being loaded by another + thread without dict_sys.latch; walking its indexes + would race with that thread. The adaptive hash index + can only point to records of indexes whose metadata + is already cached. */ + continue; + } + btr_search_disable_ref_count(table); } diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index 10eaf7a33408b..ead0bddaab2ed 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -597,7 +597,7 @@ dict_table_t *dict_sys_t::acquire_temporary_table(table_id_t id) const noexcept }); } -dict_table_t *dict_sys_t::find_table(table_id_t id) const noexcept +dict_table_t *dict_sys_t::find_table_any(table_id_t id) const noexcept { ut_ad(frozen()); return table_id_hash.cell_get(ut_fold_ull(id))-> @@ -609,7 +609,24 @@ dict_table_t *dict_sys_t::find_table(table_id_t id) const noexcept }); } -dict_table_t *dict_sys_t::find_table(const span &name) +dict_table_t *dict_sys_t::find_table(table_id_t id, bool excl) noexcept +{ + for (;;) + { + dict_table_t *t= find_table_any(id); + if (!t || !t->loading()) + return t; + /* Another thread is loading the table definition. Wait for it to + finish; on success t is valid to return directly, but if it + failed, retry from the top (the stub may have been freed, or a + fresh load of the same id may now be in progress). */ + ut_ad(!t->is_loader()); + if (wait_for_load(t, excl)) + return t; + } +} + +dict_table_t *dict_sys_t::find_table_any(const span &name) const noexcept { ut_ad(frozen()); @@ -621,6 +638,41 @@ dict_table_t *dict_sys_t::find_table(const span &name) }); } +dict_table_t *dict_sys_t::find_table(const span &name, bool excl) + noexcept +{ + for (;;) + { + dict_table_t *t= find_table_any(name); + if (!t || !t->loading()) + return t; + /* Another thread is loading the table definition. Wait for it to + finish; on success t is valid to return directly, but if it + failed, retry from the top (the stub may have been freed, or a + fresh load of the same name may now be in progress). */ + ut_ad(!t->is_loader()); + if (wait_for_load(t, excl)) + return t; + } +} + +dict_table_t *dict_sys_t::find_table_fk(const span &name) + const noexcept +{ + ut_ad(locked()); + dict_table_t *t= find_table_any(name); + /* A table whose definition is still being loaded (LOADING_DEF) is + concurrently modified by its loader without any latch; it must be + treated as if it were not cached. A table that is only waiting for + its FOREIGN KEY related tables to be loaded (LOADING_FK) has a + complete definition, and its foreign_set/referenced_set are only + modified under the exclusive latch, which we are holding; linking + a constraint into it is safe, and necessary so that two concurrent + dict_sys_t::load_table() invocations whose tables reference each + other will resolve the constraints between them. */ + return t && t->loading() == dict_table_t::LOADING_DEF ? nullptr : t; +} + /** Acquire MDL shared for the table name. @tparam trylock whether to use non-blocking operation @param[in,out] table table object @@ -689,7 +741,7 @@ dict_acquire_mdl_shared(dict_table_t *table, size_t db1_len, tbl1_len; lookup: dict_sys.freeze(SRW_LOCK_CALL); - table= dict_sys.find_table(table_id); + table= dict_sys.find_table(table_id, false); if (table) { if (!table->is_accessible()) @@ -825,7 +877,7 @@ dict_table_t *dict_table_open_on_id(table_id_t table_id, bool dict_locked, if (!dict_locked) dict_sys.freeze(SRW_LOCK_CALL); - dict_table_t *table= dict_sys.find_table(table_id); + dict_table_t *table= dict_sys.find_table(table_id, dict_locked); if (table) { @@ -978,6 +1030,29 @@ void dict_sys_t::lock_wait(SRW_LOCK_ARGS(const char *file, unsigned line)) noexc latch.wr_lock(SRW_LOCK_ARGS(file, line)); } +bool dict_sys_t::wait_for_load(dict_table_t *table, bool excl) noexcept +{ + ut_ad(excl ? locked() : frozen_not_locked()); + ut_ad(!table->is_loader()); + if (!table->try_pin_for_wait()) + /* The load already failed; nothing to wait for. */ + return false; + /* Pin the table so it survives releasing dict_sys.latch: blocking + on lock_latch while still holding dict_sys.latch could deadlock + against a loader that needs dict_sys.latch to link FOREIGN KEY + constraints. */ + if (excl) unlock(); else unfreeze(); + table->lock_shared_lock(); + table->lock_shared_unlock(); + if (excl) lock(SRW_LOCK_CALL); else freeze(SRW_LOCK_CALL); + /* The loader released lock_latch only after settling into + NOT_LOADING or LOAD_FAILED; check while our pin still guarantees + table is valid, then release it (dict_sys.latch takes over). */ + const bool ready= !table->loading(); + table->release(); + return ready; +} + #ifdef UNIV_PFS_RWLOCK ATTRIBUTE_NOINLINE void dict_sys_t::unlock() noexcept { @@ -1031,7 +1106,7 @@ dict_table_open_on_name( if (!dict_locked) { dict_sys.freeze(SRW_LOCK_CALL); - table= dict_sys.find_table(name); + table= dict_sys.find_table(name, false); if (table) { ut_ad(table->cached); @@ -1144,7 +1219,10 @@ dict_table_add_system_columns( { ut_ad(table->n_def == table->n_cols - DATA_N_SYS_COLS); ut_ad(table->magic_n == DICT_TABLE_MAGIC_N); - ut_ad(!table->cached); + /* A table that is being loaded by dict_load_table_one() was + published in the cache as an incomplete stub before its columns + were loaded. */ + ut_ad(!table->cached || table->is_loader()); /* NOTE: the system columns MUST be added in the following order (so that they can be indexed by the numerical value of DATA_ROW_ID, @@ -1187,12 +1265,17 @@ inline void dict_sys_t::add(dict_table_t *table) noexcept table->row_id= 0; table->autoinc_mutex.init(); table->lock_mutex_init(); + if (table->loading()) + /* Take lock_latch before the stub is reachable via find_table_any(). */ + table->lock_mutex_lock(); const char *name= table->name.m_name; dict_table_t **prev= table_hash.cell_get(my_crc32c(0, name, strlen(name)))-> search(&dict_table_t::name_hash, [name](const dict_table_t *t) { if (!t) return true; - ut_ad(t->cached); + /* t may be a stub that another thread is loading; check the + atomic flag before the bit-field to avoid a torn read. */ + ut_ad(t->loading() || t->cached); ut_a(strcmp(t->name.m_name, name)); return false; }); @@ -1202,7 +1285,7 @@ inline void dict_sys_t::add(dict_table_t *table) noexcept search(&dict_table_t::id_hash, [table](const dict_table_t *t) { if (!t) return true; - ut_ad(t->cached); + ut_ad(t->loading() || t->cached); ut_a(t->id != table->id); return false; }); @@ -1221,6 +1304,11 @@ static bool dict_table_can_be_evicted(dict_table_t *table) { ut_ad(dict_sys.locked()); ut_a(table->can_be_evicted); + + if (table->loading()) { + return false; + } + ut_a(table->foreign_set.empty()); ut_a(table->referenced_set.empty()); @@ -1863,7 +1951,10 @@ void dict_sys_t::remove(dict_table_t* table, bool lru, bool keep) noexcept dict_index_t* index; ut_ad(dict_lru_validate()); - ut_a(table->get_ref_count() == 0); + /* A stub discarded after a failed load (dict_load_table_one_discard()) + still carries LOAD_FAILED in n_ref_count; only its reference count + bits must have drained to zero. */ + ut_a((table->get_ref_count() & ~dict_table_t::LOADING_MASK) == 0); ut_a(table->n_rec_locks == 0); ut_ad(find(table)); ut_ad(table->magic_n == DICT_TABLE_MAGIC_N); @@ -1986,7 +2077,7 @@ dict_index_add_to_cache( ulint n_ord; ulint i; - ut_ad(dict_sys.locked()); + ut_ad(dict_sys.locked() || index->table->is_loader()); ut_ad(index->n_def == index->n_fields); ut_ad(index->magic_n == DICT_INDEX_MAGIC_N); ut_ad(!dict_index_is_online_ddl(index)); @@ -2190,7 +2281,7 @@ dict_index_find_cols( const dict_table_t* table = index->table; ut_ad(table->magic_n == DICT_TABLE_MAGIC_N); - ut_ad(dict_sys.locked()); + ut_ad(dict_sys.locked() || table->is_loader()); for (ulint i = 0; i < index->n_fields; i++) { ulint j; @@ -2452,7 +2543,7 @@ dict_index_build_internal_clust( ut_ad(index->is_primary()); ut_ad(!index->has_virtual()); - ut_ad(dict_sys.locked()); + ut_ad(dict_sys.locked() || table->is_loader()); /* Create a new index object with certainly enough fields */ new_index = dict_mem_index_create(index->table, index->name, @@ -2605,7 +2696,7 @@ dict_index_build_internal_non_clust( ibool* indexed; ut_ad(!index->is_primary()); - ut_ad(dict_sys.locked()); + ut_ad(dict_sys.locked() || table->is_loader()); /* The clustered index should be the first in the list of indexes */ clust_index = UT_LIST_GET_FIRST(table->indexes); @@ -2693,7 +2784,7 @@ dict_index_build_internal_fts( dict_index_t* new_index; ut_ad(index->type & DICT_FTS); - ut_ad(dict_sys.locked()); + ut_ad(dict_sys.locked() || index->table->is_loader()); /* Create a new index */ new_index = dict_mem_index_create(index->table, index->name, @@ -2912,11 +3003,11 @@ dict_foreign_add_to_cache( ut_ad(dict_sys.locked()); - for_table = dict_sys.find_table( + for_table = dict_sys.find_table_fk( {foreign->foreign_table_name_lookup, strlen(foreign->foreign_table_name_lookup)}); - ref_table = dict_sys.find_table( + ref_table = dict_sys.find_table_fk( {foreign->referenced_table_name_lookup, strlen(foreign->referenced_table_name_lookup)}); ut_a(for_table || ref_table); @@ -4352,7 +4443,7 @@ dict_fs2utf8( @param id_hash dict_sys.table_id_hash or dict_sys.temp_id_hash */ static void hash_insert(dict_table_t *table, hash_table_t& id_hash) noexcept { - ut_ad(table->cached); + ut_ad(table->loading() || table->cached); dict_sys.table_hash.cell_get(my_crc32c(0, table->name.m_name, strlen(table->name.m_name)))-> append(*table, &dict_table_t::name_hash); diff --git a/storage/innobase/dict/dict0load.cc b/storage/innobase/dict/dict0load.cc index b5a1f55e3ec91..7d05e41d02217 100644 --- a/storage/innobase/dict/dict0load.cc +++ b/storage/innobase/dict/dict0load.cc @@ -57,15 +57,22 @@ key constraints are loaded into memory. @param[in] name Table name in the db/tablename format @param[in] ignore_err Error to be ignored when loading table and its index definition +@param[in] fk_heap Heap on which the names appended to + fk_tables are allocated @param[out] fk_tables Related table names that must also be loaded to ensure that all foreign key constraints are loaded. +@param[in] hold_latch Whether to hold the exclusive + dict_sys.latch for the whole load; see + dict_load_hold_latch() @return table, possibly with file_unreadable flag set @retval nullptr if the table does not exist */ static dict_table_t *dict_load_table_one(mtr_t &mtr, const span &name, dict_err_ignore_t ignore_err, - dict_names_t &fk_tables); + mem_heap_t *fk_heap, + dict_names_t &fk_tables, + bool hold_latch); /** Load an index definition from a SYS_INDEXES record to dict_index_t. @return error message @@ -1311,7 +1318,7 @@ static dberr_t dict_load_columns(mtr_t &mtr, btr_pcur_t pcur; ulint n_skipped = 0; - ut_ad(dict_sys.locked()); + ut_ad(dict_sys.locked() || table->is_loader()); mtr.start(); @@ -1441,7 +1448,7 @@ dict_load_virtual_col(mtr_t &mtr, dict_table_t *table, bool uncommitted, dict_index_t* sys_virtual_index; btr_pcur_t pcur; - ut_ad(dict_sys.locked()); + ut_ad(dict_sys.locked() || table->is_loader()); mtr.start(); @@ -1688,7 +1695,7 @@ static dberr_t dict_load_fields(mtr_t &mtr, dict_index_t *index, { btr_pcur_t pcur; - ut_ad(dict_sys.locked()); + ut_ad(dict_sys.locked() || index->table->is_loader()); auto sp = mtr.get_savepoint(); dict_index_t* sys_index = dict_sys.sys_fields->indexes.start; @@ -1943,7 +1950,7 @@ dberr_t dict_load_indexes(mtr_t *mtr, dict_table_t *table, bool uncommitted, btr_pcur_t pcur; byte table_id[8]; - ut_ad(dict_sys.locked()); + ut_ad(dict_sys.locked() || table->is_loader()); mtr->start(); @@ -2228,11 +2235,17 @@ void dict_get_and_save_data_dir_path(dict_table_t *table) if (strncmp(fil_path_to_mysql_datadir, filepath, strlen(fil_path_to_mysql_datadir))) { - table->lock_mutex_lock(); + /* A loading thread already holds lock_latch exclusively (see + dict_sys_t::add()) and is the only thread that can reach this + table, so it must not re-acquire it here. */ + const bool need_latch= !table->loading(); + if (need_latch) + table->lock_mutex_lock(); table->flags|= 1 << DICT_TF_POS_DATA_DIR & ((1U << DICT_TF_BITS) - 1); table->data_dir_path= mem_heap_strdup(table->heap, filepath); os_file_make_data_dir_path(table->data_dir_path); - table->lock_mutex_unlock(); + if (need_latch) + table->lock_mutex_unlock(); } } } @@ -2303,6 +2316,61 @@ dict_load_tablespace( ut_free(filepath); } +/** Determine whether a table must be loaded without ever releasing +the exclusive dict_sys.latch, because it may be referenced by InnoDB +internal SQL. The non-reentrant internal SQL parser is serialized by +the exclusive dict_sys.latch, which the callers of que_eval_sql() hold +across parsing and execution; pars_retrieve_table_def() opens the +referenced tables in the middle of parsing. Releasing the latch there +(to publish a loading stub, or to wait for one in +dict_sys_t::wait_for_load()) would let another thread corrupt the +parser state. Because every load of such a table holds the latch throughout, +no loading stub can ever exist for it, and a parse-time lookup can +never encounter one. +@param name table name ("db/table" or an InnoDB system table name) +@return whether the table must be loaded while holding the latch */ +static bool dict_load_hold_latch(const span &name) +{ + const void* p = memchr(name.data(), '/', name.size()); + + if (!p) { + /* InnoDB system tables, such as SYS_FOREIGN, have no + database component in their names. */ + return true; + } + + const char* n = static_cast(p) + 1; + + if (name.end() > n + 4 + && n[0] == 'F' && n[1] == 'T' + && n[2] == 'S' && n[3] == '_') { + /* FULLTEXT INDEX auxiliary tables */ + return true; + } + + /* The persistent statistics tables */ + return (name.size() == sizeof TABLE_STATS_NAME - 1 + && !memcmp(name.data(), TABLE_STATS_NAME, name.size())) + || (name.size() == sizeof INDEX_STATS_NAME - 1 + && !memcmp(name.data(), INDEX_STATS_NAME, name.size())); +} + +/** Discard a table stub whose load failed (or is being retried after +DB_SUCCESS_LOCKED_REC), unlinking it from dict_sys and freeing it. +@param table a stub published by this thread via add_to_cache(), +still holding lock_latch in exclusive mode */ +static void dict_load_table_one_discard(dict_table_t *table) noexcept +{ + table->mark_load_failed(); + /* Wait for any wait_for_load() caller that had already pinned the + table to notice and release its pin; no new pin is possible once + LOAD_FAILED is set, so this can only reach zero, never rise. */ + while (!table->load_failed_refs_cleared()) { + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + dict_sys.remove(table); +} + /** Loads a table definition and also all its index definitions. Loads those foreign key constraints whose referenced table is already in @@ -2315,15 +2383,22 @@ key constraints are loaded into memory. @param[in] name Table name in the db/tablename format @param[in] ignore_err Error to be ignored when loading table and its index definition +@param[in] fk_heap Heap on which the names appended to + fk_tables are allocated @param[out] fk_tables Related table names that must also be loaded to ensure that all foreign key constraints are loaded. +@param[in] hold_latch Whether to hold the exclusive + dict_sys.latch for the whole load; see + dict_load_hold_latch() @return table, possibly with file_unreadable flag set @retval nullptr if the table does not exist */ static dict_table_t *dict_load_table_one(mtr_t &mtr, const span &name, dict_err_ignore_t ignore_err, - dict_names_t &fk_tables) + mem_heap_t *fk_heap, + dict_names_t &fk_tables, + bool hold_latch) { btr_pcur_t pcur; @@ -2332,6 +2407,7 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, ("table: %.*s", int(name.size()), name.data())); ut_ad(dict_sys.locked()); + ut_ad(!dict_load_hold_latch(name) || hold_latch); dict_index_t *sys_index = dict_sys.sys_tables->indexes.start; ut_ad(!dict_sys.sys_tables->not_redundant()); @@ -2358,7 +2434,20 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, pcur.btr_cur.page_cur.index = sys_index; bool uncommitted = false; + dict_table_t* table = nullptr; reload: + /* Phase 1 (exclusive dict_sys.latch): locate the SYS_TABLES + record, create the table object, and make it visible in the + cache as a non-evictable, incomplete stub (dict_table_t::loading()), + so that the latch can be released during the I/O below. */ + if (table) { + /* We are retrying after DB_SUCCESS_LOCKED_REC. Discard + the previously published stub; a new table object will + be created. */ + dict_load_table_one_discard(table); + table = nullptr; + } + mtr.start(); dberr_t err = btr_pcur_open_on_user_rec(&tuple, BTR_SEARCH_LEAF, &pcur, &mtr); @@ -2378,7 +2467,6 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, goto err_exit; } - dict_table_t* table; if (const char* err_msg = dict_load_table_low(&mtr, uncommitted, rec, &table)) { if (err_msg != dict_load_table_flags) { @@ -2398,6 +2486,29 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, mtr.commit(); + /* Publish the incomplete table object. Any other thread that + looks it up will observe dict_table_t::loading() and either treat + the table as not cached (dict_sys_t::find_table()) or wait for + the load to complete (dict_sys_t::load_table()). add_to_cache() + takes lock_latch in exclusive mode on our behalf before the stub + becomes reachable. The stub is inserted into table_non_LRU + (can_be_evicted=false), so that dict_sys_t::evict_table_LRU() + cannot free it. */ + ut_ad(!table->can_be_evicted); + table->start_loading(); + ut_d(table->load_thread = pthread_self()); + table->add_to_cache(); + + if (!hold_latch) { + dict_sys.unlock(); + } + + /* Phase 2 (no dict_sys.latch): open the tablespace and load the + columns and indexes. Only this thread may access the table object; + the SYS_ tables and their indexes are non-evictable, and any DDL + that would modify this table's SYS_ records must first look up the + table in the cache, observe the stub, and wait for it. */ + mem_heap_t* heap = mem_heap_create(32000); dict_load_tablespace(table, ignore_err); @@ -2406,8 +2517,10 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, case DB_SUCCESS_LOCKED_REC: ut_ad(!uncommitted); uncommitted = true; - dict_mem_table_free(table); mem_heap_free(heap); + if (!hold_latch) { + dict_sys.lock(SRW_LOCK_CALL); + } goto reload; case DB_SUCCESS: if (!dict_load_virtual(mtr, table, uncommitted)) { @@ -2415,16 +2528,16 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, } /* fall through */ default: - dict_mem_table_free(table); mem_heap_free(heap); + if (!hold_latch) { + dict_sys.lock(SRW_LOCK_CALL); + } + dict_load_table_one_discard(table); DBUG_RETURN(nullptr); } dict_table_add_system_columns(table, heap); - table->can_be_evicted = true; - table->add_to_cache(); - mem_heap_empty(heap); ut_ad(dict_tf2_is_valid(table->flags, table->flags2)); @@ -2443,19 +2556,16 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, err = dict_load_indexes(&mtr, table, uncommitted, heap, index_load_err); + bool evict_stub = false; + if (err == DB_TABLE_CORRUPT) { /* Refuse to load the table if the table has a corrupted cluster index */ ut_ad(index_load_err != DICT_ERR_IGNORE_DROP); ib::error() << "Refusing to load corrupted table " << table->name; -evict: - dict_sys.remove(table); - mem_heap_free(heap); - DBUG_RETURN(nullptr); - } - - if (err != DB_SUCCESS || !table->is_readable()) { + evict_stub = true; + } else if (err != DB_SUCCESS || !table->is_readable()) { } else if (dict_index_t* pk = dict_table_get_first_index(table)) { ut_ad(pk->is_primary()); if (pk->is_corrupted() @@ -2494,10 +2604,38 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, ut_ad(ignore_err & DICT_ERR_IGNORE_INDEX); if (ignore_err != DICT_ERR_IGNORE_DROP) { err = DB_CORRUPTION; - goto evict; + evict_stub = true; } } + if (!hold_latch) { + /* When the definition is loaded by a connection thread, + allow tests to run other statements while no + dict_sys.latch is held. */ + DEBUG_SYNC_C("dict_load_table_one_no_latch"); + + /* Phase 3 (exclusive dict_sys.latch): complete the + definition and load the foreign key constraints. */ + dict_sys.lock(SRW_LOCK_CALL); + } + + dict_sys.allow_eviction(table); + /* The definition is complete, but the table stays hidden from + dict_sys_t::find_table() until our caller dict_sys_t::load_table() + has also loaded the tables that are related to this table by + FOREIGN KEY constraints, and makes all of them visible atomically. + From this point on, constraints may be linked into this table + via dict_sys_t::find_table_fk(): the foreign_set/referenced_set + are only modified while holding the exclusive latch. */ + table->advance_to_loading_fk(); + + if (evict_stub) { +evict: + mem_heap_free(heap); + dict_load_table_one_discard(table); + DBUG_RETURN(nullptr); + } + /* We will load the foreign key information only if all indexes were loaded. */ if (!table->is_readable()) { @@ -2505,7 +2643,8 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, } else if (err == DB_SUCCESS) { auto i = fk_tables.size(); err = dict_load_foreigns(mtr, table->name.m_name, nullptr, - 0, true, ignore_err, fk_tables); + 0, true, ignore_err, fk_heap, + fk_tables); if (err != DB_SUCCESS) { fk_tables.erase(fk_tables.begin() + i, fk_tables.end()); @@ -2552,20 +2691,83 @@ static dict_table_t *dict_load_table_one(mtr_t &mtr, dict_table_t *dict_sys_t::load_table(const span &name, dict_err_ignore_t ignore) noexcept { - if (dict_table_t *table= find_table(name)) - return table; + for (;;) + { + dict_table_t *table= find_table_any(name); + if (!table) + break; + if (!table->loading()) + return table; + /* Another thread is loading the table definition. Wait for it + to complete or fail. wait_for_load() drops and reacquires + dict_sys.latch around the actual wait, but validates table + before releasing its own temporary pin on it, handing off + safety to dict_sys.latch; we hold that latch continuously from + here on, so on success table is still valid to return directly. */ + ut_ad(!table->is_loader()); + if (wait_for_load(table, true)) + return table; + } + + mem_heap_t *fk_heap= mem_heap_create(1000); dict_names_t fk_list; THD* const thd{current_thd}; mtr_t mtr{thd ? thd_to_trx(thd) : nullptr}; - dict_table_t *table= dict_load_table_one(mtr, name, ignore, fk_list); + /* The tables loaded by this invocation. They remain hidden from + other threads (dict_table_t::LOADING_FK) until all of them have + been loaded, so that no thread can operate on a table whose + foreign key constraints are not fully linked yet. Being hidden, + they cannot be referenced, dropped or evicted by any other thread + (dict_table_can_be_evicted() also skips them), so the pointers + remain valid across the latch releases below. */ + std::vector> loaded; + /* Tables that may be referenced by InnoDB internal SQL must be + loaded without ever releasing the exclusive latch, because this may + be a parse-time table lookup from pars_retrieve_table_def(), and the + non-reentrant internal SQL parser is serialized by the latch. Any + tables loaded for their FOREIGN KEY constraints inherit this, so + that no latch release can occur in the middle of parsing. */ + const bool hold_latch= dict_load_hold_latch(name); + dict_table_t *table= dict_load_table_one(mtr, name, ignore, fk_heap, fk_list, + hold_latch); + if (table) + loaded.emplace_back(table); while (!fk_list.empty()) { const char *f= fk_list.front(); - const span name{f, strlen(f)}; - if (!find_table(name)) - dict_load_table_one(mtr, name, ignore, fk_list); + const span fk_name{f, strlen(f)}; + if (dict_table_t *t= find_table_any(fk_name)) + { + if (t->loading() == dict_table_t::LOADING_DEF && !hold_latch) + { + /* Another thread is still loading the definition of this + table, so its dict_load_foreigns() has not been invoked yet + and the constraints between it and the tables that we loaded + may still be missing. Wait for it, so that we will not + publish a table whose referenced_set is incomplete. On + success its dict_load_foreigns() has since run and linked + whatever it could, same as if we had found it already loaded; + on failure, retry this name from scratch. */ + ut_ad(!t->is_loader()); + if (!wait_for_load(t, true)) + continue; + } + } + else if (dict_table_t *fk_table= + dict_load_table_one(mtr, fk_name, ignore, fk_heap, fk_list, + hold_latch || dict_load_hold_latch(fk_name))) + loaded.emplace_back(fk_table); fk_list.pop_front(); } + mem_heap_free(fk_heap); + + /* All tables related by foreign key constraints have been loaded + and linked; make them visible to other threads, and wake up any + dict_sys_t::wait_for_load() callers blocked on each one's + lock_latch. They will block on the exclusive dict_sys.latch until + our caller releases it, and then see the load as successful. */ + for (dict_table_t *t : loaded) + t->load_finish(); return table; } @@ -2588,10 +2790,6 @@ dict_load_table_on_id( ut_ad(dict_sys.locked()); - /* NOTE that the operation of this function is protected by - dict_sys.latch, and therefore no deadlocks can occur - with other dictionary operations. */ - mtr.start(); /*---------------------------------------------------*/ /* Get the secondary index based on ID for table SYS_TABLES */ @@ -2628,16 +2826,46 @@ dict_load_table_on_id( /* Check if the table id in record is the one searched for */ if (table_id == mach_read_from_8(field)) { + char name_buf[MAX_FULL_NAME_LEN + 1]; field = rec_get_nth_field_old(rec, DICT_FLD__SYS_TABLE_IDS__NAME, &len); - table = dict_sys.load_table( - {reinterpret_cast(field), - len}, ignore_err); + if (UNIV_UNLIKELY(len > sizeof name_buf)) { + /* corrupted record */ + goto func_exit; + } + /* Copy the name and release the page latch before + calling load_table(): it releases the exclusive + dict_sys.latch during the load, and it may block + waiting for a concurrent load of the same table. + Waiting while holding a latched SYS_TABLES page + could deadlock with a DDL thread that holds the + exclusive dict_sys.latch and is waiting for a + latch on this page. */ + memcpy(name_buf, field, len); + ut_d(const auto deleted = rec_get_deleted_flag(rec, + 0)); + btr_pcur_store_position(&pcur, &mtr); + mtr.commit(); + + table = dict_sys.load_table({name_buf, len}, + ignore_err); if (table && table->id != table_id) { - ut_ad(rec_get_deleted_flag(rec, 0)); + /* The record was delete-marked, and the + name has been reused by another table. */ + ut_ad(deleted); table = nullptr; } - if (!table) { + if (table) { + ut_free(pcur.old_rec_buf); + return table; + } + + /* The SYS_TABLE_IDS record may have been stale. + Keep scanning for further records with a + matching table id. */ + mtr.start(); + if (pcur.restore_position(BTR_SEARCH_LEAF, &mtr) + != btr_pcur_t::CORRUPTED) { while (btr_pcur_move_to_next(&pcur, &mtr)) { rec = btr_pcur_get_rec(&pcur); @@ -2649,7 +2877,9 @@ dict_load_table_on_id( } } +func_exit: mtr.commit(); + ut_free(pcur.old_rec_buf); return table; } @@ -2838,6 +3068,9 @@ dict_load_foreign( /*!< in: foreign constraint id */ dict_err_ignore_t ignore_err, /*!< in: error to be ignored */ + mem_heap_t* fk_heap, + /*!< in: heap for the names appended + to fk_tables */ dict_names_t& fk_tables) /*!< out: the foreign key constraint is added to the dictionary cache only if the referenced @@ -2986,10 +3219,10 @@ dict_load_foreign( goto load_error; } - ref_table = dict_sys.find_table( + ref_table = dict_sys.find_table_fk( {foreign->referenced_table_name_lookup, strlen(foreign->referenced_table_name_lookup)}); - for_table = dict_sys.find_table( + for_table = dict_sys.find_table_fk( {foreign->foreign_table_name_lookup, strlen(foreign->foreign_table_name_lookup)}); @@ -2997,11 +3230,15 @@ dict_load_foreign( /* To avoid recursively loading the tables related through the foreign key constraints, the child table name is saved here. The child table will be loaded later, along with its - foreign key constraint. */ + foreign key constraint. The name is copied to fk_heap, + which the caller keeps valid until fk_tables is drained; + ref_table->heap would not do, because ref_table may be + evicted while dict_sys.load_table() releases the exclusive + dict_sys.latch between the drain iterations. */ ut_a(ref_table != NULL); fk_tables.push_back( - mem_heap_strdupl(ref_table->heap, + mem_heap_strdupl(fk_heap, foreign->foreign_table_name_lookup, foreign_table_name_len)); load_error: @@ -3047,6 +3284,9 @@ dict_load_foreigns( bool check_charsets, /*!< in: whether to check charset compatibility */ dict_err_ignore_t ignore_err, /*!< in: error to be ignored */ + mem_heap_t* fk_heap, /*!< in: heap on which the + names appended to fk_tables + are allocated */ dict_names_t& fk_tables) /*!< out: stack of table names which must be loaded @@ -3156,7 +3396,8 @@ dict_load_foreigns( err = len < sizeof fk_id ? dict_load_foreign(mtr, table_name, false, col_names, trx_id, check_recursive, check_charsets, - {fk_id, len}, ignore_err, fk_tables) + {fk_id, len}, ignore_err, fk_heap, + fk_tables) : DB_CORRUPTION; switch (err) { diff --git a/storage/innobase/fts/fts0fts.cc b/storage/innobase/fts/fts0fts.cc index e48e24e691728..0e95447946199 100644 --- a/storage/innobase/fts/fts0fts.cc +++ b/storage/innobase/fts/fts0fts.cc @@ -1524,6 +1524,7 @@ static dberr_t fts_lock_table(trx_t *trx, const char *table_name) DICT_ERR_IGNORE_TABLESPACE)) { dberr_t err= lock_table_for_trx(table, trx, LOCK_X); + ut_ad(!table->loading()); /* Wait for purge threads to stop using the table. */ for (uint n= 15; table->get_ref_count() > 1; ) { @@ -1591,6 +1592,7 @@ static void fts_table_no_ref_count(const char *table_name) if (!table) return; + ut_ad(!table->loading()); while (table->get_ref_count() > 1) { dict_sys.unlock(); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 0f4a5cab1a5a8..e364c23be05bb 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -1254,7 +1254,10 @@ static void innodb_drop_database(handlerton*, char *path) next= table->id_hash; if (strncmp(table->name.m_name, namebuf, len + 1)) continue; - const auto n_handles= table->get_ref_count(); + /* This walks the hash table directly, so table may still be + loading(); its LOADING_MASK bits are not open handles. */ + const auto n_handles= + table->get_ref_count() & ~dict_table_t::LOADING_MASK; const bool locks= !n_handles && lock_table_has_locks(table); if (n_handles || locks) { @@ -12618,6 +12621,17 @@ create_table_info_t::create_foreign_keys() { dict_foreign_set local_fk_set; dict_foreign_set_free local_fk_set_free(local_fk_set); + /* Release the temporary reference taken below on each + foreign->referenced_table left in local_fk_set on failure; the + success path releases them itself and clears the set first, so + this is then a no-op. Declared after local_fk_set_free so it runs + first (LIFO), before dict_foreign_free() invalidates `foreign`. */ + auto unpin_referenced_tables = make_scope_exit([&local_fk_set]() { + for (const dict_foreign_t *foreign : local_fk_set) + if (dict_table_t *rt = foreign->referenced_table) { + rt->release(); + } + }); dberr_t error; ulint number = 1; static const unsigned MAX_COLS_PER_FK = 500; @@ -12685,7 +12699,7 @@ create_table_info_t::create_foreign_keys() ut_ad(alter_info); List_iterator_fast key_it(alter_info->key_list); - dict_table_t* table = dict_sys.find_table({name.str, name.length}); + dict_table_t* table = dict_sys.find_table({name.str, name.length}, true); if (!table) { ib_foreign_warn(m_trx, DB_CANNOT_ADD_CONSTRAINT, create_name, "%s table %s foreign key constraint" @@ -12839,6 +12853,16 @@ create_table_info_t::create_foreign_keys() foreign->referenced_table_name = dict_table_lookup( d, t, &foreign->referenced_table, foreign->heap); + if (foreign->referenced_table) { + /* Pin it so dict_sys.load_table() for a later + constraint in this loop cannot evict it while + referenced_set does not yet reflect this constraint. + unpin_referenced_tables releases this on failure; + made permanent via dict_sys.prevent_eviction() only + once the constraint is actually committed below. */ + foreign->referenced_table->acquire(); + } + if (!foreign->referenced_table && m_trx->check_foreigns) { char buf[MAX_TABLE_NAME_LEN + 1] = ""; char* bufend; @@ -13041,6 +13065,14 @@ create_table_info_t::create_foreign_keys() local_fk_set.end()); std::for_each(local_fk_set.begin(), local_fk_set.end(), dict_foreign_add_to_referenced_table()); + for (const dict_foreign_t *foreign : local_fk_set) { + if (dict_table_t *rt = foreign->referenced_table) { + /* Committed: make it permanently + non-evictable, replacing the temporary pin. */ + dict_sys.prevent_eviction(rt); + rt->release(); + } + } local_fk_set.clear(); dict_mem_table_fill_foreign_vcol_set(table); @@ -13170,6 +13202,13 @@ int create_table_info_t::create_table(bool create_fk) } create_fk&= !m_creating_stub; + + /* Pin the newly created table: dict_sys.load_table() inside + create_foreign_keys() and in the loop below may temporarily + release the exclusive dict_sys.latch, during which + dict_sys.evict_table_LRU() could evict an unreferenced table. */ + m_table->acquire(); + dberr_t err = create_fk ? create_foreign_keys() : DB_SUCCESS; if (err == DB_SUCCESS) { @@ -13179,17 +13218,24 @@ int create_table_info_t::create_table(bool create_fk) /* Check that also referencing constraints are ok */ dict_names_t fk_tables; mtr_t mtr{m_trx}; + /* The names in fk_tables must survive the temporary + release of the exclusive dict_sys.latch inside + dict_sys.load_table() in the loop below. */ + mem_heap_t* fk_heap = mem_heap_create(1000); err = dict_load_foreigns(mtr, m_table_name, nullptr, m_trx->id, true, - ignore_err, fk_tables); + ignore_err, fk_heap, fk_tables); while (err == DB_SUCCESS && !fk_tables.empty()) { dict_sys.load_table( {fk_tables.front(), strlen(fk_tables.front())}, ignore_err); fk_tables.pop_front(); } + mem_heap_free(fk_heap); } + m_table->release(); + switch (err) { case DB_PARENT_NO_INDEX: push_warning_printf( @@ -13972,6 +14018,7 @@ int ha_innobase::delete_table(const char *name) dict_sys.lock(SRW_LOCK_CALL); + ut_ad(!table->loading()); if (!table->release() && err == DB_SUCCESS) { /* Wait for purge threads to stop using the table. */ @@ -14247,6 +14294,7 @@ int ha_innobase::truncate() if (error == DB_SUCCESS) error= lock_table_for_trx(ib_table, trx, LOCK_X); + ut_ad(!ib_table->loading()); const bool fts= error == DB_SUCCESS && ib_table->flags2 & (DICT_TF2_FTS_HAS_DOC_ID | DICT_TF2_FTS); const bool pause_purge= error == DB_SUCCESS && ib_table->get_ref_count() > 1; @@ -14333,13 +14381,17 @@ int ha_innobase::truncate() m_prebuilt->table->def_trx_id= def_trx_id; } dict_names_t fk_tables; + /* The names in fk_tables must survive the temporary release of the + exclusive dict_sys.latch inside dict_sys.load_table() below. */ + mem_heap_t *fk_heap= mem_heap_create(1000); { mtr_t mtr{trx}; dict_load_foreigns(mtr, m_prebuilt->table->name.m_name, nullptr, 1, true, - DICT_ERR_IGNORE_FK_NOKEY, fk_tables); + DICT_ERR_IGNORE_FK_NOKEY, fk_heap, fk_tables); } for (const char *f : fk_tables) dict_sys.load_table({f, strlen(f)}); + mem_heap_free(fk_heap); } if (fts) @@ -14461,6 +14513,7 @@ ha_innobase::rename_table( t = dict_table_open_on_name( norm_from, false, DICT_ERR_IGNORE_FK_NOKEY); if (t) { + ut_ad(!t->loading()); error = lock_table_children(t, trx); if (error == DB_SUCCESS) { error = lock_table_for_trx(t, trx, LOCK_X); diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 2a8caedb1c8b0..0532b0a0b7187 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -6647,6 +6647,7 @@ prepare_inplace_alter_table_dict( const bool have_fts = user_table->flags2 & (DICT_TF2_FTS_HAS_DOC_ID | DICT_TF2_FTS); + ut_ad(!user_table->loading()); const bool pause_purge = have_fts || user_table->get_ref_count() > 1; /* Acquire a lock on the table before creating any indexes. */ bool table_lock_failed = false; @@ -7206,7 +7207,7 @@ prepare_inplace_alter_table_dict( if (dict_sys.find_table( {ctx->new_table->name.m_name, - strlen(ctx->new_table->name.m_name)})) { + strlen(ctx->new_table->name.m_name)}, true)) { my_error(ER_TABLE_EXISTS_ERROR, MYF(0), ctx->new_table->name.m_name); goto new_clustered_failed; @@ -10181,11 +10182,15 @@ innobase_update_foreign_cache( dictionary cache (work around the lack of WL#6049). */ dict_names_t fk_tables; mtr_t mtr{ctx->trx}; + /* The names in fk_tables must survive the temporary release of + the exclusive dict_sys.latch inside dict_sys.load_table() in the + loop below. */ + mem_heap_t* fk_heap = mem_heap_create(1000); err = dict_load_foreigns(mtr, user_table->name.m_name, ctx->col_names, 1, true, DICT_ERR_IGNORE_FK_NOKEY, - fk_tables); + fk_heap, fk_tables); if (err == DB_CANNOT_ADD_CONSTRAINT) { fk_tables.clear(); @@ -10196,7 +10201,7 @@ innobase_update_foreign_cache( err = dict_load_foreigns(mtr, user_table->name.m_name, ctx->col_names, 1, false, DICT_ERR_IGNORE_NONE, - fk_tables); + fk_heap, fk_tables); /* The load with "charset_check" off is successful, warn the user that the foreign key has loaded with mis-matched @@ -10229,6 +10234,8 @@ innobase_update_foreign_cache( fk_tables.pop_front(); } + mem_heap_free(fk_heap); + DBUG_RETURN(err); } diff --git a/storage/innobase/handler/i_s.cc b/storage/innobase/handler/i_s.cc index fe81dbc65eae0..007f25748d690 100644 --- a/storage/innobase/handler/i_s.cc +++ b/storage/innobase/handler/i_s.cc @@ -4698,6 +4698,8 @@ i_s_dict_fill_sys_tablestats(THD* thd, dict_table_t *table, Field **fields= table_to_fill->field; + ut_ad(!table->loading()); + { table->stats_shared_lock(); auto _ = make_scope_exit([table]() { diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h index 091f2e87a0f29..1874d909bbb3a 100644 --- a/storage/innobase/include/dict0dict.h +++ b/storage/innobase/include/dict0dict.h @@ -1366,11 +1366,23 @@ class dict_sys_t (should only happen during the rollback of CREATE...SELECT) */ dict_table_t *acquire_temporary_table(table_id_t id) const noexcept; - /** Look up a persistent table. + /** Look up a persistent table, including one whose definition is + still being loaded (dict_table_t::loading()). Of such a table, only + the name, id and hash pointers may be accessed. @param id table ID @return table @retval nullptr if not cached */ - dict_table_t *find_table(table_id_t id) const noexcept; + dict_table_t *find_table_any(table_id_t id) const noexcept; + + /** Look up a persistent table, waiting for a concurrent load of it + to finish if necessary. + @param id table ID + @param excl whether the caller holds dict_sys.latch in exclusive + mode (lock()/unlock()); if false, the caller holds it + in shared mode (freeze()/unfreeze()) + @return table + @retval nullptr if not cached, or a concurrent load of it failed */ + dict_table_t *find_table(table_id_t id, bool excl) noexcept; bool is_initialised() const noexcept { return m_initialised; } @@ -1416,7 +1428,7 @@ class dict_sys_t /** Move a table to the non-LRU list from the LRU list. */ void prevent_eviction(dict_table_t *table) { - ut_d(locked()); + ut_ad(locked()); ut_ad(find(table)); if (!table->can_be_evicted) return; @@ -1425,6 +1437,20 @@ class dict_sys_t UT_LIST_ADD_LAST(table_non_LRU, table); } + /** Move a table to the LRU list from the non-LRU list. + This is the inverse of prevent_eviction(), invoked by + dict_load_table_one() when a fully loaded table replaces + its non-evictable loading stub. */ + void allow_eviction(dict_table_t *table) + { + ut_ad(locked()); + ut_ad(find(table)); + ut_ad(!table->can_be_evicted); + table->can_be_evicted= true; + UT_LIST_REMOVE(table_non_LRU, table); + UT_LIST_ADD_FIRST(table_LRU, table); + } + #ifdef UNIV_DEBUG /** @return whether the current thread is holding the latch */ bool frozen() const noexcept { return latch.have_any(); } @@ -1488,17 +1514,62 @@ class dict_sys_t @return number of tables evicted */ ulint evict_table_LRU(bool half) noexcept; - /** Look up a table in the dictionary cache. + /** Look up a table in the dictionary cache, including tables whose + definition is still being loaded (dict_table_t::loading()). Of such + a table, only the name, id and hash pointers may be accessed. @param name table name @return table handle @retval nullptr if not found */ - dict_table_t *find_table(const span &name) const noexcept; + dict_table_t *find_table_any(const span &name) const noexcept; - /** Look up or load a table definition + /** Look up a table in the dictionary cache, waiting for a + concurrent load of it to finish if necessary. + @param name table name + @param excl whether the caller holds dict_sys.latch in exclusive + mode (lock()/unlock()); if false, the caller holds it + in shared mode (freeze()/unfreeze()) + @return table handle + @retval nullptr if not found, or a concurrent load of it failed */ + dict_table_t *find_table(const span &name, bool excl) noexcept; + + /** Look up a table for FOREIGN KEY constraint linkage. + Like find_table(), except that a table whose definition is complete + and that is only waiting for the tables related to it by FOREIGN KEY + constraints to be loaded (dict_table_t::LOADING_FK) is visible: + linking constraints into such a table is protected by the exclusive + latch, which both the linking thread and the loading thread hold + while modifying foreign_set/referenced_set. @param name table name - @param ignore errors to ignore when loading the table definition @return table handle @retval nullptr if not found */ + dict_table_t *find_table_fk(const span &name) const noexcept; + + /** Wait for a concurrent dict_load_table_one() of a specific table + to finish. To be invoked with the latch held in the given mode, + after observing a table with dict_table_t::loading() set; releases + and reacquires the latch, in that same mode, around the wait. + @param table a table observed to be loading; must not be nullptr + @param excl whether the caller holds the latch in exclusive mode + (lock()/unlock()); if false, the caller holds it in + shared mode (freeze()/unfreeze()) + @return whether the load succeeded; the latch is held on return + either way, in the mode the caller held it in, so on success table + is still valid and usable + @retval false if the load failed; table may already have been freed */ + bool wait_for_load(dict_table_t *table, bool excl) noexcept; + + /** Look up or load a table definition. The table and any tables + that were loaded because they are related to it by FOREIGN KEY + constraints become visible to other threads atomically, only after + all of them have been loaded. + @param name table name + @param ignore errors to ignore when loading the table definition + @return table handle + @retval nullptr if not found + @note The exclusive latch may be temporarily released and reacquired, + both while waiting for a concurrent load of the same table and during + the I/O phases of loading; any pointers into the cache that the + caller obtained earlier may be stale on return. */ dict_table_t *load_table(const span &name, dict_err_ignore_t ignore= DICT_ERR_IGNORE_NONE) noexcept; diff --git a/storage/innobase/include/dict0dict.inl b/storage/innobase/include/dict0dict.inl index fa2ba394bdbb4..0fd57d499e2dc 100644 --- a/storage/innobase/include/dict0dict.inl +++ b/storage/innobase/include/dict0dict.inl @@ -1076,8 +1076,8 @@ dict_table_is_file_per_table( /** Acquire the table handle. */ inline void dict_table_t::acquire() { - ut_d(const auto old=) n_ref_count++; - ut_ad(old || dict_sys.frozen()); + ut_d(const auto old=) n_ref_count.fetch_add(1, std::memory_order_relaxed); + ut_ad((old & ~LOADING_MASK) || dict_sys.frozen()); } /** Release the table handle. @@ -1086,11 +1086,33 @@ inline bool dict_table_t::release() { - auto n = n_ref_count--; - ut_ad(n > 0); + auto n = n_ref_count.fetch_sub(1, std::memory_order_relaxed); + ut_ad(n & ~LOADING_MASK); return n == 1; } +/** Clear loading(), and release the exclusive lock_latch taken for +the load, making the table fully visible. */ +inline void dict_table_t::load_finish() noexcept +{ + ut_ad(dict_sys.locked()); + ut_ad(is_loader()); + ut_d(const auto old=) + n_ref_count.fetch_sub(LOADING_FK, std::memory_order_relaxed); + ut_ad((old & LOADING_MASK) == LOADING_FK); + lock_mutex_unlock(); +} + +/** Mark a failed load, and release the exclusive lock_latch taken +for it. */ +inline void dict_table_t::mark_load_failed() noexcept +{ + ut_ad(dict_sys.locked()); + ut_ad(is_loader()); + n_ref_count.fetch_or(LOAD_FAILED, std::memory_order_relaxed); + lock_mutex_unlock(); +} + /** Encode the number of columns and number of virtual columns in a 4 bytes value. We could do this because the number of columns in InnoDB is limited to 1017 diff --git a/storage/innobase/include/dict0load.h b/storage/innobase/include/dict0load.h index aec0b992e503b..b6e382957d408 100644 --- a/storage/innobase/include/dict0load.h +++ b/storage/innobase/include/dict0load.h @@ -98,6 +98,11 @@ dict_load_foreigns( bool check_charsets, /*!< in: whether to check charset compatibility */ dict_err_ignore_t ignore_err, /*!< in: error to be ignored */ + mem_heap_t* fk_heap, /*!< in: heap on which the + names appended to fk_tables + are allocated; must remain + valid until fk_tables has + been drained by the caller */ dict_names_t& fk_tables) /*!< out: stack of table names which must be loaded subsequently to load all the diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 8fbbd1fb744ac..6dbf641d7162d 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -1928,9 +1928,13 @@ typedef enum { zero-initialized in dict_table_t::create(). */ struct dict_table_t { - /** Get reference count. - @return current value of n_ref_count */ - inline uint32_t get_ref_count() const { return n_ref_count; } + /** Get reference count. The caller must already know the table + is not loading(), typically because it was obtained via + find_table() or load_table(); a caller that may see a loading + table must mask off LOADING_MASK itself. + @return current value of the reference count */ + inline uint32_t get_ref_count() const + { return n_ref_count.load(std::memory_order_relaxed); } /** Acquire the table handle. */ inline void acquire(); @@ -2261,6 +2265,85 @@ struct dict_table_t { or ONLINE_INDEX_ABORTED_DROPPED. */ unsigned drop_aborted:1; + /** columns/indexes are being loaded by dict_load_table_one(); + only the loading thread may access the object, which holds + lock_latch in exclusive mode */ + static constexpr uint32_t LOADING_DEF= 1U << 30; + /** the definition is complete; only the FOREIGN KEY related + tables are still being loaded by dict_sys_t::load_table(). + find_table_fk() exposes the table so constraints can be linked + into it under the exclusive dict_sys.latch */ + static constexpr uint32_t LOADING_FK= 2U << 30; + /** the load failed; the stub is unlinked once every reference + taken before the failure (see try_pin_for_wait()) is released */ + static constexpr uint32_t LOAD_FAILED= 3U << 30; + /** mask of LOADING_DEF|LOADING_FK|LOAD_FAILED within n_ref_count */ + static constexpr uint32_t LOADING_MASK= 3U << 30; + + /** @return the progress of loading the table definition, 0 if + fully loaded */ + uint32_t loading() const noexcept + { return n_ref_count.load(std::memory_order_relaxed) & LOADING_MASK; } + +#ifdef UNIV_DEBUG + /** The thread that set the loading flag */ + pthread_t load_thread; + + /** @return whether the current thread is loading this table */ + bool is_loader() const + { return loading() && pthread_equal(pthread_self(), load_thread); } +#endif + + /** Try to pin the table only to wait on lock_latch for a + concurrent load to finish; refuses once LOAD_FAILED, so that a + late pin cannot defeat the loader's drain to zero references. + @return whether a reference was acquired */ + bool try_pin_for_wait() noexcept + { + uint32_t n= n_ref_count.load(std::memory_order_relaxed); + while (!((n & LOADING_MASK) == LOAD_FAILED)) + if (n_ref_count.compare_exchange_weak(n, n + 1, + std::memory_order_relaxed, + std::memory_order_relaxed)) + return true; + return false; + } + + /** Mark a freshly created, not yet published table as loading. */ + void start_loading() noexcept + { + ut_ad(!n_ref_count.load(std::memory_order_relaxed)); + n_ref_count.store(LOADING_DEF, std::memory_order_relaxed); + } + + /** Advance loading() from LOADING_DEF to LOADING_FK. */ + void advance_to_loading_fk() noexcept + { + ut_d(const auto old=) + n_ref_count.fetch_add(LOADING_FK - LOADING_DEF, + std::memory_order_relaxed); + ut_ad((old & LOADING_MASK) == LOADING_DEF); + } + + /** Clear loading(), and release the exclusive lock_latch taken + for the load, making the table fully visible. Defined in + dict0dict.inl, where dict_sys is visible for the assertions. */ + void load_finish() noexcept; + + /** Mark a failed load, and release the exclusive lock_latch taken + for it. Defined in dict0dict.inl, where dict_sys is visible for + the assertions. */ + void mark_load_failed() noexcept; + + /** @return whether every reference taken before mark_load_failed() + (see try_pin_for_wait()) has been released, so the stub can be + freed. Only to be called after mark_load_failed(). */ + bool load_failed_refs_cleared() const noexcept + { + ut_ad(loading() == LOAD_FAILED); + return n_ref_count.load(std::memory_order_relaxed) == LOAD_FAILED; + } + /** Array of column descriptions. */ dict_col_t* cols; @@ -2411,9 +2494,13 @@ struct dict_table_t { #else typedef srw_spin_lock_low lock_latch_type; #endif - /** RW-lock protecting locks and statistics on this table */ + /** RW-lock protecting locks and statistics on this table. + Also held in exclusive mode by dict_load_table_one() while loading() + is set, so that dict_sys_t::load_table() can wait for a concurrent + load of this table by acquiring it in shared mode. */ lock_latch_type lock_latch; public: + /** The next DB_ROW_ID value */ Atomic_counter row_id{0}; /** Autoinc counter value to give to the next inserted row. */ @@ -2456,10 +2543,13 @@ struct dict_table_t { @see trx_lock_t::trx_locks */ Atomic_counter n_rec_locks; private: - /** Count of how many handles are opened to this table. Dropping of the - table is NOT allowed until this count gets to zero. MySQL does NOT - itself check the number of open handles at DROP. */ - Atomic_counter n_ref_count; + /** Count of how many handles are opened to this table, in the low + 30 bits; the top 2 bits hold loading() (see above). Dropping of the + table is NOT allowed until the reference count reaches zero. MySQL + does NOT itself check the number of open handles at DROP. + The two live in one atomic so that acquire()/release() are + unconditional and never touch the top bits. */ + std::atomic n_ref_count; public: /** List of locks on the table. Protected by lock_sys.assert_locked(lock). */ table_lock_list_t locks; diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 2f6585032c253..c6a33d28c197c 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -4705,7 +4705,7 @@ void lock_release(trx_t *trx) dict_sys.lock(SRW_LOCK_CALL); LockMutexGuard g{SRW_LOCK_CALL}; for (const table_id_t id : to_evict) - if (dict_table_t *table= dict_sys.find_table(id)) + if (dict_table_t *table= dict_sys.find_table(id, true)) if (!table->get_ref_count() && !UT_LIST_GET_LEN(table->locks)) dict_sys.remove(table, true); dict_sys.unlock(); diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc index b5d43d5c3c5a2..14a13b6f32920 100644 --- a/storage/innobase/row/row0merge.cc +++ b/storage/innobase/row/row0merge.cc @@ -4083,6 +4083,7 @@ row_merge_drop_indexes( ut_ad(dict_index_get_online_status(index) == ONLINE_INDEX_COMPLETE); /* the caller should have an open handle to the table */ + ut_ad(!table->loading()); ut_ad(table->get_ref_count() >= 1); /* It is possible that table->n_ref_count > 1 when diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index 4b99a5756cc43..7111d4540d493 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -2834,6 +2834,12 @@ row_rename_table_for_mysql( /* We only want to switch off some of the type checking in an ALTER TABLE, not in a RENAME. */ dict_names_t fk_tables; + /* The names in fk_tables must survive the temporary + release of the exclusive dict_sys.latch inside + dict_sys.load_table() in the drain loop below. */ + if (!heap) { + heap = mem_heap_create(1000); + } { mtr_t mtr{trx}; err = dict_load_foreigns(mtr, new_name, nullptr, @@ -2843,7 +2849,7 @@ row_rename_table_for_mysql( fk == RENAME_ALTER_COPY ? DICT_ERR_IGNORE_NONE : DICT_ERR_IGNORE_FK_NOKEY, - fk_tables); + heap, fk_tables); } if (err != DB_SUCCESS) { if (old_is_tmp) { diff --git a/storage/innobase/row/row0purge.cc b/storage/innobase/row/row0purge.cc index 1991dbb870575..f428dd60aee7a 100644 --- a/storage/innobase/row/row0purge.cc +++ b/storage/innobase/row/row0purge.cc @@ -117,7 +117,7 @@ row_purge_remove_clust_if_poss_low( if (table_id) { retry: dict_sys.lock(SRW_LOCK_CALL); - table = dict_sys.find_table(table_id); + table = dict_sys.find_table(table_id, true); if (!table) { dict_sys.unlock(); } else if (table->n_rec_locks) { @@ -167,6 +167,7 @@ row_purge_remove_clust_if_poss_low( &node->pcur, nullptr, &mtr); if (space_id) { if (table) { + ut_ad(!table->loading()); if (table->get_ref_count() == 0) { dict_sys.remove(table); } else if (table->space_id == space_id) { diff --git a/storage/innobase/trx/trx0purge.cc b/storage/innobase/trx/trx0purge.cc index 02706abb8e50d..e15b41cf01a9f 100644 --- a/storage/innobase/trx/trx0purge.cc +++ b/storage/innobase/trx/trx0purge.cc @@ -1190,7 +1190,7 @@ static purge_table trx_purge_table_open(table_id_t table_id, for (;;) { dict_sys.freeze(SRW_LOCK_CALL); - result.table= dict_sys.find_table(table_id); + result.table= dict_sys.find_table(table_id, false); if (result.table) break; dict_sys.unfreeze(); diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index e95df263ff278..125cc14d61e9b 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -1304,13 +1304,14 @@ void trx_t::evict_table(table_id_t table_id, bool reset_only) { ut_ad(in_rollback); - dict_table_t* table = dict_sys.find_table(table_id); + dict_table_t* table = dict_sys.find_table(table_id, true); if (!table) { return; } table->def_trx_id = 0; + ut_ad(!table->loading()); if (auto ref_count = table->get_ref_count()) { /* This must be a DDL operation that is being rolled back in an active connection. */ diff --git a/storage/innobase/trx/trx0undo.cc b/storage/innobase/trx/trx0undo.cc index 611863d5e2415..503144c14ce67 100644 --- a/storage/innobase/trx/trx0undo.cc +++ b/storage/innobase/trx/trx0undo.cc @@ -306,7 +306,7 @@ void UndorecApplier::apply_undo_rec(const trx_undo_rec_t *rec, uint16_t offset) &cmpl_info, &updated_extern, &undo_no, &table_id); dict_sys.freeze(SRW_LOCK_CALL); - dict_table_t *table= dict_sys.find_table(table_id); + dict_table_t *table= dict_sys.find_table(table_id, false); dict_sys.unfreeze(); ut_ad(table);