[Commits] 08c07fe: MDEV-8075: DROP TEMPORARY TABLE not marked as ddl, causing optimistic parallel replication to fail

Kristian Nielsen knielsen at knielsen-hq.org
Wed May 6 13:14:44 EEST 2015


revision-id: 08c07feb0410ed1e49f46172fb503e1be42fc37b
parent(s): 53382ac128d9571b5f779bf625567fc3ea0b475e
committer: Kristian Nielsen
branch nick: mariadb
timestamp: 2015-05-06 12:14:33 +0200
message:

MDEV-8075: DROP TEMPORARY TABLE not marked as ddl, causing optimistic parallel replication to fail

CREATE/DROP TEMPORARY TABLE are not safe to optimistically replicate in
parallel with other transactions, so they need to be marked as "ddl" in the
binlog.

This was already done for stand-alone CREATE/DROP TEMPORARY. But temporary
tables can also be created and dropped inside a BEGIN...END transaction, and
such transactions were not marked as ddl. Nor was the DROP TEMPORARY TABLE
statement emitted implicitly when a client connection is closed.

So this patch adds such ddl mark for the missing cases.

---
 .../suite/rpl/r/rpl_parallel_optimistic.result     | 27 ++++++++++++++++++++++
 .../suite/rpl/t/rpl_parallel_optimistic.test       | 25 ++++++++++++++++++++
 sql/handler.h                                      |  9 ++++++++
 sql/log_event.cc                                   |  6 +++--
 sql/sql_base.cc                                    |  1 +
 sql/sql_class.h                                    | 10 ++++++++
 sql/sql_insert.cc                                  |  8 +++++++
 sql/sql_parse.cc                                   |  3 +--
 sql/sql_table.cc                                   | 22 +++++++++++++-----
 sql/transaction.cc                                 | 25 +++++++++-----------
 10 files changed, 112 insertions(+), 24 deletions(-)

diff --git a/mysql-test/suite/rpl/r/rpl_parallel_optimistic.result b/mysql-test/suite/rpl/r/rpl_parallel_optimistic.result
index 7593b49..2da7830 100644
--- a/mysql-test/suite/rpl/r/rpl_parallel_optimistic.result
+++ b/mysql-test/suite/rpl/r/rpl_parallel_optimistic.result
@@ -455,6 +455,33 @@ a	b
 include/stop_slave.inc
 SET GLOBAL debug_dbug= @old_debug;
 include/start_slave.inc
+*** MDEV-8075: DROP TEMPORARY TABLE not marked as ddl, causing optimistic parallel replication to fail ***
+include/stop_slave.inc
+INSERT INTO t1 VALUES (40, 10);
+CREATE TEMPORARY TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (41);
+BEGIN;
+INSERT INTO t2 SELECT a, 20 FROM t1;
+DROP TEMPORARY TABLE t1;
+COMMIT;
+INSERT INTO t1 VALUES (42, 10);
+include/save_master_gtid.inc
+SELECT * FROM t1 WHERE a >= 40 ORDER BY a;
+a	b
+40	10
+42	10
+SELECT * FROM t2 WHERE a >= 40 ORDER BY a;
+a	b
+41	20
+include/start_slave.inc
+include/sync_with_master_gtid.inc
+SELECT * FROM t1 WHERE a >= 40 ORDER BY a;
+a	b
+40	10
+42	10
+SELECT * FROM t2 WHERE a >= 40 ORDER BY a;
+a	b
+41	20
 include/stop_slave.inc
 SET GLOBAL slave_parallel_mode=@old_parallel_mode;
 SET GLOBAL slave_parallel_threads=@old_parallel_threads;
diff --git a/mysql-test/suite/rpl/t/rpl_parallel_optimistic.test b/mysql-test/suite/rpl/t/rpl_parallel_optimistic.test
index e7d4f18..376c8d6 100644
--- a/mysql-test/suite/rpl/t/rpl_parallel_optimistic.test
+++ b/mysql-test/suite/rpl/t/rpl_parallel_optimistic.test
@@ -429,6 +429,31 @@ SET GLOBAL debug_dbug= @old_debug;
 --source include/start_slave.inc
 
 
+--echo *** MDEV-8075: DROP TEMPORARY TABLE not marked as ddl, causing optimistic parallel replication to fail ***
+
+--connection server_2
+--source include/stop_slave.inc
+
+--connection server_1
+INSERT INTO t1 VALUES (40, 10);
+CREATE TEMPORARY TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (41);
+BEGIN;
+INSERT INTO t2 SELECT a, 20 FROM t1;
+DROP TEMPORARY TABLE t1;
+COMMIT;
+INSERT INTO t1 VALUES (42, 10);
+--source include/save_master_gtid.inc
+SELECT * FROM t1 WHERE a >= 40 ORDER BY a;
+SELECT * FROM t2 WHERE a >= 40 ORDER BY a;
+
+--connection server_2
+--source include/start_slave.inc
+--source include/sync_with_master_gtid.inc
+SELECT * FROM t1 WHERE a >= 40 ORDER BY a;
+SELECT * FROM t2 WHERE a >= 40 ORDER BY a;
+
+
 # Clean up.
 
 --connection server_2
diff --git a/sql/handler.h b/sql/handler.h
index 279bfaf..dd94ed9 100644
--- a/sql/handler.h
+++ b/sql/handler.h
@@ -1450,6 +1450,15 @@ struct THD_TRANS
     DBUG_PRINT("debug", ("mark_created_temp_table"));
     m_unsafe_rollback_flags|= CREATED_TEMP_TABLE;
   }
+  void mark_dropped_temp_table()
+  {
+    DBUG_PRINT("debug", ("mark_dropped_temp_table"));
+    m_unsafe_rollback_flags|= DROPPED_TEMP_TABLE;
+  }
+  bool has_created_dropped_temp_table() const {
+    return
+      (m_unsafe_rollback_flags & (CREATED_TEMP_TABLE|DROPPED_TEMP_TABLE)) != 0;
+  }
   void mark_trans_did_wait() { m_unsafe_rollback_flags|= DID_WAIT; }
   bool trans_did_wait() const {
     return (m_unsafe_rollback_flags & DID_WAIT) != 0;
diff --git a/sql/log_event.cc b/sql/log_event.cc
index aa34996..c047366 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -6405,8 +6405,10 @@ Gtid_log_event::Gtid_log_event(THD *thd_arg, uint64 seq_no_arg,
   if (thd_arg->transaction.stmt.trans_did_wait() ||
       thd_arg->transaction.all.trans_did_wait())
     flags2|= FL_WAITED;
-  if (sql_command_flags[thd->lex->sql_command] &
-      (CF_DISALLOW_IN_RO_TRANS | CF_AUTO_COMMIT_TRANS))
+  if ((sql_command_flags[thd->lex->sql_command] &
+       (CF_DISALLOW_IN_RO_TRANS | CF_AUTO_COMMIT_TRANS)) ||
+      (thd_arg->transaction.stmt.has_created_dropped_temp_table() ||
+       thd_arg->transaction.all.has_created_dropped_temp_table()))
     flags2|= FL_DDL;
   else if (is_transactional)
     flags2|= FL_TRANSACTIONAL;
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index c400598..6ef46ca 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -1253,6 +1253,7 @@ bool close_temporary_tables(THD *thd)
       thd->variables.character_set_client= cs_save;
 
       thd->get_stmt_da()->set_overwrite_status(true);
+      thd->transaction.stmt.mark_dropped_temp_table();
       if ((error= (mysql_bin_log.write(&qinfo) || error)))
       {
         /*
diff --git a/sql/sql_class.h b/sql/sql_class.h
index a39c8cd..b761ba6 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -3953,6 +3953,16 @@ class THD :public Statement,
   {
     main_lex.restore_set_statement_var();
   }
+  /* Copy relevant `stmt` transaction flags to `all` transaction. */
+  void merge_unsafe_rollback_flags()
+  {
+    if (transaction.stmt.modified_non_trans_table)
+      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::DROPPED_TEMP_TABLE));
+  }
 };
 
 
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index 24d75d9..90e68d5 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -4226,6 +4226,14 @@ void select_create::store_values(List<Item> &values)
 
 bool select_create::send_eof()
 {
+  /*
+    The routine that writes the statement in the binary log
+    is in select_insert::send_eof(). For that reason, we
+    mark the flag at this point.
+  */
+  if (create_info->options & HA_LEX_CREATE_TMP_TABLE)
+    thd->transaction.stmt.mark_created_temp_table();
+
   if (select_insert::send_eof())
   {
     abort_result_set();
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 5eca972..08d74b4 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -6781,8 +6781,7 @@ void THD::reset_for_next_command()
   if (!thd->in_multi_stmt_transaction_mode())
   {
     thd->variables.option_bits&= ~OPTION_KEEP_LOG;
-    thd->transaction.all.modified_non_trans_table= FALSE;
-    thd->transaction.all.m_unsafe_rollback_flags&= ~THD_TRANS::DID_WAIT;
+    thd->transaction.all.reset();
   }
   DBUG_ASSERT(thd->security_ctx== &thd->main_security_ctx);
   thd->thread_specific_used= FALSE;
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 1f96793..9abe012 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -2559,6 +2559,9 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
   if (non_trans_tmp_table_deleted ||
       trans_tmp_table_deleted || non_tmp_table_deleted)
   {
+    if (non_trans_tmp_table_deleted || trans_tmp_table_deleted)
+      thd->transaction.stmt.mark_dropped_temp_table();
+
     query_cache_invalidate3(thd, tables, 0);
     if (!dont_log_query && mysql_bin_log.is_open())
     {
@@ -5014,6 +5017,9 @@ bool mysql_create_table(THD *thd, TABLE_LIST *create_table,
   if (thd->is_current_stmt_binlog_format_row() && create_info->tmp_table())
     DBUG_RETURN(result);
 
+  if (create_info->options & HA_LEX_CREATE_TMP_TABLE)
+    thd->transaction.stmt.mark_created_temp_table();
+
   /* Write log if no error or if we already deleted a table */
   if (!result || thd->log_current_statement)
   {
@@ -5491,13 +5497,17 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table,
     DBUG_PRINT("info",
                ("res: %d  tmp_table: %d  create_info->table: %p",
                 res, create_info->tmp_table(), local_create_info.table));
-    if (!res && create_info->tmp_table() && local_create_info.table)
+    if (create_info->tmp_table())
     {
-      /*
-        Remember that tmp table creation was logged so that we know if
-        we should log a delete of it.
-      */
-      local_create_info.table->s->table_creation_was_logged= 1;
+      thd->transaction.stmt.mark_created_temp_table();
+      if (!res && local_create_info.table)
+      {
+        /*
+          Remember that tmp table creation was logged so that we know if
+          we should log a delete of it.
+        */
+        local_create_info.table->s->table_creation_was_logged= 1;
+      }
     }
     do_logging= TRUE;
   }
diff --git a/sql/transaction.cc b/sql/transaction.cc
index f84c4e5..fa6d338 100644
--- a/sql/transaction.cc
+++ b/sql/transaction.cc
@@ -154,8 +154,7 @@ bool trans_begin(THD *thd, uint flags)
     The following set should not be needed as the flag should always be 0
     when we come here.  We should at some point change this to an assert.
   */
-  thd->transaction.all.modified_non_trans_table= FALSE;
-  thd->transaction.all.m_unsafe_rollback_flags&= ~THD_TRANS::DID_WAIT;
+  thd->transaction.all.reset();
   thd->has_waiter= false;
   thd->waiting_on_group_commit= false;
 
@@ -251,8 +250,7 @@ bool trans_commit(THD *thd)
   else
     (void) RUN_HOOK(transaction, after_commit, (thd, FALSE));
   thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
-  thd->transaction.all.modified_non_trans_table= FALSE;
-  thd->transaction.all.m_unsafe_rollback_flags&= ~THD_TRANS::DID_WAIT;
+  thd->transaction.all.reset();
   thd->lex->start_transaction_opt= 0;
 
   DBUG_RETURN(MY_TEST(res));
@@ -299,8 +297,7 @@ bool trans_commit_implicit(THD *thd)
   }
 
   thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
-  thd->transaction.all.modified_non_trans_table= FALSE;
-  thd->transaction.all.m_unsafe_rollback_flags&= ~THD_TRANS::DID_WAIT;
+  thd->transaction.all.reset();
 
   /*
     Upon implicit commit, reset the current transaction
@@ -345,8 +342,7 @@ bool trans_rollback(THD *thd)
   thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
   /* Reset the binlog transaction marker */
   thd->variables.option_bits&= ~OPTION_GTID_BEGIN;
-  thd->transaction.all.modified_non_trans_table= FALSE;
-  thd->transaction.all.m_unsafe_rollback_flags&= ~THD_TRANS::DID_WAIT;
+  thd->transaction.all.reset();
   thd->lex->start_transaction_opt= 0;
 
   DBUG_RETURN(MY_TEST(res));
@@ -390,8 +386,7 @@ bool trans_rollback_implicit(THD *thd)
     preserve backward compatibility.
   */
   thd->variables.option_bits&= ~(OPTION_KEEP_LOG);
-  thd->transaction.all.modified_non_trans_table= false;
-  thd->transaction.all.m_unsafe_rollback_flags&= ~THD_TRANS::DID_WAIT;
+  thd->transaction.all.reset();
 
   /* Rollback should clear transaction_rollback_request flag. */
   DBUG_ASSERT(! thd->transaction_rollback_request);
@@ -427,6 +422,8 @@ bool trans_commit_stmt(THD *thd)
   */
   DBUG_ASSERT(! thd->in_sub_stmt);
 
+  thd->merge_unsafe_rollback_flags();
+
   if (thd->transaction.stmt.ha_list)
   {
     if (WSREP_ON)
@@ -481,6 +478,8 @@ bool trans_rollback_stmt(THD *thd)
   */
   DBUG_ASSERT(! thd->in_sub_stmt);
 
+  thd->merge_unsafe_rollback_flags();
+
   if (thd->transaction.stmt.ha_list)
   {
     if (WSREP_ON)
@@ -913,8 +912,7 @@ bool trans_xa_commit(THD *thd)
   }
 
   thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
-  thd->transaction.all.modified_non_trans_table= FALSE;
-  thd->transaction.all.m_unsafe_rollback_flags&= ~THD_TRANS::DID_WAIT;
+  thd->transaction.all.reset();
   thd->server_status&=
     ~(SERVER_STATUS_IN_TRANS | SERVER_STATUS_IN_TRANS_READONLY);
   DBUG_PRINT("info", ("clearing SERVER_STATUS_IN_TRANS"));
@@ -969,8 +967,7 @@ bool trans_xa_rollback(THD *thd)
   res= xa_trans_force_rollback(thd);
 
   thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
-  thd->transaction.all.modified_non_trans_table= FALSE;
-  thd->transaction.all.m_unsafe_rollback_flags&= ~THD_TRANS::DID_WAIT;
+  thd->transaction.all.reset();
   thd->server_status&=
     ~(SERVER_STATUS_IN_TRANS | SERVER_STATUS_IN_TRANS_READONLY);
   DBUG_PRINT("info", ("clearing SERVER_STATUS_IN_TRANS"));


More information about the commits mailing list