MDEV-32899 InnoDB is holding shared dict_sys.latch while waiting for FOREIGN KEY child table lock on DDL

lock_table_children(): A new function to lock all child tables of a table.
We will only hold dict_sys.latch while traversing
dict_table_t::referenced_set. To prevent a race condition with
std::set::erase() we will copy the pointers to the child tables to a
local vector. Once we have acquired MDL and references to all child tables,
we can safely release dict_sys.latch, wait for the locks, and finally
release the references.

dict_acquire_mdl_shared(): A new variant that takes mdl_context as a
parameter.

lock_table_for_trx(): Assert that we are not holding dict_sys.latch.

ha_innobase::truncate(): When foreign_key_checks=ON, assert that
no child tables exist (other than the current table).
In any case, we will invoke lock_table_children()
so that the child table metadata can be safely updated.
(It is possible that a child table is being created concurrently
with TRUNCATE TABLE.)

ha_innobase::delete_table(): Before and after acquiring exclusive
locks on the current table as well as all child tables,
check that FOREIGN KEY constraints will not be violated.
In this way, we can reject impossible DROP TABLE without having to
wait for locks first.

This fixes up commit 2ca1123464 (MDEV-26217)
and commit c3c53926c4 (MDEV-26554).
This commit is contained in:
Marko Mäkelä 2024-02-01 15:48:46 +02:00 committed by Daniel Black
commit b2654ba826
8 changed files with 248 additions and 94 deletions

View file

@ -13395,6 +13395,49 @@ ha_innobase::discard_or_import_tablespace(
DBUG_RETURN(0);
}
/** Report a DROP TABLE failure due to a FOREIGN KEY constraint.
@param name table name
@param foreign constraint */
ATTRIBUTE_COLD
static void delete_table_cannot_drop_foreign(const table_name_t &name,
const dict_foreign_t &foreign)
{
mysql_mutex_lock(&dict_foreign_err_mutex);
rewind(dict_foreign_err_file);
ut_print_timestamp(dict_foreign_err_file);
fputs(" Cannot drop table ", dict_foreign_err_file);
ut_print_name(dict_foreign_err_file, nullptr, name.m_name);
fputs("\nbecause it is referenced by ", dict_foreign_err_file);
ut_print_name(dict_foreign_err_file, nullptr, foreign.foreign_table_name);
putc('\n', dict_foreign_err_file);
mysql_mutex_unlock(&dict_foreign_err_mutex);
}
/** Check if DROP TABLE would fail due to a FOREIGN KEY constraint.
@param table table to be dropped
@param sqlcom thd_sql_command(current_thd)
@return whether child tables that refer to this table exist */
static bool delete_table_check_foreigns(const dict_table_t &table,
enum_sql_command sqlcom)
{
const bool drop_db{sqlcom == SQLCOM_DROP_DB};
for (const auto foreign : table.referenced_set)
{
/* We should allow dropping a referenced table if creating
that referenced table has failed for some reason. For example
if referenced table is created but it column types that are
referenced do not match. */
if (foreign->foreign_table == &table ||
(drop_db &&
dict_tables_have_same_db(table.name.m_name,
foreign->foreign_table_name_lookup)))
continue;
delete_table_cannot_drop_foreign(table.name, *foreign);
return true;
}
return false;
}
/** DROP TABLE (possibly as part of DROP DATABASE, CREATE/ALTER TABLE)
@param name table name
@ -13410,6 +13453,7 @@ int ha_innobase::delete_table(const char *name)
DBUG_EXECUTE_IF("test_normalize_table_name_low",
test_normalize_table_name_low(););
const enum_sql_command sqlcom= enum_sql_command(thd_sql_command(thd));
trx_t *parent_trx= check_trx_exists(thd);
dict_table_t *table;
@ -13446,6 +13490,13 @@ int ha_innobase::delete_table(const char *name)
DBUG_RETURN(0);
}
if (parent_trx->check_foreigns &&
delete_table_check_foreigns(*table, sqlcom))
{
dict_sys.unlock();
DBUG_RETURN(HA_ERR_ROW_IS_REFERENCED);
}
table->acquire();
dict_sys.unlock();
@ -13478,14 +13529,7 @@ int ha_innobase::delete_table(const char *name)
/* FOREIGN KEY constraints cannot exist on partitioned tables. */;
#endif
else
{
dict_sys.freeze(SRW_LOCK_CALL);
for (const dict_foreign_t* f : table->referenced_set)
if (dict_table_t* child= f->foreign_table)
if ((err= lock_table_for_trx(child, trx, LOCK_X)) != DB_SUCCESS)
break;
dict_sys.unfreeze();
}
err= lock_table_children(table, trx);
}
dict_table_t *table_stats= nullptr, *index_stats= nullptr;
@ -13495,7 +13539,6 @@ int ha_innobase::delete_table(const char *name)
const bool fts= err == DB_SUCCESS &&
(table->flags2 & (DICT_TF2_FTS_HAS_DOC_ID | DICT_TF2_FTS));
const enum_sql_command sqlcom= enum_sql_command(thd_sql_command(thd));
if (fts)
{
@ -13653,36 +13696,16 @@ err_exit:
DBUG_RETURN(convert_error_code_to_mysql(err, 0, NULL));
}
if (!table->no_rollback() && trx->check_foreigns)
if (!table->no_rollback())
{
const bool drop_db= sqlcom == SQLCOM_DROP_DB;
for (auto foreign : table->referenced_set)
if (trx->check_foreigns && delete_table_check_foreigns(*table, sqlcom))
{
/* We should allow dropping a referenced table if creating
that referenced table has failed for some reason. For example
if referenced table is created but it column types that are
referenced do not match. */
if (foreign->foreign_table == table ||
(drop_db &&
dict_tables_have_same_db(table->name.m_name,
foreign->foreign_table_name_lookup)))
continue;
mysql_mutex_lock(&dict_foreign_err_mutex);
rewind(dict_foreign_err_file);
ut_print_timestamp(dict_foreign_err_file);
fputs(" Cannot drop table ", dict_foreign_err_file);
ut_print_name(dict_foreign_err_file, trx, table->name.m_name);
fputs("\nbecause it is referenced by ", dict_foreign_err_file);
ut_print_name(dict_foreign_err_file, trx, foreign->foreign_table_name);
putc('\n', dict_foreign_err_file);
mysql_mutex_unlock(&dict_foreign_err_mutex);
err= DB_CANNOT_DROP_CONSTRAINT;
goto err_exit;
}
}
if (!table->no_rollback())
err= trx->drop_table_foreign(table->name);
}
if (err == DB_SUCCESS && table_stats && index_stats)
err= trx->drop_table_statistics(table->name);
@ -13801,6 +13824,19 @@ int ha_innobase::truncate()
update_thd();
#ifdef UNIV_DEBUG
if (!thd_test_options(m_user_thd, OPTION_NO_FOREIGN_KEY_CHECKS))
{
/* fk_truncate_illegal_if_parent() should have failed in
Sql_cmd_truncate_table::handler_truncate() if foreign_key_checks=ON
and child tables exist. */
dict_sys.freeze(SRW_LOCK_CALL);
for (const auto foreign : m_prebuilt->table->referenced_set)
ut_ad(foreign->foreign_table == m_prebuilt->table);
dict_sys.unfreeze();
}
#endif
if (is_read_only())
DBUG_RETURN(HA_ERR_TABLE_READONLY);
@ -13883,14 +13919,7 @@ int ha_innobase::truncate()
dict_table_t *table_stats = nullptr, *index_stats = nullptr;
MDL_ticket *mdl_table = nullptr, *mdl_index = nullptr;
dberr_t error= DB_SUCCESS;
dict_sys.freeze(SRW_LOCK_CALL);
for (const dict_foreign_t *f : ib_table->referenced_set)
if (dict_table_t *child= f->foreign_table)
if ((error= lock_table_for_trx(child, trx, LOCK_X)) != DB_SUCCESS)
break;
dict_sys.unfreeze();
dberr_t error= lock_table_children(ib_table, trx);
if (error == DB_SUCCESS)
error= lock_table_for_trx(ib_table, trx, LOCK_X);
@ -14081,16 +14110,7 @@ ha_innobase::rename_table(
/* There is no need to lock any FOREIGN KEY child tables. */
} else if (dict_table_t *table = dict_table_open_on_name(
norm_from, false, DICT_ERR_IGNORE_FK_NOKEY)) {
dict_sys.freeze(SRW_LOCK_CALL);
for (const dict_foreign_t* f : table->referenced_set) {
if (dict_table_t* child = f->foreign_table) {
error = lock_table_for_trx(child, trx, LOCK_X);
if (error != DB_SUCCESS) {
break;
}
}
}
dict_sys.unfreeze();
error = lock_table_children(table, trx);
if (error == DB_SUCCESS) {
error = lock_table_for_trx(table, trx, LOCK_X);
}