MDEV-36122 Assertion ctx0->old_table->get_ref_count() == 1

trx_purge_close_tables(): Before releasing any metadata locks (MDL),
release all table references, in case an ALTER TABLE…ALGORITHM=COPY
operation has confused our logic.

trx_purge_table_acquire(), trx_purge_table_open(): Do not acquire any
table reference before successfully acquiring a necessary metadata lock.
In this way, if purge is waiting for MDL, a concurrent
ha_innobase::commit_inplace_alter_table(commit=true) that is holding
a conflicting MDL_EXCLUSIVE will only observe its own reference on
the table that it may need to replace.

dict_acquire_mdl_shared<false>(): Unless we hold an MDL or one is
not needed, do not hold a table reference when releasing dict_sys.latch.
After loading a table into the dictionary cache, we will look up the
table again, because the table could be evicted or dropped while we
were not holding any dict_sys.latch.

Reviewed by: Debarun Banerjee
This commit is contained in:
Marko Mäkelä 2025-03-26 14:30:46 +02:00
commit 4a21cba7fc
2 changed files with 84 additions and 66 deletions

View file

@ -637,35 +637,37 @@ dict_acquire_mdl_shared(dict_table_t *table,
MDL_context *mdl_context, MDL_ticket **mdl,
dict_table_op_t table_op)
{
table_id_t table_id= table->id;
char db_buf[NAME_LEN + 1], db_buf1[NAME_LEN + 1];
char tbl_buf[NAME_LEN + 1], tbl_buf1[NAME_LEN + 1];
size_t db_len, tbl_len;
bool unaccessible= false;
if (!table->parse_name<!trylock>(db_buf, tbl_buf, &db_len, &tbl_len))
/* The name of an intermediate table starts with #sql */
return table;
retry:
if (!unaccessible && (!table->is_readable() || table->corrupted))
ut_ad(!trylock == dict_sys.frozen());
ut_ad(trylock || table->get_ref_count());
if (!table->is_readable() || table->corrupted)
{
if (!trylock)
table->release();
if (*mdl)
{
mdl_context->release_lock(*mdl);
*mdl= nullptr;
}
unaccessible= true;
return nullptr;
}
const table_id_t table_id{table->id};
if (!trylock)
{
table->release();
if (unaccessible)
return nullptr;
if (!trylock)
dict_sys.unfreeze();
}
{
MDL_request request;
@ -691,11 +693,37 @@ retry:
}
}
size_t db1_len, tbl1_len;
lookup:
dict_sys.freeze(SRW_LOCK_CALL);
table= dict_sys.find_table(table_id);
if (table)
{
if (!table->is_accessible())
{
table= nullptr;
unlock_and_return_without_mdl:
if (trylock)
dict_sys.unfreeze();
return_without_mdl:
if (*mdl)
{
mdl_context->release_lock(*mdl);
*mdl= nullptr;
}
return table;
}
table->acquire();
if (!table && table_op != DICT_TABLE_OP_OPEN_ONLY_IF_CACHED)
if (!table->parse_name<true>(db_buf1, tbl_buf1, &db1_len, &tbl1_len))
{
/* The table was renamed to #sql prefix.
Release MDL (if any) for the old name and return. */
goto unlock_and_return_without_mdl;
}
}
else if (table_op != DICT_TABLE_OP_OPEN_ONLY_IF_CACHED)
{
dict_sys.unfreeze();
dict_sys.lock(SRW_LOCK_CALL);
@ -703,31 +731,15 @@ retry:
table_op == DICT_TABLE_OP_LOAD_TABLESPACE
? DICT_ERR_IGNORE_RECOVER_LOCK
: DICT_ERR_IGNORE_FK_NOKEY);
if (table)
table->acquire();
dict_sys.unlock();
/* At this point, the freshly loaded table may already have been evicted.
We must look it up again while holding a shared dict_sys.latch. We keep
trying this until the table is found in the cache or it cannot be found
in the dictionary (because the table has been dropped or rebuilt). */
if (table)
goto lookup;
if (!trylock)
dict_sys.freeze(SRW_LOCK_CALL);
}
if (!table || !table->is_accessible())
{
return_without_mdl:
if (trylock)
dict_sys.unfreeze();
if (*mdl)
{
mdl_context->release_lock(*mdl);
*mdl= nullptr;
}
return nullptr;
}
size_t db1_len, tbl1_len;
if (!table->parse_name<true>(db_buf1, tbl_buf1, &db1_len, &tbl1_len))
{
/* The table was renamed to #sql prefix.
Release MDL (if any) for the old name and return. */
goto return_without_mdl;
}

View file

@ -1052,16 +1052,25 @@ inline trx_purge_rec_t purge_sys_t::fetch_next_rec()
/** Close all tables that were opened in a purge batch for a worker.
@param node purge task context
@param thd purge coordinator thread handle */
static void trx_purge_close_tables(purge_node_t *node, THD *thd)
static void trx_purge_close_tables(purge_node_t *node, THD *thd) noexcept
{
for (auto &t : node->tables)
{
if (!t.second.first);
else if (t.second.first == reinterpret_cast<dict_table_t*>(-1));
else
dict_table_t *table= t.second.first;
if (table != nullptr && table != reinterpret_cast<dict_table_t*>(-1))
table->release();
}
MDL_context *mdl_context= static_cast<MDL_context*>(thd_mdl_context(thd));
for (auto &t : node->tables)
{
dict_table_t *table= t.second.first;
if (table != nullptr && table != reinterpret_cast<dict_table_t*>(-1))
{
dict_table_close(t.second.first, thd, t.second.second);
t.second.first= reinterpret_cast<dict_table_t*>(-1);
if (mdl_context != nullptr && t.second.second != nullptr)
mdl_context->release_lock(t.second.second);
}
}
}
@ -1073,36 +1082,35 @@ void purge_sys_t::wait_FTS(bool also_sys)
}
__attribute__((nonnull))
/** Aqcuire a metadata lock on a table.
/** Acquire a metadata lock on a table.
@param table table handle
@param mdl_context metadata lock acquisition context
@param mdl metadata lcok
@param mdl metadata lock
@return table handle
@retval nullptr if the table is not found or accessible
@retval -1 if the purge of history must be suspended due to DDL */
static dict_table_t *trx_purge_table_acquire(dict_table_t *table,
MDL_context *mdl_context,
MDL_ticket **mdl)
MDL_ticket **mdl) noexcept
{
ut_ad(dict_sys.frozen_not_locked());
*mdl= nullptr;
if (!table->is_readable() || table->corrupted)
{
table->release();
return nullptr;
}
size_t db_len= dict_get_db_name_len(table->name.m_name);
if (db_len == 0)
return table; /* InnoDB system tables are not covered by MDL */
{
/* InnoDB system tables are not covered by MDL */
got_table:
table->acquire();
return table;
}
if (purge_sys.must_wait_FTS())
{
must_wait:
table->release();
return reinterpret_cast<dict_table_t*>(-1);
}
char db_buf[NAME_LEN + 1];
char tbl_buf[NAME_LEN + 1];
@ -1110,7 +1118,7 @@ static dict_table_t *trx_purge_table_acquire(dict_table_t *table,
if (!table->parse_name<true>(db_buf, tbl_buf, &db_len, &tbl_len))
/* The name of an intermediate table starts with #sql */
return table;
goto got_table;
{
MDL_request request;
@ -1123,37 +1131,38 @@ static dict_table_t *trx_purge_table_acquire(dict_table_t *table,
goto must_wait;
}
return table;
goto got_table;
}
/** Open a table handle for the purge of committed transaction history
@param table_id InnoDB table identifier
@param mdl_context metadata lock acquisition context
@param mdl metadata lcok
@param mdl metadata lock
@return table handle
@retval nullptr if the table is not found or accessible
@retval -1 if the purge of history must be suspended due to DDL */
static dict_table_t *trx_purge_table_open(table_id_t table_id,
MDL_context *mdl_context,
MDL_ticket **mdl)
MDL_ticket **mdl) noexcept
{
dict_table_t *table;
for (;;)
{
dict_sys.freeze(SRW_LOCK_CALL);
dict_table_t *table= dict_sys.find_table(table_id);
table= dict_sys.find_table(table_id);
if (table)
table->acquire();
else
{
break;
dict_sys.unfreeze();
dict_sys.lock(SRW_LOCK_CALL);
table= dict_load_table_on_id(table_id, DICT_ERR_IGNORE_FK_NOKEY);
if (table)
table->acquire();
dict_sys.unlock();
if (!table)
return nullptr;
dict_sys.freeze(SRW_LOCK_CALL);
/* At this point, the freshly loaded table may already have been evicted.
We must look it up again while holding a shared dict_sys.latch. We keep
trying this until the table is found in the cache or it cannot be found
in the dictionary (because the table has been dropped or rebuilt). */
}
table= trx_purge_table_acquire(table, mdl_context, mdl);
@ -1172,10 +1181,7 @@ dict_table_t *purge_sys_t::close_and_reopen(table_id_t id, THD *thd,
for (que_thr_t *thr= UT_LIST_GET_FIRST(purge_sys.query->thrs); thr;
thr= UT_LIST_GET_NEXT(thrs, thr))
{
purge_node_t *node= static_cast<purge_node_t*>(thr->child);
trx_purge_close_tables(node, thd);
}
trx_purge_close_tables(static_cast<purge_node_t*>(thr->child), thd);
m_active= false;
wait_FTS(false);