revision-id: e1ffe81bb08b67ad246fd4d3b2a7970e051cec69 (mariadb-10.2.24-8-ge1ffe81bb08) parent(s): 8ce702aa90c174566f4ac950e85cc7570bf9b647 author: Sujatha committer: Sujatha timestamp: 2019-05-13 13:16:36 +0530 message: MDEV-19076: rpl_parallel_temptable result mismatch '-33 optimistic' Problem: ======== The test now fails with the following trace: CURRENT_TEST: rpl.rpl_parallel_temptable --- /mariadb/10.4/mysql-test/suite/rpl/r/rpl_parallel_temptable.result +++ /mariadb/10.4/mysql-test/suite/rpl/r/rpl_parallel_temptable.reject @@ -194,7 +194,6 @@ 30 conservative 31 conservative 32 optimistic -33 optimistic Analysis: ========= The part of test which fails with result content mismatch is given below. CREATE TEMPORARY TABLE t4 (a INT PRIMARY KEY) ENGINE=InnoDB; INSERT INTO t4 VALUES (32); INSERT INTO t4 VALUES (33); INSERT INTO t1 SELECT a, "optimistic" FROM t4; slave_parallel_mode=optimistic The expectation of the above test script is, INSERT FROM SELECT should read both 32, 33 and populate table 't1'. But this expectation fails occasionally. All three INSERT statements are handed over to three different slave parallel workers. Temporary tables are not safe for parallel replication. They were designed to be visible to one thread only, so have no table locking. Thus there is no protection against two conflicting transactions committing in parallel and things like that. So anything that uses temporary tables will be serialized with anything before it, when using parallel replication by using a "wait_for_prior_commit" function call. This will ensure that the each transaction is executed sequentially. But there exists a code path in which the above wait doesn't happen. Because of this at times INSERT from SELECT doesn't wait for the INSERT (33) to complete and it completes its executes and enters commit stage. Hence only row 32 is found in those cases resulting in test failure. The wait needs to be added within "open_temporary_table" call. The code looks like this within "open_temporary_table". Each thread tries to open temporary table in 3 different ways: case 1: Find a temporary table which is already in use by using find_temporary_table(tl) && wait_for_prior_commit() case 2: If above failed then try to look for temporary table which is marked for free for reuse. This internally calls "wait_for_prior_commit()" if table is found. find_and_use_tmp_table(tl, &table) case 3: If none of the above open a new table handle from table share. if (!table && (share= find_tmp_table_share(tl))) { table= open_temporary_table(share, tl->get_table_name(), true); } At present the "wait_for_prior_commit" happens only in case 1 & 2. Fix: ==== On slave add a call for "wait_for_prior_commit" for case 3. The above wait on slave will solve the issue. A more detailed fix would be to mark temporary tables as not safe for parallel execution on the master side. In order to do that, on the master side, mark the Gtid_log_event specific flag FL_TRANSACTIONAL to be false all the time. So that they are not scheduled parallely. --- sql/log_event.cc | 3 ++- sql/sql_lex.h | 32 ++++++++++++++++++++++++++++++++ sql/temporary_tables.cc | 13 +++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index 8aea32004c3..5b916495c20 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -7546,6 +7546,7 @@ Gtid_log_event::Gtid_log_event(THD *thd_arg, uint64 seq_no_arg, flags2((standalone ? FL_STANDALONE : 0) | (commit_id_arg ? FL_GROUP_COMMIT_ID : 0)) { cache_type= Log_event::EVENT_NO_CACHE; + bool is_tmp_table= thd_arg->lex->stmt_accessed_temp_table(); if (thd_arg->transaction.stmt.trans_did_wait() || thd_arg->transaction.all.trans_did_wait()) flags2|= FL_WAITED; @@ -7554,7 +7555,7 @@ Gtid_log_event::Gtid_log_event(THD *thd_arg, uint64 seq_no_arg, thd_arg->transaction.all.trans_did_ddl() || thd_arg->transaction.all.has_created_dropped_temp_table()) flags2|= FL_DDL; - else if (is_transactional) + else if (is_transactional && !is_tmp_table) flags2|= FL_TRANSACTIONAL; if (!(thd_arg->variables.option_bits & OPTION_RPL_SKIP_PARALLEL)) flags2|= FL_ALLOW_PARALLEL; diff --git a/sql/sql_lex.h b/sql/sql_lex.h index a880b6e4283..1889a25b0fb 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -1761,6 +1761,38 @@ class Query_tables_list DBUG_RETURN((stmt_accessed_table_flag & (1U << accessed_table)) != 0); } + /** + Checks either a trans/non trans temporary table is being accessed while + executing a statement. + + @return + @retval TRUE if a temporary table is being accessed + @retval FALSE otherwise + */ + inline bool stmt_accessed_temp_table() + { + DBUG_ENTER("THD::stmt_accessed_temp_table"); + DBUG_RETURN(stmt_accessed_non_trans_temp_table() || + stmt_accessed_trans_temp_table()); + } + + /** + Checks if a temporary transactional table is being accessed while executing + a statement. + + @return + @retval TRUE if a temporary transactional table is being accessed + @retval FALSE otherwise + */ + inline bool stmt_accessed_trans_temp_table() + { + DBUG_ENTER("THD::stmt_accessed_trans_temp_table"); + + DBUG_RETURN((stmt_accessed_table_flag & + ((1U << STMT_READS_TEMP_TRANS_TABLE) | + (1U << STMT_WRITES_TEMP_TRANS_TABLE))) != 0); + } + /** Checks if a temporary non-transactional table is about to be accessed while executing a statement. diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc index ed23dae06d2..da938db00f9 100644 --- a/sql/temporary_tables.cc +++ b/sql/temporary_tables.cc @@ -369,6 +369,19 @@ bool THD::open_temporary_table(TABLE_LIST *tl) if (!table && (share= find_tmp_table_share(tl))) { table= open_temporary_table(share, tl->get_table_name(), true); + /* + Temporary tables are not safe for parallel replication. They were + designed to be visible to one thread only, so have no table locking. + Thus there is no protection against two conflicting transactions + committing in parallel and things like that. + + So for now, anything that uses temporary tables will be serialised + with anything before it, when using parallel replication. + */ + if (table && rgi_slave && + rgi_slave->is_parallel_exec && + wait_for_prior_commit()) + DBUG_RETURN(true); } if (!table)