From 381e9adb6cf838a30da6cf3bcd359ef9ebc3d575 Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 17 May 2024 13:42:55 +0300 Subject: [PATCH] MDEV-34150 Assertion failure in Diagnostics_area::set_error_status upon binary logging hitting tmp space limit - Moved writing to binlog_cache from close_thread_tables() to binlog_commit(). - In select_create() delete cached row events instead of flushing them to disk. This was done to avoid possible disk write error in this code. --- mysql-test/include/show_binlog_events.inc | 2 +- .../suite/binlog/r/binlog_unsafe.result | 6 +- sql/log.cc | 60 ++++++++++++++++--- sql/online_alter.cc | 2 + sql/sql_base.cc | 19 +++--- sql/sql_class.cc | 27 ++++----- sql/sql_class.h | 3 +- sql/sql_insert.cc | 7 ++- 8 files changed, 81 insertions(+), 45 deletions(-) diff --git a/mysql-test/include/show_binlog_events.inc b/mysql-test/include/show_binlog_events.inc index 57fe1ffe0e3..e33a9852254 100644 --- a/mysql-test/include/show_binlog_events.inc +++ b/mysql-test/include/show_binlog_events.inc @@ -20,7 +20,7 @@ # # $binlog_start # Position for the 'FROM' clause of SHOW BINLOG EVENTS. If none -# given, starts right after the Binlog_checkpoint_log_even. +# given, starts right after the Binlog_checkpoint_log_event. # # $binlog_limit # Limit for the 'LIMIT' clause of SHOW BINLOG EVENTS, i.e.: diff --git a/mysql-test/suite/binlog/r/binlog_unsafe.result b/mysql-test/suite/binlog/r/binlog_unsafe.result index 67849fc7fd1..e3786d3368f 100644 --- a/mysql-test/suite/binlog/r/binlog_unsafe.result +++ b/mysql-test/suite/binlog/r/binlog_unsafe.result @@ -2542,11 +2542,7 @@ master-bin.000001 # Annotate_rows # # insert into t2(a) values(new.a) master-bin.000001 # Table_map # # table_id: # (test.t1) master-bin.000001 # Table_map # # table_id: # (test.t3) master-bin.000001 # Table_map # # table_id: # (test.t2) -master-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F -master-bin.000001 # Annotate_rows # # insert into t3(a) values(new.a) -master-bin.000001 # Table_map # # table_id: # (test.t1) -master-bin.000001 # Table_map # # table_id: # (test.t3) -master-bin.000001 # Table_map # # table_id: # (test.t2) +master-bin.000001 # Write_rows_v1 # # table_id: # master-bin.000001 # Write_rows_v1 # # table_id: # master-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F master-bin.000001 # Query # # COMMIT diff --git a/sql/log.cc b/sql/log.cc index 2b601f9e5a4..2e87cadca2e 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -437,6 +437,28 @@ private: binlog_cache_mngr(const binlog_cache_mngr& info); }; + +/* + Remove all row event from all binlog caches and clear all caches. + This is only called from CREATE .. SELECT, in which case it safe to delete + also events from the statement cache. +*/ + +void THD::binlog_remove_rows_events() +{ + binlog_cache_mngr *cache_mngr= binlog_get_cache_mngr(); + DBUG_ENTER("THD::binlog_remove_rows_events"); + + if (!cache_mngr || + (!WSREP_EMULATE_BINLOG_NNULL(this) && !mysql_bin_log.is_open())) + DBUG_VOID_RETURN; + + MYSQL_BIN_LOG::remove_pending_rows_event(this, &cache_mngr->stmt_cache); + MYSQL_BIN_LOG::remove_pending_rows_event(this, &cache_mngr->trx_cache); + cache_mngr->reset(1,1); + DBUG_VOID_RETURN; +} + /** The function handles the first phase of two-phase binlogged ALTER. On master binlogs START ALTER when that is configured to do so. @@ -1897,20 +1919,19 @@ binlog_commit_flush_xid_caches(THD *thd, binlog_cache_mngr *cache_mngr, static int binlog_truncate_trx_cache(THD *thd, binlog_cache_mngr *cache_mngr, bool all) { + int error=0; DBUG_ENTER("binlog_truncate_trx_cache"); if(!WSREP_EMULATE_BINLOG_NNULL(thd) && !mysql_bin_log.is_open()) DBUG_RETURN(0); - int error=0; - DBUG_PRINT("info", ("thd->options={ %s %s}, transaction: %s", FLAGSTR(thd->variables.option_bits, OPTION_NOT_AUTOCOMMIT), FLAGSTR(thd->variables.option_bits, OPTION_BEGIN), all ? "all" : "stmt")); - auto &trx_cache= cache_mngr->trx_cache; - MYSQL_BIN_LOG::remove_pending_rows_event(thd, &trx_cache); + binlog_cache_data *trx_cache= &cache_mngr->trx_cache; + MYSQL_BIN_LOG::remove_pending_rows_event(thd, trx_cache); thd->reset_binlog_for_next_statement(); /* @@ -1919,7 +1940,7 @@ binlog_truncate_trx_cache(THD *thd, binlog_cache_mngr *cache_mngr, bool all) */ if (ending_trans(thd, all)) { - if (trx_cache.has_incident()) + if (trx_cache->has_incident()) error= mysql_bin_log.write_incident(thd); DBUG_ASSERT(thd->binlog_table_maps == 0); @@ -1931,9 +1952,9 @@ binlog_truncate_trx_cache(THD *thd, binlog_cache_mngr *cache_mngr, bool all) transaction cache to remove the statement. */ else - trx_cache.restore_prev_position(); + trx_cache->restore_prev_position(); - DBUG_ASSERT(trx_cache.pending() == NULL); + DBUG_ASSERT(trx_cache->pending() == NULL); DBUG_RETURN(error); } @@ -2190,6 +2211,11 @@ int binlog_commit(THD *thd, bool all, bool ro_1pc) cache_mngr->need_unlog= false; } } + else if (thd->lock) + { + /* If not in LOCK TABLES, flush the transaction log */ + error= thd->binlog_flush_pending_rows_event(TRUE, TRUE); + } /* This is part of the stmt rollback. */ @@ -6583,6 +6609,26 @@ int binlog_flush_pending_rows_event(THD *thd, bool stmt_end, } +/* + Check if there are pending row events in the binlog cache + + @return 0 no + @return 1 rows in stmt_cache + @return 2 rows in trx_cache + @return 3 rows in both +*/ + +uint THD::has_pending_row_events() +{ + binlog_cache_mngr *cache_mngr; + if (!mysql_bin_log.is_open() || + !(cache_mngr= binlog_get_cache_mngr())) + return 0; + return ((cache_mngr->stmt_cache.pending() ? 1 : 0) | + (cache_mngr->trx_cache.pending() ? 2 : 0)); +} + + /** This function removes the pending rows event, discarding any outstanding rows. If there is no pending rows event available, this is effectively a diff --git a/sql/online_alter.cc b/sql/online_alter.cc index 2c5eb4dea62..defe3334d53 100644 --- a/sql/online_alter.cc +++ b/sql/online_alter.cc @@ -222,6 +222,8 @@ int online_alter_end_trans(Online_alter_cache_list &cache_list, THD *thd, { // Do not set STMT_END for last event to leave table open in altering thd error= binlog_flush_pending_rows_event(thd, false, true, binlog, &cache); + if (error) + do_commit= commit= 0; } if (do_commit) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index f5a348a739e..046be59822f 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -933,7 +933,7 @@ int close_thread_tables(THD *thd) Note that even if we are in LTM_LOCK_TABLES mode and statement requires prelocking (e.g. when we are closing tables after - failing ot "open" all tables required for statement execution) + failing at "open" all tables required for statement execution) we will exit this function a few lines below. */ if (! thd->lex->requires_prelocking()) @@ -964,16 +964,13 @@ int close_thread_tables(THD *thd) if (thd->lock) { /* - For RBR we flush the pending event just before we unlock all the - tables. This means that we are at the end of a topmost - statement, so we ensure that the STMT_END_F flag is set on the - pending event. For statements that are *inside* stored - functions, the pending event will not be flushed: that will be - handled either before writing a query log event (inside - binlog_query()) or when preparing a pending event. - */ - (void)thd->binlog_flush_pending_rows_event(TRUE); - error= mysql_unlock_tables(thd, thd->lock); + Ensure binlog caches has been flushed in binlog_query() or + binlog_commit(). + */ + DBUG_ASSERT((thd->state_flags & Open_tables_state::BACKUPS_AVAIL) || + !thd->has_pending_row_events()); + if (mysql_unlock_tables(thd, thd->lock)) + error= 1; thd->lock=0; } /* diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 4975fbbe12e..76701972e7e 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -7553,11 +7553,12 @@ int THD::binlog_flush_pending_rows_event(bool stmt_end, bool is_transactional) if (variables.option_bits & OPTION_GTID_BEGIN) is_transactional= 1; - auto *cache_mngr= binlog_get_cache_mngr(); + binlog_cache_mngr *cache_mngr= binlog_get_cache_mngr(); if (!cache_mngr) DBUG_RETURN(0); - auto *cache= binlog_get_cache_data(cache_mngr, - use_trans_cache(this, is_transactional)); + binlog_cache_data *cache= + binlog_get_cache_data(cache_mngr, + use_trans_cache(this, is_transactional)); int error= ::binlog_flush_pending_rows_event(this, stmt_end, is_transactional, @@ -7852,22 +7853,14 @@ int THD::binlog_query(THD::enum_binlog_query_type qtype, char const *query_arg, } /* - If we are not in prelocked mode, mysql_unlock_tables() will be - called after this binlog_query(), so we have to flush the pending - rows event with the STMT_END_F set to unlock all tables at the - slave side as well. - - If we are in prelocked mode, the flushing will be done inside the - top-most close_thread_tables(). + We should not flush row events for sub-statements, like a trigger or + function. The main statement will take care of the flushing when + calling binlog_query(). */ - if (this->locked_tables_mode <= LTM_LOCK_TABLES) + if (!in_sub_stmt) { - int error; - if (unlikely(error= binlog_flush_pending_rows_event(TRUE, is_trans))) - { - DBUG_ASSERT(error > 0); - DBUG_RETURN(error); - } + if (unlikely(binlog_flush_pending_rows_event(TRUE, is_trans))) + DBUG_RETURN(my_errno); // Return error code as required } /* diff --git a/sql/sql_class.h b/sql/sql_class.h index bdcfaf4b99b..68c94acb497 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3279,7 +3279,8 @@ public: binlog_flush_pending_rows_event(stmt_end, TRUE)); } int binlog_flush_pending_rows_event(bool stmt_end, bool is_transactional); - + void binlog_remove_rows_events(); + uint has_pending_row_events(); bool binlog_need_stmt_format(bool is_transactional) const { if (!log_current_statement()) diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 24641a2065b..695d1f98f34 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -5342,14 +5342,15 @@ void select_create::abort_result_set() thd->transaction->stmt.modified_non_trans_table= FALSE; thd->variables.option_bits= save_option_bits; - /* possible error of writing binary log is ignored deliberately */ - (void) thd->binlog_flush_pending_rows_event(TRUE, TRUE); - if (table) { bool tmp_table= table->s->tmp_table; bool table_creation_was_logged= (!tmp_table || table->s->table_creation_was_logged); + + /* CREATE SELECT failed. Remove all row events and clear caches */ + thd->binlog_remove_rows_events(); + if (tmp_table) { DBUG_ASSERT(saved_tmp_table_share);