List:Commits« Previous MessageNext Message »
From:Mattias Jonsson Date:December 11 2009 11:37am
Subject:bzr commit into mysql-5.1-bugteam branch (mattias.jonsson:3238)
Bug#42438
View as plain text  
#At file:///Users/mattiasj/clones/bzrroot/b42438-51-bt/ based on revid:bar@stripped

 3238 Mattias Jonsson	2009-12-11
      Bug#42438: Crash ha_partition::change_table_ptr
      
      There was two problems:
      The first was the symptom, caused by bad error handling in
      ha_partition. It did not handle print_error etc. when
      having no partitions (when used by dummy handler).
      
      The second was the real problem that when dropping tables
      it reused the table type (storage engine) from when the lock
      was asked for, not the table type that it had when gaining
      the exclusive name lock. So that it tried to delete tables
      from wrong storage engines.
      
      Solutions for the first problem was to accept some handler
      calls to the partitioning handler even if it was not setup
      with any partitions, and also if possible fallback
      to use the base handler's default functions.
      
      Solution for the second problem was to re-read the current
      table type from the frm file, when under LOCK_open, just
      before deleting the tables from the storage engine.
     @ mysql-test/r/partition_debug_sync.result
        Bug#42438: Crash ha_partition::change_table_ptr
        
        New result file using DEBUG_SYNC for deterministic results.
     @ mysql-test/t/partition_debug_sync.test
        Bug#42438: Crash ha_partition::change_table_ptr
        
        New test file using DEBUG_SYNC for deterministic results.
     @ sql/ha_partition.cc
        Bug#42438: Crash ha_partition::change_table_ptr
        
        allow some handler calls, used by error handling, even when
        no partitions are setup. Fallback to default handling if possible.
     @ sql/sql_base.cc
        Bug#42438: Crash ha_partition::change_table_ptr
        
        Added DEBUG_SYNC point for deterministic test cases.
     @ sql/sql_table.cc
        Bug#42438: Crash ha_partition::change_table_ptr
        
        Always use the table type written in the .frm-file
        (i.e. the current table type) when deleting a table.
        
        Added DEBUG_SYNC points for deterministic test cases.

    added:
      mysql-test/r/partition_debug_sync.result
      mysql-test/t/partition_debug_sync.test
    modified:
      sql/ha_partition.cc
      sql/sql_base.cc
      sql/sql_table.cc
=== added file 'mysql-test/r/partition_debug_sync.result'
--- a/mysql-test/r/partition_debug_sync.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/partition_debug_sync.result	2009-12-11 11:37:17 +0000
@@ -0,0 +1,57 @@
+DROP TABLE IF EXISTS t1, t2;
+SET DEBUG_SYNC= 'RESET';
+#
+# Bug#42438: Crash ha_partition::change_table_ptr
+# Test when remove partitioning is done while drop table is waiting
+# for the table.
+# Con 1
+SET DEBUG_SYNC= 'RESET';
+CREATE TABLE t1
+(a INTEGER,
+b INTEGER NOT NULL,
+KEY (b))
+ENGINE = MYISAM
+/*!50100  PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (2),
+PARTITION p1 VALUES LESS THAN (20),
+PARTITION p2 VALUES LESS THAN (100),
+PARTITION p3 VALUES LESS THAN MAXVALUE ) */;
+SET DEBUG_SYNC= 'alter_table_before_create_table_no_lock SIGNAL removing_partitioning WAIT_FOR waiting_for_alter';
+SET DEBUG_SYNC= 'alter_table_before_main_binlog SIGNAL partitioning_removed';
+ALTER TABLE t1 REMOVE PARTITIONING;
+# Con default
+SET DEBUG_SYNC= 'now WAIT_FOR removing_partitioning';
+SET DEBUG_SYNC= 'waiting_for_table SIGNAL waiting_for_alter';
+SET DEBUG_SYNC= 'rm_table_part2_before_delete_table WAIT_FOR partitioning_removed';
+DROP TABLE IF EXISTS t1;
+# Con 1
+SET DEBUG_SYNC= 'RESET';
+SET DEBUG_SYNC= 'RESET';
+#
+# Bug#42438: Crash ha_partition::change_table_ptr
+# Test when remove partitioning is failing due to drop table is already
+# in progress.
+CREATE TABLE t2
+(a INTEGER,
+b INTEGER NOT NULL,
+KEY (b))
+ENGINE = MYISAM
+/*!50100  PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (2),
+PARTITION p1 VALUES LESS THAN (20),
+PARTITION p2 VALUES LESS THAN (100),
+PARTITION p3 VALUES LESS THAN MAXVALUE ) */;
+SET DEBUG_SYNC= 'before_lock_tables_takes_lock SIGNAL removing_partitions WAIT_FOR waiting_for_alter';
+SET DEBUG_SYNC= 'alter_table_before_rename_result_table WAIT_FOR delete_done';
+ALTER TABLE t2 REMOVE PARTITIONING;
+# Con default
+SET DEBUG_SYNC= 'now WAIT_FOR removing_partitions';
+SET DEBUG_SYNC= 'waiting_for_table SIGNAL waiting_for_alter';
+SET DEBUG_SYNC= 'rm_table_part2_before_binlog SIGNAL delete_done';
+DROP TABLE IF EXISTS t2;
+# Con 1
+ERROR 42S02: Table 'test.t2' doesn't exist
+SET DEBUG_SYNC= 'RESET';
+# Con default
+SET DEBUG_SYNC= 'RESET';
+End of 5.1 tests

=== added file 'mysql-test/t/partition_debug_sync.test'
--- a/mysql-test/t/partition_debug_sync.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/partition_debug_sync.test	2009-12-11 11:37:17 +0000
@@ -0,0 +1,81 @@
+#--disable_abort_on_error
+#
+# Test for the partition storage engine which require DEBUG_SYNC feature to
+# Created by Mattias Jonsson
+#
+--source include/have_partition.inc
+--source include/have_debug_sync.inc
+
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2;
+SET DEBUG_SYNC= 'RESET';
+--enable_warnings
+
+--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.
+connect(con1, localhost, root,,);
+--echo # Con 1
+SET DEBUG_SYNC= 'RESET';
+CREATE TABLE t1
+(a INTEGER,
+ b INTEGER NOT NULL,
+ KEY (b))
+ENGINE = MYISAM
+/*!50100  PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (2),
+ PARTITION p1 VALUES LESS THAN (20),
+ PARTITION p2 VALUES LESS THAN (100),
+ PARTITION p3 VALUES LESS THAN MAXVALUE ) */;
+SET DEBUG_SYNC= 'alter_table_before_create_table_no_lock SIGNAL removing_partitioning WAIT_FOR waiting_for_alter';
+SET DEBUG_SYNC= 'alter_table_before_main_binlog SIGNAL partitioning_removed';
+--send ALTER TABLE t1 REMOVE PARTITIONING
+connection default;
+--echo # Con default
+SET DEBUG_SYNC= 'now WAIT_FOR removing_partitioning';
+SET DEBUG_SYNC= 'waiting_for_table SIGNAL waiting_for_alter';
+SET DEBUG_SYNC= 'rm_table_part2_before_delete_table WAIT_FOR partitioning_removed';
+DROP TABLE IF EXISTS t1;
+--echo # Con 1
+connection con1;
+--reap
+connection default;
+SET DEBUG_SYNC= 'RESET';
+connection con1;
+SET DEBUG_SYNC= 'RESET';
+
+--echo #
+--echo # Bug#42438: Crash ha_partition::change_table_ptr
+--echo # Test when remove partitioning is failing due to drop table is already
+--echo # in progress.
+CREATE TABLE t2
+(a INTEGER,
+ b INTEGER NOT NULL,
+ KEY (b))
+ENGINE = MYISAM
+/*!50100  PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (2),
+ PARTITION p1 VALUES LESS THAN (20),
+ PARTITION p2 VALUES LESS THAN (100),
+ PARTITION p3 VALUES LESS THAN MAXVALUE ) */;
+SET DEBUG_SYNC= 'before_lock_tables_takes_lock SIGNAL removing_partitions WAIT_FOR waiting_for_alter';
+SET DEBUG_SYNC= 'alter_table_before_rename_result_table WAIT_FOR delete_done';
+--send ALTER TABLE t2 REMOVE PARTITIONING
+connection default;
+--echo # Con default
+SET DEBUG_SYNC= 'now WAIT_FOR removing_partitions';
+SET DEBUG_SYNC= 'waiting_for_table SIGNAL waiting_for_alter';
+SET DEBUG_SYNC= 'rm_table_part2_before_binlog SIGNAL delete_done';
+DROP TABLE IF EXISTS t2;
+--echo # Con 1
+connection con1;
+--error ER_NO_SUCH_TABLE
+--reap
+SET DEBUG_SYNC= 'RESET';
+disconnect con1;
+connection default;
+--echo # Con default
+SET DEBUG_SYNC= 'RESET';
+
+--echo End of 5.1 tests

=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2009-10-09 07:56:07 +0000
+++ b/sql/ha_partition.cc	2009-12-11 11:37:17 +0000
@@ -1719,13 +1719,19 @@ void ha_partition::update_create_info(HA
 
 void ha_partition::change_table_ptr(TABLE *table_arg, TABLE_SHARE *share)
 {
-  handler **file_array= m_file;
+  handler **file_array;
   table= table_arg;
   table_share= share;
-  do
+  if (m_file)
   {
-    (*file_array)->change_table_ptr(table_arg, share);
-  } while (*(++file_array));
+    file_array= m_file;
+    DBUG_ASSERT(*file_array);
+    do
+    {
+      (*file_array)->change_table_ptr(table_arg, share);
+    } while (*(++file_array));
+  }
+
   if (m_added_file && m_added_file[0])
   {
     /* if in middle of a drop/rename etc */
@@ -5986,7 +5992,12 @@ void ha_partition::print_error(int error
   if (error == HA_ERR_NO_PARTITION_FOUND)
     m_part_info->print_no_partition_found(table);
   else
-    m_file[m_last_part]->print_error(error, errflag);
+  {
+    if (m_file)
+      m_file[m_last_part]->print_error(error, errflag);
+    else
+      handler::print_error(error, errflag);
+  }
   DBUG_VOID_RETURN;
 }
 
@@ -5996,7 +6007,10 @@ bool ha_partition::get_error_message(int
   DBUG_ENTER("ha_partition::get_error_message");
 
   /* Should probably look for my own errors first */
-  DBUG_RETURN(m_file[m_last_part]->get_error_message(error, buf));
+  if (m_file)
+    DBUG_RETURN(m_file[m_last_part]->get_error_message(error, buf));
+  DBUG_RETURN(handler::get_error_message(error, buf));
+
 }
 
 

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2009-11-20 20:56:43 +0000
+++ b/sql/sql_base.cc	2009-12-11 11:37:17 +0000
@@ -2165,6 +2165,7 @@ void wait_for_condition(THD *thd, pthrea
   proc_info=thd->proc_info;
   thd_proc_info(thd, "Waiting for table");
   DBUG_ENTER("wait_for_condition");
+  DEBUG_SYNC(thd, "waiting_for_table");
   if (!thd->killed)
     (void) pthread_cond_wait(cond, mutex);
 

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2009-11-27 13:34:39 +0000
+++ b/sql/sql_table.cc	2009-12-11 11:37:17 +0000
@@ -22,6 +22,9 @@
 #include "sp_head.h"
 #include "sql_trigger.h"
 #include "sql_show.h"
+#if defined(ENABLED_DEBUG_SYNC)
+#include "debug_sync.h"
+#endif
 
 #ifdef __WIN__
 #include <io.h>
@@ -1904,7 +1907,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
   {
     char *db=table->db;
     handlerton *table_type;
-    enum legacy_db_type frm_db_type;
+    enum legacy_db_type frm_db_type= DB_TYPE_UNKNOWN;
 
     DBUG_PRINT("table", ("table_l: '%s'.'%s'  table: 0x%lx  s: 0x%lx",
                          table->db, table->table_name, (long) table->table,
@@ -1996,6 +1999,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
                                         table->internal_tmp_table ?
                                         FN_IS_TMP : 0);
     }
+    DEBUG_SYNC(thd, "rm_table_part2_before_delete_table");
     if (drop_temporary ||
         ((table_type == NULL &&        
          access(path, F_OK) &&
@@ -2014,13 +2018,25 @@ int mysql_rm_table_part2(THD *thd, TABLE
     else
     {
       char *end;
-      if (table_type == NULL)
+      /*
+        Cannot use the db_type from the table, since that might have changed
+        while waiting for the exclusive name lock. We are under LOCK_open,
+        so reading from the frm-file is safe.
+      */
+      if (frm_db_type == DB_TYPE_UNKNOWN)
+      {
+        mysql_frm_type(thd, path, &frm_db_type);
+        DBUG_PRINT("info", ("frm_db_type %d from %s", frm_db_type, path));
+      }
+      if (table_type == NULL || table_type->db_type != frm_db_type)
       {
-	mysql_frm_type(thd, path, &frm_db_type);
+        DBUG_PRINT("info", ("table_type has changed during wait_for_condition,"
+                            " using type %d from .frm-file", frm_db_type));
         table_type= ha_resolve_by_legacy_type(thd, frm_db_type);
       }
       // Remove extension for delete
       *(end= path + path_length - reg_ext_length)= '\0';
+      DBUG_PRINT("info", ("deleting table of type %d", table_type->db_type));
       error= ha_delete_table(thd, table_type, path, db, table->table_name,
                              !dont_log_query);
       if ((error == ENOENT || error == HA_ERR_NO_SUCH_TABLE) && 
@@ -2062,6 +2078,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
     on the table name.
   */
   pthread_mutex_unlock(&LOCK_open);
+  DEBUG_SYNC(thd, "rm_table_part2_before_binlog");
   thd->thread_specific_used|= tmp_table_deleted;
   error= 0;
   if (wrong_tables.length())
@@ -7097,6 +7114,7 @@ view_err:
   else
     create_info->data_file_name=create_info->index_file_name=0;
 
+  DEBUG_SYNC(thd, "alter_table_before_create_table_no_lock");
   /*
     Create a table with a temporary name.
     With create_info->frm_only == 1 this creates a .frm file only.
@@ -7296,6 +7314,7 @@ view_err:
     intern_close_table(new_table);
     my_free(new_table,MYF(0));
   }
+  DEBUG_SYNC(thd, "alter_table_before_rename_result_table");
   VOID(pthread_mutex_lock(&LOCK_open));
   if (error)
   {
@@ -7438,6 +7457,7 @@ view_err:
   thd_proc_info(thd, "end");
 
   DBUG_EXECUTE_IF("sleep_alter_before_main_binlog", my_sleep(6000000););
+  DEBUG_SYNC(thd, "alter_table_before_main_binlog");
 
   ha_binlog_log_query(thd, create_info->db_type, LOGCOM_ALTER_TABLE,
                       thd->query(), thd->query_length(),


Attachment: [text/bzr-bundle]
Thread
bzr commit into mysql-5.1-bugteam branch (mattias.jonsson:3238)Bug#42438Mattias Jonsson11 Dec