List:Commits« Previous MessageNext Message »
From:Mattias Jonsson Date:August 20 2010 10:07am
Subject:bzr commit into mysql-5.5-bugfixing branch (mattias.jonsson:3184) Bug#54747
View as plain text  
#At file:///Users/mattiasj/mysql-bzr/b54747-5.5-bf/ based on revid:joerg@stripped

 3184 Mattias Jonsson	2010-08-20
      Bug#54747: Deadlock between REORGANIZE PARTITION and SELECT is not detected
      
      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.
     @ mysql-test/r/partition_debug_sync.result
        updated result
     @ mysql-test/t/partition_debug_sync.test
        added test
     @ sql/sql_base.cc
        added debug sync for easier testing
     @ 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_prepare/commit_alter_copy_data

    modified:
      mysql-test/r/partition_debug_sync.result
      mysql-test/t/partition_debug_sync.test
      sql/sql_base.cc
      sql/sql_partition.cc
      sql/sql_table.cc
      sql/sql_table.h
=== modified file 'mysql-test/r/partition_debug_sync.result'
--- a/mysql-test/r/partition_debug_sync.result	2010-07-01 13:53:46 +0000
+++ b/mysql-test/r/partition_debug_sync.result	2010-08-20 10:07:33 +0000
@@ -1,6 +1,38 @@
 DROP TABLE IF EXISTS t1, t2;
 SET DEBUG_SYNC= 'RESET';
 #
+# Bug#54747: Deadlock between REORGANIZE PARTITION and
+#            SELECT is not detected
+#
+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));
+LOAD DATA LOCAL INFILE 'MYSQLTEST_VARDIR/t_part_range.load' 
+INTO TABLE t1 (user_num, hours);
+# Start a ALTER PARTITION and wait between the copy and rename of table.
+# con1
+SET DEBUG_SYNC= 'wait_while_table_is_used SIGNAL run_select WAIT_FOR select_started';
+ALTER TABLE t1
+REORGANIZE PARTITION hour_003, hour_004 INTO
+(PARTITION oldest VALUES LESS THAN (4));
+# Start a concurrent SELECT
+SET DEBUG_SYNC= 'now WAIT_FOR run_select';
+SET DEBUG_SYNC= 'after_lock_tables_takes_lock SIGNAL select_started';
+SELECT COUNT(*) FROM t1;
+COUNT(*)
+10000
+SET DEBUG_SYNC= 'RESET';
+SET GLOBAL innodb_thread_concurrency = 0;
+DROP TABLE t1;
+#
 # Bug#42438: Crash ha_partition::change_table_ptr
 # Test when remove partitioning is done while drop table is waiting
 # for the table.

=== modified file 'mysql-test/t/partition_debug_sync.test'
--- a/mysql-test/t/partition_debug_sync.test	2010-07-01 13:53:46 +0000
+++ b/mysql-test/t/partition_debug_sync.test	2010-08-20 10:07:33 +0000
@@ -12,6 +12,70 @@ SET DEBUG_SYNC= 'RESET';
 --enable_warnings
 
 --echo #
+--echo # Bug#54747: Deadlock between REORGANIZE PARTITION and
+--echo #            SELECT is not detected
+--echo #
+
+let $thread_concurrency = `SELECT @@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));
+
+--perl
+open( DATA, ">$ENV{MYSQLTEST_VARDIR}/t_part_range.load" ) 
+	|| die "Could not open file $ENV{MYSQLTEST_VARDIR}/t_part_range.load for writing: $!";
+for ( 1..10000 )
+{
+	print DATA ( int(rand(9000000000))+1000000000 ), "\t", ( int(rand(5)) ), "\n";
+}
+close( DATA );
+EOF
+
+--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
+eval LOAD DATA LOCAL INFILE '$MYSQLTEST_VARDIR/t_part_range.load' 
+	INTO TABLE t1 (user_num, hours);
+
+--remove_file $MYSQLTEST_VARDIR/t_part_range.load
+
+--echo # Start a ALTER PARTITION and wait between the copy and rename of table.
+--echo # con1
+
+--connect (con1,localhost,root,,)
+SET DEBUG_SYNC= 'wait_while_table_is_used SIGNAL run_select WAIT_FOR select_started';
+--send
+ALTER TABLE t1
+REORGANIZE PARTITION hour_003, hour_004 INTO
+(PARTITION oldest VALUES LESS THAN (4));
+
+--echo # Start a concurrent SELECT
+
+--connection default
+#START TRANSACTION;
+SET DEBUG_SYNC= 'now WAIT_FOR run_select';
+SET DEBUG_SYNC= 'after_lock_tables_takes_lock SIGNAL select_started';
+SELECT COUNT(*) FROM t1;
+
+--connection con1
+--reap
+
+--disconnect con1
+
+--connection default
+
+SET DEBUG_SYNC= 'RESET';
+eval SET GLOBAL innodb_thread_concurrency = $thread_concurrency;
+DROP TABLE t1;
+
+--echo #
 --echo # Bug#42438: Crash ha_partition::change_table_ptr
 --echo # Test when remove partitioning is done while drop table is waiting
 --echo # for the table.

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2010-08-13 07:50:25 +0000
+++ b/sql/sql_base.cc	2010-08-20 10:07:33 +0000
@@ -2197,6 +2197,8 @@ bool wait_while_table_is_used(THD *thd, 
                        table->s->table_name.str, (ulong) table->s,
                        table->db_stat, table->s->version));
 
+  DEBUG_SYNC(thd, "wait_while_table_is_used");
+
   if (thd->mdl_context.upgrade_shared_lock_to_exclusive(
              table->mdl_ticket, thd->variables.lock_wait_timeout))
     DBUG_RETURN(TRUE);

=== modified file 'sql/sql_partition.cc'
--- a/sql/sql_partition.cc	2010-08-13 07:50:25 +0000
+++ b/sql/sql_partition.cc	2010-08-20 10:07:33 +0000
@@ -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, 
                               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, 
   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,16 +5528,24 @@ static bool mysql_change_partitions(ALTE
   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_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);
   }
+
+  if (mysql_commit_alter_copy_data(thd))
+    DBUG_RETURN(TRUE);                      /* The error has been reported */
+
   DBUG_RETURN(FALSE);
 }
 

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2010-08-16 12:58:38 +0000
+++ b/sql/sql_table.cc	2010-08-20 10:07:33 +0000
@@ -6702,6 +6702,54 @@ err_with_mdl:
 }
 /* mysql_alter_table */
 
+
+
+/**
+  Prepare the transaction for the alter table's copy phase.
+*/
+
+bool mysql_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_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 tables 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,
@@ -6728,14 +6776,7 @@ copy_data_between_tables(TABLE *from,TAB
   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_prepare_alter_copy_data(thd))
     DBUG_RETURN(-1);
   
   if (!(copy= new Copy_field[to->s->fields]))
@@ -6894,20 +6935,8 @@ copy_data_between_tables(TABLE *from,TAB
   }
   to->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
 
-  if (ha_enable_transaction(thd, TRUE))
-  {
+  if (mysql_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;

=== modified file 'sql/sql_table.h'
--- a/sql/sql_table.h	2010-08-16 12:53:30 +0000
+++ b/sql/sql_table.h	2010-08-20 10:07:33 +0000
@@ -142,6 +142,8 @@ bool mysql_create_table_no_lock(THD *thd
 bool mysql_prepare_alter_table(THD *thd, TABLE *table,
                                HA_CREATE_INFO *create_info,
                                Alter_info *alter_info);
+bool mysql_prepare_alter_copy_data(THD *thd);
+bool mysql_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,


Attachment: [text/bzr-bundle] bzr/mattias.jonsson@oracle.com-20100820100733-uwp14yiuccu4n2nt.bundle
Thread
bzr commit into mysql-5.5-bugfixing branch (mattias.jonsson:3184) Bug#54747Mattias Jonsson20 Aug