Bug#54747: Deadlock between REORGANIZE PARTITION and SELECT is not detected

The ALTER PARTITION and SELECT seemed to be deadlocked
when having innodb_thread_concurrency = 1.

Problem was that there was unreleased latches
in the ALTER PARTITION thread which was needed
by the SELECT thread to be able to continue.

Solution was to release the latches by commit 
before requesting upgrade to exclusive MDL lock.

Updated according to reviewers comments (3).

mysql-test/r/partition_innodb.result:
  updated test result
mysql-test/t/partition_innodb.test:
  added test
sql/sql_partition.cc:
  Moved implicit commit into mysql_change_partition
  so that if latches are taken, they are always released
  before waiting on exclusive lock.
sql/sql_table.cc:
  refactored the code to prepare and commit
  around copy_data_between_tables, to be able
  to reuse it in mysql_change_partitions
sql/sql_table.h:
  exporting mysql_trans_prepare/commit_alter_copy_data

5 files changed, 156 insertions, 31 deletions

diff --git a/mysql-test/r/partition_innodb.result b/mysql-test/r/partition_innodb.result
index 107ad719e2b..93cb1617d2f 100644
--- a/mysql-test/r/partition_innodb.result
+++ b/mysql-test/r/partition_innodb.result
@@ -1,5 +1,41 @@
 drop table if exists t1, t2;
 #
+# Bug#54747: Deadlock between REORGANIZE PARTITION and
+#            SELECT is not detected
+#
+SET @old_innodb_thread_concurrency:= @@innodb_thread_concurrency;
+SET GLOBAL innodb_thread_concurrency = 1;
+CREATE TABLE t1
+(user_num BIGINT,
+hours SMALLINT,
+KEY user_num (user_num))
+ENGINE = InnoDB   
+PARTITION BY RANGE COLUMNS (hours)
+(PARTITION hour_003 VALUES LESS THAN (3),
+PARTITION hour_004 VALUES LESS THAN (4),
+PARTITION hour_005 VALUES LESS THAN (5),
+PARTITION hour_last VALUES LESS THAN (MAXVALUE));
+INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5);
+BEGIN;
+SELECT COUNT(*) FROM t1;
+COUNT(*)
+5
+# con1
+# SEND a ALTER PARTITION which waits on the ongoing transaction.
+ALTER TABLE t1
+REORGANIZE PARTITION hour_003, hour_004 INTO
+(PARTITION oldest VALUES LESS THAN (4));
+# Connection default wait until the ALTER is in 'waiting for table...'
+# state and then continue the transaction by trying a SELECT
+SELECT COUNT(*) FROM t1;
+COUNT(*)
+5
+COMMIT;
+# con1, reaping ALTER.
+# Disconnecting con1 and switching to default. Cleaning up.
+SET GLOBAL innodb_thread_concurrency = @old_innodb_thread_concurrency;
+DROP TABLE t1;
+#
 # Bug#50418: DROP PARTITION does not interact with transactions
 #
 CREATE TABLE t1 (
diff --git a/mysql-test/t/partition_innodb.test b/mysql-test/t/partition_innodb.test
index f6adf014468..3e9ac2ce2b5 100644
--- a/mysql-test/t/partition_innodb.test
+++ b/mysql-test/t/partition_innodb.test
@@ -9,6 +9,63 @@ drop table if exists t1, t2;
 let $MYSQLD_DATADIR= `SELECT @@datadir`;
 
 --echo #
+--echo # Bug#54747: Deadlock between REORGANIZE PARTITION and
+--echo #            SELECT is not detected
+--echo #
+
+SET @old_innodb_thread_concurrency:= @@innodb_thread_concurrency;
+SET GLOBAL innodb_thread_concurrency = 1;
+
+CREATE TABLE t1
+(user_num BIGINT,
+ hours SMALLINT,
+ KEY user_num (user_num))
+ENGINE = InnoDB   
+PARTITION BY RANGE COLUMNS (hours)
+(PARTITION hour_003 VALUES LESS THAN (3),
+ PARTITION hour_004 VALUES LESS THAN (4),
+ PARTITION hour_005 VALUES LESS THAN (5),
+ PARTITION hour_last VALUES LESS THAN (MAXVALUE));
+
+INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5);
+
+BEGIN;
+SELECT COUNT(*) FROM t1;
+
+--echo # con1
+--connect (con1,localhost,root,,)
+--echo # SEND a ALTER PARTITION which waits on the ongoing transaction.
+--send
+ALTER TABLE t1
+REORGANIZE PARTITION hour_003, hour_004 INTO
+(PARTITION oldest VALUES LESS THAN (4));
+
+--echo # Connection default wait until the ALTER is in 'waiting for table...'
+--echo # state and then continue the transaction by trying a SELECT
+--connection default
+let $wait_condition =
+SELECT COUNT(*) = 1
+FROM information_schema.processlist
+WHERE INFO like 'ALTER TABLE t1%REORGANIZE PARTITION hour_003, hour_004%'
+AND STATE = 'Waiting for table metadata lock';
+--source include/wait_condition.inc
+SELECT COUNT(*) FROM t1;
+COMMIT;
+
+--echo # con1, reaping ALTER.
+--connection con1
+--reap
+
+--echo # Disconnecting con1 and switching to default. Cleaning up.
+--disconnect con1
+
+--connection default
+
+SET GLOBAL innodb_thread_concurrency = @old_innodb_thread_concurrency;
+DROP TABLE t1;
+
+
+--echo #
 --echo # Bug#50418: DROP PARTITION does not interact with transactions
 --echo #
 CREATE TABLE t1 (
diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc
index 5bbe946179e..b72816f8ce3 100644
--- a/sql/sql_partition.cc
+++ b/sql/sql_partition.cc
@@ -63,6 +63,7 @@
 #include "sql_table.h"                  // build_table_filename,
                                         // build_table_shadow_filename,
                                         // table_to_filename
+                                        // mysql_*_alter_copy_data
 #include "opt_range.h"                  // store_key_image_to_rec
 #include "sql_analyse.h"                // append_escaped
 
@@ -4377,7 +4378,6 @@ static int fast_end_partition(THD *thd, ulonglong copied,
                               ALTER_PARTITION_PARAM_TYPE *lpt,
                               bool written_bin_log)
 {
-  int error;
   char tmp_name[80];
   DBUG_ENTER("fast_end_partition");
 
@@ -4386,13 +4386,6 @@ static int fast_end_partition(THD *thd, ulonglong copied,
   if (!is_empty)
     query_cache_invalidate3(thd, table_list, 0);
 
-  error= trans_commit_stmt(thd);
-  if (trans_commit_implicit(thd))
-    error= 1;
-
-  if (error)
-    DBUG_RETURN(TRUE);                      /* The error has been reported */
-
   if ((!is_empty) && (!written_bin_log) &&
       (!thd->lex->no_write_to_binlog) &&
     write_bin_log(thd, FALSE, thd->query(), thd->query_length()))
@@ -5535,17 +5528,25 @@ static bool mysql_change_partitions(ALTER_PARTITION_PARAM_TYPE *lpt)
   char path[FN_REFLEN+1];
   int error;
   handler *file= lpt->table->file;
+  THD *thd= lpt->thd;
   DBUG_ENTER("mysql_change_partitions");
 
   build_table_filename(path, sizeof(path) - 1, lpt->db, lpt->table_name, "", 0);
+
+  if(mysql_trans_prepare_alter_copy_data(thd))
+    DBUG_RETURN(TRUE);
+
   if ((error= file->ha_change_partitions(lpt->create_info, path, &lpt->copied,
                                          &lpt->deleted, lpt->pack_frm_data,
                                          lpt->pack_frm_len)))
   {
     file->print_error(error, MYF(error != ER_OUTOFMEMORY ? 0 : ME_FATALERROR));
-    DBUG_RETURN(TRUE);
   }
-  DBUG_RETURN(FALSE);
+
+  if (mysql_trans_commit_alter_copy_data(thd))
+    DBUG_RETURN(TRUE);                      /* The error has been reported */
+
+  DBUG_RETURN(test(error));
 }
 
 
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index bd5f381b8b8..3bb7f7667ea 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -6740,6 +6740,54 @@ err_with_mdl:
 }
 /* mysql_alter_table */
 
+
+
+/**
+  Prepare the transaction for the alter table's copy phase.
+*/
+
+bool mysql_trans_prepare_alter_copy_data(THD *thd)
+{
+  DBUG_ENTER("mysql_prepare_alter_copy_data");
+  /*
+    Turn off recovery logging since rollback of an alter table is to
+    delete the new table so there is no need to log the changes to it.
+    
+    This needs to be done before external_lock.
+  */
+  if (ha_enable_transaction(thd, FALSE))
+    DBUG_RETURN(TRUE);
+  DBUG_RETURN(FALSE);
+}
+
+
+/**
+  Commit the copy phase of the alter table.
+*/
+
+bool mysql_trans_commit_alter_copy_data(THD *thd)
+{
+  bool error= FALSE;
+  DBUG_ENTER("mysql_commit_alter_copy_data");
+
+  if (ha_enable_transaction(thd, TRUE))
+    DBUG_RETURN(TRUE);
+  
+  /*
+    Ensure that the new table is saved properly to disk before installing
+    the new .frm.
+    And that InnoDB's internal latches are released, to avoid deadlock
+    when waiting on other instances of the table before rename (Bug#54747).
+  */
+  if (trans_commit_stmt(thd))
+    error= TRUE;
+  if (trans_commit_implicit(thd))
+    error= TRUE;
+
+  DBUG_RETURN(error);
+}
+
+
 static int
 copy_data_between_tables(TABLE *from,TABLE *to,
 			 List<Create_field> &create,
@@ -6766,14 +6814,7 @@ copy_data_between_tables(TABLE *from,TABLE *to,
   ulonglong prev_insert_id;
   DBUG_ENTER("copy_data_between_tables");
 
-  /*
-    Turn off recovery logging since rollback of an alter table is to
-    delete the new table so there is no need to log the changes to it.
-    
-    This needs to be done before external_lock
-  */
-  error= ha_enable_transaction(thd, FALSE);
-  if (error)
+  if (mysql_trans_prepare_alter_copy_data(thd))
     DBUG_RETURN(-1);
   
   if (!(copy= new Copy_field[to->s->fields]))
@@ -6932,20 +6973,8 @@ copy_data_between_tables(TABLE *from,TABLE *to,
   }
   to->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
 
-  if (ha_enable_transaction(thd, TRUE))
-  {
+  if (mysql_trans_commit_alter_copy_data(thd))
     error= 1;
-    goto err;
-  }
-  
-  /*
-    Ensure that the new table is saved properly to disk so that we
-    can do a rename
-  */
-  if (trans_commit_stmt(thd))
-    error=1;
-  if (trans_commit_implicit(thd))
-    error=1;
 
  err:
   thd->variables.sql_mode= save_sql_mode;
diff --git a/sql/sql_table.h b/sql/sql_table.h
index ae5beefea37..eb0b1aa94dd 100644
--- a/sql/sql_table.h
+++ b/sql/sql_table.h
@@ -143,6 +143,8 @@ bool mysql_create_table_no_lock(THD *thd, const char *db,
 bool mysql_prepare_alter_table(THD *thd, TABLE *table,
                                HA_CREATE_INFO *create_info,
                                Alter_info *alter_info);
+bool mysql_trans_prepare_alter_copy_data(THD *thd);
+bool mysql_trans_commit_alter_copy_data(THD *thd);
 bool mysql_alter_table(THD *thd, char *new_db, char *new_name,
                        HA_CREATE_INFO *create_info,
                        TABLE_LIST *table_list,