The problem was that parallel replication of temporary tables using statement-based binlogging could overlap the COMMIT in one thread with a DML or DROP TEMPORARY TABLE in another thread using the same temporary table. Temporary tables are not safe for concurrent access, so this caused reference to freed memory and possibly other nastiness. The fix is to disable the optimisation with overlapping commits of one transaction with the start of a later transaction, when temporary tables are in use. Then the following event groups will be blocked from starting until the one using temporary tables is completed. This also fixes occasional test failures of rpl.rpl_parallel_temptable seen in Buildbot. Signed-off-by: Kristian Nielsen --- sql/rpl_parallel.cc | 18 +++++++++++++++++- sql/sql_class.cc | 4 ++++ sql/sql_class.h | 13 +++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/sql/rpl_parallel.cc b/sql/rpl_parallel.cc index 3bd27c73932..fc4434b75de 100644 --- a/sql/rpl_parallel.cc +++ b/sql/rpl_parallel.cc @@ -218,6 +218,7 @@ finish_event_group(rpl_parallel_thread *rpt, uint64 sub_id, waiting for this). In most cases (normal DML), it will be a no-op. */ rgi->mark_start_commit_no_lock(); + rgi->commit_orderer.wakeup_blocked= false; if (entry->last_committed_sub_id < sub_id) { @@ -1425,7 +1426,22 @@ handle_rpl_parallel_thread(void *arg) if (!thd->killed) { DEBUG_SYNC(thd, "rpl_parallel_before_mark_start_commit"); - rgi->mark_start_commit(); + if (thd->lex->stmt_accessed_temp_table()) + { + /* + Temporary tables are special, they require strict + single-threaded use as they have no locks protecting concurrent + access. Therefore, we cannot safely use the optimization of + overlapping the commit of this transaction with the start of the + following. + So we skip the early mark_start_commit() and also block any + wakeup_subsequent_commits() until this event group is fully + done, inside finish_event_group(). + */ + rgi->commit_orderer.wakeup_blocked= true; + } + else + rgi->mark_start_commit(); DEBUG_SYNC(thd, "rpl_parallel_after_mark_start_commit"); } } diff --git a/sql/sql_class.cc b/sql/sql_class.cc index e7e27401d61..17feb006e21 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -7536,6 +7536,7 @@ wait_for_commit::reinit() wakeup_error= 0; wakeup_subsequent_commits_running= false; commit_started= false; + wakeup_blocked= false; #ifdef SAFE_MUTEX /* When using SAFE_MUTEX, the ordering between taking the LOCK_wait_commit @@ -7808,6 +7809,9 @@ wait_for_commit::wakeup_subsequent_commits2(int wakeup_error) { wait_for_commit *waiter; + if (unlikely(wakeup_blocked)) + return; + mysql_mutex_lock(&LOCK_wait_commit); wakeup_subsequent_commits_running= true; waiter= subsequent_commits_list; diff --git a/sql/sql_class.h b/sql/sql_class.h index 4487a67c76d..4c172ba8e2a 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -2142,6 +2142,19 @@ struct wait_for_commit group commit as T1. */ bool commit_started; + /* + Set to temporarily ignore calls to wakeup_subsequent_commits(). The + caller must arrange that another wakeup_subsequent_commits() gets called + later after wakeup_blocked has been set back to false. + + This is used for parallel replication with temporary tables. + Temporary tables require strict single-threaded operation. The normal + optimization, of doing wakeup_subsequent_commits early and overlapping + part of the commit with the following transaction, is not safe. Thus + when temporary tables are replicated, wakeup is blocked until the + event group is fully done. + */ + bool wakeup_blocked; void register_wait_for_prior_commit(wait_for_commit *waitee); int wait_for_prior_commit(THD *thd, bool allow_kill=true) -- 2.30.2