Merge c-870ae253.1238-1-64736c10.cust.bredbandsbolaget.se:/home/pappa/clean-mysql-5.1-new

into  c-870ae253.1238-1-64736c10.cust.bredbandsbolaget.se:/home/pappa/bug19122


sql/mysql_priv.h:
  Auto merged
sql/sql_base.cc:
  Auto merged
sql/sql_partition.cc:
  Auto merged
mysql-test/r/partition.result:
  manual merge
mysql-test/t/partition.test:
  manual merge

mysql-test/r/partition.result

@@ -890,6 +890,16 @@ s1
 2
 3
 drop table t1;
+create table t1 (a int)
+partition by key (a)
+(partition p1 engine = innodb);
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
 create table t1 (a int) engine=memory
 partition by key(a);
 insert into t1 values (1);

mysql-test/t/partition.test

@@ -1023,6 +1023,21 @@ insert into t1 values (1);
 create index inx1 on t1(a);
 drop table t1;
 
+#
+# BUG 19122 Crash after ALTER TABLE t1 REBUILD PARTITION p1
+#
+create table t1 (a int)
+partition by key (a)
+(partition p1 engine = innodb);
+
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+alter table t1 rebuild partition p1;
+
 #
 # BUG 19304 Partitions: MERGE handler not allowed in partitioned tables
 #

sql/lock.cc

@@ -828,7 +828,7 @@ int lock_and_wait_for_table_name(THD *thd, TABLE_LIST *table_list)
   if (wait_if_global_read_lock(thd, 0, 1))
     DBUG_RETURN(1);
   VOID(pthread_mutex_lock(&LOCK_open));
-  if ((lock_retcode = lock_table_name(thd, table_list)) < 0)
+  if ((lock_retcode = lock_table_name(thd, table_list, TRUE)) < 0)
     goto end;
   if (lock_retcode && wait_for_locked_table_names(thd, table_list))
   {
@@ -851,6 +851,7 @@ end:
     lock_table_name()
     thd			Thread handler
     table_list		Lock first table in this list
+    check_in_use        Do we need to check if table already in use by us
 
   WARNING
     If you are going to update the table, you should use
@@ -870,7 +871,7 @@ end:
     > 0  table locked, but someone is using it
 */
 
-int lock_table_name(THD *thd, TABLE_LIST *table_list)
+int lock_table_name(THD *thd, TABLE_LIST *table_list, bool check_in_use)
 {
   TABLE *table;
   char  key[MAX_DBKEY_LENGTH];
@@ -882,17 +883,22 @@ int lock_table_name(THD *thd, TABLE_LIST *table_list)
 
   key_length= create_table_def_key(thd, key, table_list, 0);
 
-  /* Only insert the table if we haven't insert it already */
+  if (check_in_use)
-  for (table=(TABLE*) hash_first(&open_cache, (byte*)key, key_length, &state);
-       table ;
-       table = (TABLE*) hash_next(&open_cache,(byte*) key,key_length, &state))
   {
-    if (table->in_use == thd)
+    /* Only insert the table if we haven't insert it already */
+    for (table=(TABLE*) hash_first(&open_cache, (byte*)key,
+                                   key_length, &state);
+         table ;
+         table = (TABLE*) hash_next(&open_cache,(byte*) key,
+                                    key_length, &state))
     {
-      DBUG_PRINT("info", ("Table is in use"));
+      if (table->in_use == thd)
-      table->s->version= 0;                  // Ensure no one can use this
+      {
-      table->locked_by_name= 1;
+        DBUG_PRINT("info", ("Table is in use"));
-      DBUG_RETURN(0);
+        table->s->version= 0;                  // Ensure no one can use this
+        table->locked_by_name= 1;
+        DBUG_RETURN(0);
+      }
     }
   }
   /*
@@ -917,10 +923,17 @@ int lock_table_name(THD *thd, TABLE_LIST *table_list)
     my_free((gptr) table,MYF(0));
     DBUG_RETURN(-1);
   }
-  
+
-  /* Return 1 if table is in use */
+  if (!check_in_use)
-  DBUG_RETURN(test(remove_table_from_cache(thd, db, table_list->table_name,
+  {
-                                           RTFC_NO_FLAG)));
+    DBUG_RETURN(0);
+  }
+  else
+  {
+    /* Return 1 if table is in use */
+    DBUG_RETURN(test(remove_table_from_cache(thd, db, table_list->table_name,
+                                             RTFC_NO_FLAG)));
+  }
 }
 
 
@@ -1003,7 +1016,7 @@ bool lock_table_names(THD *thd, TABLE_LIST *table_list)
   for (lock_table= table_list; lock_table; lock_table= lock_table->next_local)
   {
     int got_lock;
-    if ((got_lock=lock_table_name(thd,lock_table)) < 0)
+    if ((got_lock=lock_table_name(thd,lock_table, TRUE)) < 0)
       goto end;					// Fatal error
     if (got_lock)
       got_all_locks=0;				// Someone is using table

sql/mysql_priv.h

@@ -1165,7 +1165,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
                                 HA_CREATE_INFO *create_info,
                                 TABLE_LIST *table_list,
                                 List<create_field> *create_list,
-                                List<Key> *key_list, const char *db,
+                                List<Key> *key_list, char *db,
                                 const char *table_name,
                                 uint fast_alter_partition);
 uint prep_alter_part_table(THD *thd, TABLE *table, ALTER_INFO *alter_info,
@@ -1195,6 +1195,7 @@ void create_subpartition_name(char *out, const char *in1,
 
 typedef struct st_lock_param_type
 {
+  TABLE_LIST table_list;
   ulonglong copied;
   ulonglong deleted;
   THD *thd;
@@ -1205,7 +1206,7 @@ typedef struct st_lock_param_type
   List<Key> new_key_list;
   TABLE *table;
   KEY *key_info_buffer;
-  const char *db;
+  char *db;
   const char *table_name;
   const void *pack_frm_data;
   enum thr_lock_type old_lock_type;
@@ -1677,7 +1678,7 @@ void unset_protect_against_global_read_lock(void);
 
 /* Lock based on name */
 int lock_and_wait_for_table_name(THD *thd, TABLE_LIST *table_list);
-int lock_table_name(THD *thd, TABLE_LIST *table_list);
+int lock_table_name(THD *thd, TABLE_LIST *table_list, bool check_in_use);
 void unlock_table_name(THD *thd, TABLE_LIST *table_list);
 bool wait_for_locked_table_names(THD *thd, TABLE_LIST *table_list);
 bool lock_table_names(THD *thd, TABLE_LIST *table_list);

sql/sql_base.cc

@@ -2684,7 +2684,7 @@ retry:
       goto err;
 
      // Code below is for repairing a crashed file
-     if ((error= lock_table_name(thd, table_list)))
+     if ((error= lock_table_name(thd, table_list, TRUE)))
      {
        if (error < 0)
  	goto err;

sql/sql_partition.cc

@@ -5372,6 +5372,79 @@ static void release_log_entries(partition_info *part_info)
 }
 
 
+/*
+  Get a lock on table name to avoid that anyone can open the table in
+  a critical part of the ALTER TABLE.
+  SYNOPSIS
+    get_name_lock()
+    lpt                        Struct carrying parameters
+  RETURN VALUES
+    FALSE                      Success
+    TRUE                       Failure
+*/
+
+static int get_name_lock(ALTER_PARTITION_PARAM_TYPE *lpt)
+{
+  int error= 0;
+  DBUG_ENTER("get_name_lock");
+
+  bzero(&lpt->table_list, sizeof(lpt->table_list));
+  lpt->table_list.db= lpt->db;
+  lpt->table_list.table= lpt->table;
+  lpt->table_list.table_name= (char*)lpt->table_name;
+  pthread_mutex_lock(&LOCK_open);
+  error= lock_table_name(lpt->thd, &lpt->table_list, FALSE);
+  pthread_mutex_unlock(&LOCK_open);
+  DBUG_RETURN(error);
+}
+
+
+/*
+  Unlock and close table before renaming and dropping partitions
+  SYNOPSIS
+    alter_close_tables()
+    lpt                        Struct carrying parameters
+  RETURN VALUES
+    0
+*/
+
+static int alter_close_tables(ALTER_PARTITION_PARAM_TYPE *lpt)
+{
+  THD *thd= lpt->thd;
+  TABLE *table= lpt->table;
+  DBUG_ENTER("alter_close_tables");
+  /*
+    We need to also unlock tables and close all handlers.
+    We set lock to zero to ensure we don't do this twice
+    and we set db_stat to zero to ensure we don't close twice.
+  */
+  mysql_unlock_tables(thd, thd->lock);
+  thd->lock= 0;
+  table->file->close();
+  table->db_stat= 0;
+  DBUG_RETURN(0);
+}
+
+
+/*
+  Release a lock name
+  SYNOPSIS
+    release_name_lock()
+    lpt
+  RETURN VALUES
+    0
+*/
+
+static int release_name_lock(ALTER_PARTITION_PARAM_TYPE *lpt)
+{
+  DBUG_ENTER("release_name_lock");
+  pthread_mutex_lock(&LOCK_open);
+  unlock_table_name(lpt->thd, &lpt->table_list);
+  pthread_mutex_unlock(&LOCK_open);
+  DBUG_RETURN(0);
+}
+
+
 /*
   Handle errors for ALTER TABLE for partitioning
   SYNOPSIS
@@ -5522,7 +5595,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
                                 HA_CREATE_INFO *create_info,
                                 TABLE_LIST *table_list,
                                 List<create_field> *create_list,
-                                List<Key> *key_list, const char *db,
+                                List<Key> *key_list, char *db,
                                 const char *table_name,
                                 uint fast_alter_partition)
 {
@@ -5662,8 +5735,16 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
       2) Write the ddl log to ensure that the operation is completed
          even in the presence of a MySQL Server crash
       3) Lock the table in TL_WRITE_ONLY to ensure all other accesses to
-         the table have completed
+         the table have completed. This ensures that other threads can not
-      4) Write the bin log
+         execute on the table in parallel.
+      4) Get a name lock on the table. This ensures that we can release all
+         locks on the table and since no one can open the table, there can
+         be no new threads accessing the table. They will be hanging on the
+         name lock.
+      5) Close all tables that have already been opened but didn't stumble on
+         the abort locked previously.
+      6) We are now ready to release all locks we got in this thread.
+      7) Write the bin log
          Unfortunately the writing of the binlog is not synchronised with
          other logging activities. So no matter in which order the binlog
          is written compared to other activities there will always be cases
@@ -5674,14 +5755,13 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
          require writing the statement first in the ddl log and then
          when recovering from the crash read the binlog and insert it into
          the binlog if not written already.
-      5) Install the previously written shadow frm file
+      8) Install the previously written shadow frm file
-      6) Ensure that any users that has opened the table but not yet
+      9) Prepare handlers for drop of partitions
-         reached the abort lock do that before downgrading the lock.
+      10) Drop the partitions
-      7) Prepare MyISAM handlers for drop of partitions
+      11) Remove entries from ddl log
-      8) Drop the partitions
+      12) Release name lock so that all other threads can access the table
-      9) Remove entries from ddl log
+          again.
-      10) Wait until all accesses using the old frm file has completed
+      13) Complete query
-      11) Complete query
 
       We insert Error injections at all places where it could be interesting
       to test if recovery is properly done.
@@ -5694,22 +5774,26 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
         ERROR_INJECT_CRASH("crash_drop_partition_3") ||
         (not_completed= FALSE) ||
         abort_and_upgrade_lock(lpt) || /* Always returns 0 */
+        ERROR_INJECT_CRASH("crash_drop_partition_4") ||
         ((!thd->lex->no_write_to_binlog) &&
          (write_bin_log(thd, FALSE,
                         thd->query, thd->query_length), FALSE)) ||
-        ERROR_INJECT_CRASH("crash_drop_partition_4") ||
-        (table->file->extra(HA_EXTRA_PREPARE_FOR_DELETE), FALSE) ||
         ERROR_INJECT_CRASH("crash_drop_partition_5") ||
         ((frm_install= TRUE), FALSE) ||
         mysql_write_frm(lpt, WFRM_INSTALL_SHADOW) ||
         ((frm_install= FALSE), FALSE) ||
-        (close_open_tables_and_downgrade(lpt), FALSE) || 
         ERROR_INJECT_CRASH("crash_drop_partition_6") ||
-        mysql_drop_partitions(lpt) ||
+        get_name_lock(lpt) || /* Always returns 0 */
         ERROR_INJECT_CRASH("crash_drop_partition_7") ||
-        (write_log_completed(lpt, FALSE), FALSE) ||
+        (close_open_tables_and_downgrade(lpt), FALSE) ||
         ERROR_INJECT_CRASH("crash_drop_partition_8") ||
-        (mysql_wait_completed_table(lpt, table), FALSE))
+        alter_close_tables(lpt) ||
+        ERROR_INJECT_CRASH("crash_drop_partition_9") ||
+        mysql_drop_partitions(lpt) ||
+        ERROR_INJECT_CRASH("crash_drop_partition_10") ||
+        (write_log_completed(lpt, FALSE), FALSE) ||
+        ERROR_INJECT_CRASH("crash_drop_partition_11") ||
+        (release_name_lock(lpt), FALSE)) 
     {
       handle_alter_part_error(lpt, not_completed, TRUE, frm_install);
       DBUG_RETURN(TRUE);
@@ -5814,16 +5898,20 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
       5) Lock all partitions in TL_WRITE_ONLY to ensure that no users
          are still using the old partitioning scheme. Wait until all
          ongoing users have completed before progressing.
-      6) Prepare MyISAM handlers for rename and delete of partitions
+      6) Get a name lock of the table
-      7) Rename the reorged partitions such that they are no longer
+      7) Close all tables opened but not yet locked, after this call we are
-         used and rename those added to their real new names.
+         certain that no other thread is in the lock wait queue or has
-      8) Write bin log
+         opened the table. The name lock will ensure that they are blocked
-      9) Install the shadow frm file
+         on the open call.
-      10) Wait until all accesses using the old frm file has completed
+      8) Close all partitions opened by this thread, but retain name lock.
-      11) Drop the reorganised partitions
+      9) Write bin log
-      12) Remove log entry
+      10) Prepare handlers for rename and delete of partitions
-      13)Wait until all accesses using the old frm file has completed
+      11) Rename and drop the reorged partitions such that they are no
-      14)Complete query
+          longer used and rename those added to their real new names.
+      12) Install the shadow frm file
+      13) Release the name lock to enable other threads to start using the
+          table again.
+      14) Complete query
     */
     if (write_log_add_change_partition(lpt) ||
         ERROR_INJECT_CRASH("crash_change_partition_1") ||
@@ -5835,22 +5923,25 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
         ERROR_INJECT_CRASH("crash_change_partition_4") ||
         (not_completed= FALSE) ||
         abort_and_upgrade_lock(lpt) || /* Always returns 0 */
+        ERROR_INJECT_CRASH("crash_change_partition_5") ||
         ((!thd->lex->no_write_to_binlog) &&
          (write_bin_log(thd, FALSE,
                         thd->query, thd->query_length), FALSE)) ||
-        ERROR_INJECT_CRASH("crash_change_partition_5") ||
-        (table->file->extra(HA_EXTRA_PREPARE_FOR_DELETE), FALSE) ||
         ERROR_INJECT_CRASH("crash_change_partition_6") ||
-        mysql_rename_partitions(lpt) ||
-        ((frm_install= TRUE), FALSE) ||
-        ERROR_INJECT_CRASH("crash_change_partition_7") ||
         mysql_write_frm(lpt, WFRM_INSTALL_SHADOW) ||
+        ERROR_INJECT_CRASH("crash_change_partition_7") ||
+        get_name_lock(lpt) ||
         ERROR_INJECT_CRASH("crash_change_partition_8") ||
         (close_open_tables_and_downgrade(lpt), FALSE) ||
         ERROR_INJECT_CRASH("crash_change_partition_9") ||
-        (write_log_completed(lpt, FALSE), FALSE) ||
+        alter_close_tables(lpt) ||
         ERROR_INJECT_CRASH("crash_change_partition_10") ||
-        (mysql_wait_completed_table(lpt, table), FALSE))
+        mysql_rename_partitions(lpt) ||
+        ((frm_install= TRUE), FALSE) ||
+        ERROR_INJECT_CRASH("crash_change_partition_11") ||
+        (write_log_completed(lpt, FALSE), FALSE) ||
+        ERROR_INJECT_CRASH("crash_change_partition_12") ||
+        (release_name_lock(lpt), FALSE))
     {
       handle_alter_part_error(lpt, not_completed, FALSE, frm_install);
       DBUG_RETURN(TRUE);