From 6066e5d13c2154e4348f40d59eb3af32eb5734c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 26 Mar 2025 14:23:45 +0200 Subject: [PATCH] MDEV-36122: Work around missing MDL in purge prepare_inplace_alter_table_dict(): If an unexpected reference to the table exists, wait for the purge subsystem to release its table handle, similar to how we would do in case FULLTEXT INDEX existed. This function is supposed to be protected by MDL_EXCLUSIVE on the table name. If purge is going to access the table again later during is ALTER TABLE operation, it will have access to an MDL compatible name for it and therefore should conflict with any MDL_EXCLUSIVE that would cover ha_innobase::commit_inplace_alter_table(commit=true). This change should prevent race conditions where purge had initially looked up a table for which row-level undo log records had been written by ALTER IGNORE TABLE, and purge did not finish before a subsequent ALTER TABLE is trying to rebuild the table. trx_purge_attach_undo_recs(), purge_sys_t::batch_cleanup(): Clear purge_sys.m_active only after all table handles have been released. ha_innobase::truncate(): Before locking the data dictionary, ensure that the purge subsystem is not holding a reference to the table due to insufficient metadata locking related to an earlier ALTER IGNORE TABLE operation. Reviewed by: Debarun Banerjee --- storage/innobase/handler/ha_innodb.cc | 3 +++ storage/innobase/handler/handler0alter.cc | 18 ++++++++++++------ storage/innobase/trx/trx0purge.cc | 4 ++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 4d2b179738f..fdfd0cb395d 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -14023,6 +14023,7 @@ int ha_innobase::truncate() 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; if (fts) { @@ -14030,6 +14031,8 @@ int ha_innobase::truncate() purge_sys.stop_FTS(*ib_table); error= fts_lock_tables(trx, *ib_table); } + else if (pause_purge) + purge_sys.stop_FTS(); /* Wait for purge threads to stop using the table. */ for (uint n = 15; ib_table->get_ref_count() > 1; ) diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 09c1febf61d..3fe02af5beb 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -6603,8 +6603,9 @@ prepare_inplace_alter_table_dict( mem_heap_alloc(ctx->heap, ctx->num_to_add_index * sizeof *ctx->add_key_numbers)); - const bool fts_exist = ctx->new_table->flags2 + const bool have_fts = user_table->flags2 & (DICT_TF2_FTS_HAS_DOC_ID | DICT_TF2_FTS); + 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; @@ -6631,13 +6632,18 @@ acquire_lock: user_table->lock_shared_unlock(); } - if (fts_exist) { - purge_sys.stop_FTS(*ctx->new_table); + if (pause_purge) { + purge_sys.stop_FTS(); + if (have_fts) { + purge_sys.stop_FTS(*user_table, true); + } if (error == DB_SUCCESS) { - error = fts_lock_tables(ctx->trx, *ctx->new_table); + error = fts_lock_tables(ctx->trx, *user_table); } } + ut_ad(user_table->get_ref_count() == 1); + if (error == DB_SUCCESS) { error = lock_sys_tables(ctx->trx); } @@ -7470,7 +7476,7 @@ error_handling_drop_uncached: /* fts_create_common_tables() may drop old common tables, whose files would be deleted here. */ commit_unlock_and_unlink(ctx->trx); - if (fts_exist) { + if (pause_purge) { purge_sys.resume_FTS(); } @@ -7566,7 +7572,7 @@ err_exit: ctx->trx->free(); } trx_commit_for_mysql(ctx->prebuilt->trx); - if (fts_exist) { + if (pause_purge) { purge_sys.resume_FTS(); } diff --git a/storage/innobase/trx/trx0purge.cc b/storage/innobase/trx/trx0purge.cc index ef85dd7dbda..8292bb063b4 100644 --- a/storage/innobase/trx/trx0purge.cc +++ b/storage/innobase/trx/trx0purge.cc @@ -1292,8 +1292,6 @@ static purge_sys_t::iterator trx_purge_attach_undo_recs(THD *thd, break; } - purge_sys.m_active= false; - #ifdef UNIV_DEBUG thr= UT_LIST_GET_FIRST(purge_sys.query->thrs); for (ulint i= 0; thr && i < *n_work_items; @@ -1342,6 +1340,8 @@ static void trx_purge_wait_for_workers_to_complete() TRANSACTIONAL_INLINE void purge_sys_t::batch_cleanup(const purge_sys_t::iterator &head) { + m_active= false; + /* Release the undo pages. */ for (auto p : pages) p.second->unfix();