From c436b6a2bcdcee7b3ffa4e8e1d23ac2539d42de8 Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Tue, 5 Mar 2024 02:38:04 +0100 Subject: [PATCH] MDEV-33450 Assertion fails in main.alter_table_online_debug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assertion "from->s->online_alter_binlog == NULL" fails in copy_data_between_tables, signalizing that a table share is being reused (in another alter) after a lock upgrade to EXCLUSIVE fails. Commit 3059f27 relaxed the lock to be upgraded to MDL_SHARED_NO_WRITE, leaving it to happen later by a common path wait_while_table_is_used() call. However the error handling there is not enough for online alter case, where we require (for now) the table to be flushed, in order to clean up the memory properly. * Add another lock upgrade (to MDL_EXCLUSIVE) after the second replication stage in copy_data_between_tables. The error from this upgrade will be handled by the branch presented further in the function. MDEV-33450 Assertion fails in main.alter_table_online_debug `TABLE_SHARE` that is being online-altered has a shared `s->online_alter_binlog` member that all concurrent DMLs are writing to. Online alter thread deletes it under the MDL_EXCLUSIVE. If upgrading the lock to MDL_EXCLUSIVE fails, table as marked as `flushed` and it's freed automatically when its usage drops to zero. In commit 3059f27 the lock upgrade was relaxed to MDL_SHARED_NO_WRITE to allow concurrent SELECT threads during the final `online_alter_read_from_binlog()` pass. An attempt to upgrade the lock to MDL_EXCLUSIVE was still happening, but much later — after the code that marked the table `flushed`. That is, if the upgrade failed, the table was left with a stale `s->online_alter_binlog` triggering an assert in a future online alter. To fix this, upgrade the lock to MDL_EXCLUSIVE earlier, after the final `online_alter_read_from_binlog()`. --- sql/sql_table.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 4c4162cf9fa..f6ece121dfa 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -12423,6 +12423,17 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to, thd_progress_next_stage(thd); error= online_alter_read_from_binlog(thd, &rgi, binlog, &found_count); } + + /* + We'll do it earlier than usually in mysql_alter_table to handle the + online-related errors more comfortably. + */ + lock_error= thd->mdl_context.upgrade_shared_lock(from->mdl_ticket, + MDL_EXCLUSIVE, + (double)thd->variables.lock_wait_timeout); + if (!error) + error= lock_error; + to->pos_in_table_list= NULL; // Safety DBUG_ASSERT(thd->lex->sql_command == saved_sql_command); thd->lex->sql_command= saved_sql_command; // Just in case