From 561b6c7e513abc4ceba263252b519bf715ce80f4 Mon Sep 17 00:00:00 2001 From: Andrei Elkin Date: Mon, 25 Oct 2021 18:25:03 +0300 Subject: MDEV-26833 Missed statement rollback in case transaction drops or create temporary table When transaction creates or drops temporary tables and afterward its statement faces an error even the transactional table statement's cached ROW format events get involved into binlog and are visible after the transaction's commit. Fixed with proper analysis of whether the errored-out statement needs to be rolled back in binlog. For instance a fact of already cached CREATE or DROP for temporary tables by previous statements alone does not cause to retain the being errored-out statement events in the cache. Conversely, if the statement creates or drops a temporary table itself it can't be rolled back - this rule remains. --- sql/handler.h | 11 +++++++++++ sql/log.cc | 40 ++++++++++++---------------------------- sql/sp_head.cc | 8 +++++++- sql/sql_class.h | 3 ++- 4 files changed, 32 insertions(+), 30 deletions(-) (limited to 'sql') diff --git a/sql/handler.h b/sql/handler.h index 542b91c570d..02a4a76c6c1 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -1477,6 +1477,9 @@ struct THD_TRANS /* Define the type of statemens which cannot be rolled back safely. Each type occupies one bit in m_unsafe_rollback_flags. + MODIFIED_NON_TRANS_TABLE is limited to mark only the temporary + non-transactional table *when* it's cached along with the transactional + events; the regular table is covered by the "namesake" bool var. */ static unsigned int const MODIFIED_NON_TRANS_TABLE= 0x01; static unsigned int const CREATED_TEMP_TABLE= 0x02; @@ -1485,6 +1488,14 @@ struct THD_TRANS static unsigned int const DID_DDL= 0x10; static unsigned int const EXECUTED_TABLE_ADMIN_CMD= 0x20; + void mark_modified_non_trans_temp_table() + { + m_unsafe_rollback_flags|= MODIFIED_NON_TRANS_TABLE; + } + bool has_modified_non_trans_temp_table() const + { + return (m_unsafe_rollback_flags & MODIFIED_NON_TRANS_TABLE) != 0; + } void mark_executed_table_admin_cmd() { DBUG_PRINT("debug", ("mark_executed_table_admin_cmd")); diff --git a/sql/log.cc b/sql/log.cc index 1d9b4645421..895eeec454b 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -252,8 +252,9 @@ void make_default_log_name(char **out, const char* log_ext, bool once) class binlog_cache_data { public: - binlog_cache_data(): m_pending(0), before_stmt_pos(MY_OFF_T_UNDEF), - incident(FALSE), changes_to_non_trans_temp_table_flag(FALSE), + binlog_cache_data(): m_pending(0), + before_stmt_pos(MY_OFF_T_UNDEF), + incident(FALSE), saved_max_binlog_cache_size(0), ptr_binlog_cache_use(0), ptr_binlog_cache_disk_use(0) { } @@ -289,16 +290,6 @@ public: return(incident); } - void set_changes_to_non_trans_temp_table() - { - changes_to_non_trans_temp_table_flag= TRUE; - } - - bool changes_to_non_trans_temp_table() - { - return (changes_to_non_trans_temp_table_flag); - } - void reset() { compute_statistics(); @@ -306,7 +297,6 @@ public: if(cache_log.file != -1) my_chsize(cache_log.file, 0, 0, MYF(MY_WME)); - changes_to_non_trans_temp_table_flag= FALSE; incident= FALSE; before_stmt_pos= MY_OFF_T_UNDEF; /* @@ -401,12 +391,6 @@ private: */ bool incident; - /* - This flag indicates if the cache has changes to temporary tables. - @TODO This a temporary fix and should be removed after BUG#54562. - */ - bool changes_to_non_trans_temp_table_flag; - /** This function computes binlog cache and disk usage. */ @@ -1942,13 +1926,12 @@ static int binlog_prepare(handlerton *hton, THD *thd, bool all) */ static bool trans_cannot_safely_rollback(THD *thd, bool all) { - binlog_cache_mngr *const cache_mngr= - (binlog_cache_mngr*) thd_get_ha_data(thd, binlog_hton); + DBUG_ASSERT(ending_trans(thd, all)); return ((thd->variables.option_bits & OPTION_KEEP_LOG) || (trans_has_updated_non_trans_table(thd) && thd->wsrep_binlog_format() == BINLOG_FORMAT_STMT) || - (cache_mngr->trx_cache.changes_to_non_trans_temp_table() && + (thd->transaction.all.has_modified_non_trans_temp_table() && thd->wsrep_binlog_format() == BINLOG_FORMAT_MIXED) || (trans_has_updated_non_trans_table(thd) && ending_single_stmt_trans(thd,all) && @@ -2092,17 +2075,19 @@ static int binlog_rollback(handlerton *hton, THD *thd, bool all) /* Truncate the cache if: . aborting a single or multi-statement transaction or; - . the OPTION_KEEP_LOG is not active and; + . the current statement created or dropped a temporary table + while having actual STATEMENT format; . the format is not STMT or no non-trans table was updated and; . the format is not MIXED or no temporary non-trans table was updated. */ else if (ending_trans(thd, all) || - (!(thd->variables.option_bits & OPTION_KEEP_LOG) && + (!(thd->transaction.stmt.has_created_dropped_temp_table() && + !thd->is_current_stmt_binlog_format_row()) && (!stmt_has_updated_non_trans_table(thd) || thd->wsrep_binlog_format() != BINLOG_FORMAT_STMT) && - (!cache_mngr->trx_cache.changes_to_non_trans_temp_table() || + (!thd->transaction.stmt.has_modified_non_trans_temp_table() || thd->wsrep_binlog_format() != BINLOG_FORMAT_MIXED))) error= binlog_truncate_trx_cache(thd, cache_mngr, all); } @@ -6276,9 +6261,8 @@ bool MYSQL_BIN_LOG::write(Log_event *event_info, my_bool *with_annotate) file= cache_mngr->get_binlog_cache_log(is_trans_cache); cache_data= cache_mngr->get_binlog_cache_data(is_trans_cache); - if (thd->lex->stmt_accessed_non_trans_temp_table()) - cache_data->set_changes_to_non_trans_temp_table(); - + if (thd->lex->stmt_accessed_non_trans_temp_table() && is_trans_cache) + thd->transaction.stmt.mark_modified_non_trans_temp_table(); thd->binlog_start_trans_and_stmt(); } DBUG_PRINT("info",("event type: %d",event_info->get_type_code())); diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 6a6bdf8fe09..cb5348cd110 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -3040,8 +3040,13 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp, It's reset further in the common code part. It's merged with the saved parent's value at the exit of this func. */ - bool parent_modified_non_trans_table= thd->transaction.stmt.modified_non_trans_table; + bool parent_modified_non_trans_table= + thd->transaction.stmt.modified_non_trans_table; + unsigned int parent_unsafe_rollback_flags= + thd->transaction.stmt.m_unsafe_rollback_flags; thd->transaction.stmt.modified_non_trans_table= FALSE; + thd->transaction.stmt.m_unsafe_rollback_flags= 0; + DBUG_ASSERT(!thd->derived_tables); DBUG_ASSERT(thd->Item_change_list::is_empty()); /* @@ -3162,6 +3167,7 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp, what is needed from the substatement gained */ thd->transaction.stmt.modified_non_trans_table |= parent_modified_non_trans_table; + thd->transaction.stmt.m_unsafe_rollback_flags |= parent_unsafe_rollback_flags; TRANSACT_TRACKER(add_trx_state_from_thd(thd)); diff --git a/sql/sql_class.h b/sql/sql_class.h index 5f871f9caf6..a767a34d869 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -4524,7 +4524,8 @@ public: transaction.all.modified_non_trans_table= TRUE; transaction.all.m_unsafe_rollback_flags|= (transaction.stmt.m_unsafe_rollback_flags & - (THD_TRANS::DID_WAIT | THD_TRANS::CREATED_TEMP_TABLE | + (THD_TRANS::MODIFIED_NON_TRANS_TABLE | + THD_TRANS::DID_WAIT | THD_TRANS::CREATED_TEMP_TABLE | THD_TRANS::DROPPED_TEMP_TABLE | THD_TRANS::DID_DDL | THD_TRANS::EXECUTED_TABLE_ADMIN_CMD)); } -- cgit v1.2.1