MySQL Bugs: #50561: ALTER PARTITIONS does not have adequate lock, breaks with concurrent I

Bug #50561	ALTER PARTITIONS does not have adequate lock, breaks with concurrent I_S query
Submitted:	23 Jan 2010 1:12	Modified:	14 Oct 2010 12:58
Reporter:	Mattias Jonsson	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Partitions	Severity:	S2 (Serious)
Version:	5.1+	OS:	Any
Assigned to:	Mattias Jonsson	CPU Architecture:	Any

Description:
When running concurrent SELECT * FROM I_S.PARTITIONS and ALTER TABLE t_part ADD/DROP/REORGANIZE PARTITION concurrently, there is a possibility that the table gets opened in get_all_tables right before the final rename stage of the alter. This results in a collision which can lead to several different things. Among one is that the partition to be deleted cannot be deleted since it is opened (by the I_S connection) which results in inconsistency in the innodb data dictionary.

Found when testing bugs around bug#47343 (bug#45808 etc.)

How to repeat:
Patch with (from mysql-5.1-bugteam, davi.arnaut@sun.com-20100121002220-hycqwoeyv23y6wul):
=== modified file 'sql/ha_partition.cc'
--- sql/ha_partition.cc	2010-01-19 16:02:51 +0000
+++ sql/ha_partition.cc	2010-01-22 23:36:50 +0000
@@ -58,6 +58,10 @@
 
 #include <mysql/plugin.h>
 
+#if defined(ENABLED_DEBUG_SYNC)
+#include "debug_sync.h"
+#endif
+
 static const char *ha_par_ext= ".par";
 #ifdef NOT_USED
 static int free_share(PARTITION_SHARE * share);
@@ -690,6 +694,7 @@
   DBUG_ASSERT(!strcmp(path, get_canonical_filename(m_file[0], path,
                                                    norm_name_buff)));
 
+  DEBUG_SYNC(ha_thd(), "before_rename_partitions");
   if (temp_partitions)
   {
     /*

=== modified file 'sql/sql_show.cc'
--- sql/sql_show.cc	2010-01-19 09:03:40 +0000
+++ sql/sql_show.cc	2010-01-23 00:18:07 +0000
@@ -30,6 +30,9 @@
 #include "event_data_objects.h"
 #endif
 #include <my_dir.h>
+#if defined(ENABLED_DEBUG_SYNC)
+#include "debug_sync.h"
+#endif
 
 #define STR_OR_NIL(S) ((S) ? (S) : "<nil>")
 
@@ -3361,6 +3364,7 @@
     goto err;
   }
 
+  DEBUG_SYNC(thd, "before_open_in_get_all_tables");
   if (make_db_list(thd, &db_names, &lookup_field_vals, &with_i_schema))
     goto err;
   it.rewind(); /* To get access to new elements in basis list */
@@ -3499,6 +3503,7 @@
       with_i_schema= 0;
     }
   }
+  DEBUG_SYNC(thd, "after_open_in_get_all_tables");
 
   error= 0;
 err:

And run this test:
--source include/have_innodb.inc
--source include/have_partition.inc
--source include/have_debug_sync.inc

create table t1 (a int)
engine = innodb
partition by range (a)
(partition p0 values less than (10),
 partition p10 values less than (20),
 partition p20 values less than (30),
 partition p30 values less than MAXVALUE);
insert into t1 values (1), (11), (21), (33);

SET DEBUG_SYNC='before_open_in_get_all_tables SIGNAL parked WAIT_FOR open';
SET DEBUG_SYNC='after_open_in_get_all_tables SIGNAL alter WAIT_FOR finish';
send
SELECT * FROM INFORMATION_SCHEMA.PARTITIONS WHERE TABLE_NAME = 't1';

connect (con1, localhost, root,,);
SET DEBUG_SYNC = 'now WAIT_FOR parked';
SET DEBUG_SYNC = 'before_rename_partitions SIGNAL open WAIT_FOR alter';
ALTER TABLE t1 REORGANIZE PARTITION p30 INTO
(PARTITION p30 VALUES LESS THAN (40),
 PARTITION p40 VALUES LESS THAN MAXVALUE);
SET DEBUG_SYNC = 'now SIGNAL finish';

disconnect con1;
connection default;
--reap
drop table t1;
SET DEBUG_SYNC = 'RESET';

Suggested fix:
Increase the time holding LOCK_open to include the rename/delete operation in the final stage of fast_alter_partition_table.

Also change the error handling to:
1) rename old partition to #REN#
2) rename new partition to old (original name)
3) delete #REN# partition

instead of starting with deleting old partition, leaving no route to restore in case of errors.

NOTE that I_S is ignoring both name-locks and flush as an optimization. LOCK_open is needed in this case to prevent I_S to break the final stage of ALTER. (as is done in non partitioned ALTER).

Thank you for the report.

Which result should be after test case run?

Apply this diff to a mysql-5.1(-bugteam) tree

Attachment: b50561.how_to_repeat.diff (application/octet-stream, text), 2.28 KiB.

unpack these test files under mysql-test and run ./mtr b50561

Attachment: b50561-test.tgz (application/x-gzip, text), 1.33 KiB.

Then test with the current behavior (i.e allow I_S to ignore flush and name locks) by changing '#if 1' to '#if 0' in sql/sql_show.cc (see diff).

Then re-run the test and you should get:
1) t1#P#p0#TMP#.ibd 'left-over' file in the test db
2) 11,22 and 33 as duplicate rows !

And this in the error log:
100126 19:00:09 [ERROR] Failed to open table test/t1#P#p10 after 10 attemtps.

100126 19:00:09 [ERROR] Cannot find or open table test/t1#P#p10 from
the internal data dictionary of InnoDB though the .frm file for the
table exists. Maybe you have deleted and recreated InnoDB data
files but have forgotten to delete the corresponding .frm files
of InnoDB tables, or you have moved .frm files to another database?
or, the table contains indexes that this version of the engine
doesn't support.
See http://dev.mysql.com/doc/refman/5.1/en/innodb-troubleshooting.html
how you can resolve the problem.

100126 19:00:09 InnoDB: Warning: MySQL is trying to drop table `test`.`t1` /* Partition `p0` */
InnoDB: though there are still open handles to it.
InnoDB: Adding the table to the background drop queue.
100126 19:00:10 InnoDB: Error; possible reasons:
InnoDB: 1) Table rename would cause two FOREIGN KEY constraints
InnoDB: to have the same internal name in case-insensitive comparison.
InnoDB: 2) table `test`.`t1` /* Partition `p0` */ exists in the InnoDB internal data
InnoDB: dictionary though MySQL is trying to rename table `test`.`t1` /* Temporary Partition `p0` */ to it.
InnoDB: Have you deleted the .frm file and not used DROP TABLE?
InnoDB: You can look for further help from
InnoDB: http://dev.mysql.com/doc/refman/5.1/en/innodb-troubleshooting.html
InnoDB: If table `test`.`t1` /* Partition `p0` */ is a temporary table #sql..., then it can be that
InnoDB: there are still queries running on the table, and it will be
InnoDB: dropped automatically when the queries end.
InnoDB: You can drop the orphaned table inside InnoDB by
InnoDB: creating an InnoDB table with the same name in another
InnoDB: database and copying the .frm file to the current database.
InnoDB: Then MySQL thinks the table exists, and DROP TABLE will
InnoDB: succeed.

Thank you for the report.

Verified as described.

I will try to fix the test case from bug#47343 which will fail after the MDL implementation has been pushed and probably add a DBUG_EXECUTE_IF to return failure instead to simulate error in the storage engine.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/99810

3332 Mattias Jonsson	2010-02-10
      Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
      concurrent I_S query
      
      Problem was the mutex LOCK_open was released while handling the frm-file
      and renaming partitions. Which allowed the table to be opened with
      MYSQL_LOCK_IGNORE_FLUSH flag resulting in renaming a partition that was
      already in use, which could cause the table to be unusable.
      
      Solution was to hold LOCK_open from the time of waiting on all other
      instances of the table to be closed until all renaming operations are
      done.
     @ mysql-test/suite/parts/r/partition_debug_sync.result
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        New test result file
     @ mysql-test/suite/parts/t/partition_debug_sync.test
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        New test file
     @ sql/ha_partition.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added two DEBUG_SYNC point to be able to verify test
        
        Minor spelling correction
     @ sql/ha_partition.h
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Minor spelling correction
     @ sql/mysql_priv.h
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Modified wait_while_table_is_used to also be able to check if
        the thread is killed
        
        Added WFRM_HOLDING_LOCK for signalling that LOCK_open is held
        when calling mysql_write_frm
     @ sql/sql_base.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added one DEBUG_SYNC point.
        
        Updated the wait_while_table_is_used calls.
        
        Removed abort_and_upgrade_lock, since partitioning now uses
        the underlying calls directly, as mysql_alter_table does.
        
        Removed close_open_tables_and_downgrade, since that was dead code.
        (Note that mysql_lock_downgrade_write still exists unused!)
     @ sql/sql_partition.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Changed the flow in fast_alter_partition_table take
        LOCK_open, wait_while_table_is_used, close_data_file_and_morph_locks
        directly instead of relying on wrapper functions, since LOCK_open must
        be held during this critical section.
        
        Removed alter_close_tables, abort_and_upgrade_lock since they
        was only used in fast_alter_partition_table and is no longer used.
     @ sql/sql_show.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added one DEBUG_SYNC point.
     @ sql/sql_table.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added a flag to mysql_write_frm to be able to signal if LOCK_open
        already is held, or if it is needed to be taken.
        
        Added a flag to wait_while_table_is_used to allow the function
        to be used by partitioning.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/100579

3332 Mattias Jonsson	2010-02-17
      Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
      concurrent I_S query
      
      Problem was the mutex LOCK_open was released while handling the frm-file
      and renaming partitions. Which allowed the table to be opened with
      MYSQL_LOCK_IGNORE_FLUSH flag resulting in renaming a partition that was
      already in use, which could cause the table to be unusable.
      
      Solution was to hold LOCK_open from the time of waiting on all other
      instances of the table to be closed until all renaming operations are
      done.
      
      (Update: removed ifdef dependency for include of debug_sync.h)
     @ mysql-test/suite/parts/r/partition_debug_sync.result
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        New test result file
     @ mysql-test/suite/parts/t/partition_debug_sync.test
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        New test file
     @ sql/ha_partition.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added two DEBUG_SYNC point to be able to verify test
        
        Minor spelling correction
     @ sql/ha_partition.h
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Minor spelling correction
     @ sql/mysql_priv.h
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Modified wait_while_table_is_used to also be able to check if
        the thread is killed
        
        Added WFRM_HOLDING_LOCK for signalling that LOCK_open is held
        when calling mysql_write_frm
     @ sql/sql_base.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added one DEBUG_SYNC point.
        
        Updated the wait_while_table_is_used calls.
        
        Removed abort_and_upgrade_lock, since partitioning now uses
        the underlying calls directly, as mysql_alter_table does.
        
        Removed close_open_tables_and_downgrade, since that was dead code.
        (Note that mysql_lock_downgrade_write still exists unused!)
     @ sql/sql_partition.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Changed the flow in fast_alter_partition_table take
        LOCK_open, wait_while_table_is_used, close_data_file_and_morph_locks
        directly instead of relying on wrapper functions, since LOCK_open must
        be held during this critical section.
        
        Removed alter_close_tables, abort_and_upgrade_lock since they
        was only used in fast_alter_partition_table and is no longer used.
     @ sql/sql_show.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added one DEBUG_SYNC point.
     @ sql/sql_table.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added a flag to mysql_write_frm to be able to signal if LOCK_open
        already is held, or if it is needed to be taken.
        
        Added a flag to wait_while_table_is_used to allow the function
        to be used by partitioning.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/103516

3404 Mattias Jonsson	2010-03-17
      Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
      concurrent I_S query
      
      There were two problem:
      1) MYSQL_LOCK_IGNORE_FLUSH also ignored name locks
      2) there was a race between abort_and_upgrade_locks and
         alter_close_tables
         (i.e. remove_table_from_cache and
          close_data_files_and_morph_locks)
      
      Which allowed the table to be opened with MYSQL_LOCK_IGNORE_FLUSH flag
      resulting in renaming a partition that was already in use,
      which could cause the table to be unusable.
      
      Solution was to not release the LOCK_open mutex in
      abort_and_upgrade_locks, and not take it again in alter_close_tables
      (i.e. keep it locked).
      
      And to not allow IGNORE_FLUSH to skip waiting for a named locked table.
     @ mysql-test/suite/parts/r/partition_debug_sync_innodb.result
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added test result
     @ mysql-test/suite/parts/t/partition_debug_sync_innodb-master.opt
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added test option
     @ mysql-test/suite/parts/t/partition_debug_sync_innodb.test
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added test file
     @ sql/authors.h
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Time to be acknowledged :)
     @ sql/ha_partition.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added DEBUG_SYNC for deterministic testing
     @ sql/sql_base.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Changed MYSQL_LOCK_IGNORE_FLUSH to not ignore name locks
        
        Do not release LOCK_open in abort_and_upgrade_locks
        (to be released in alter_close_tables)
        
        Added DEBUG_SYNC for deterministic testing
     @ sql/sql_partition.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Do not take LOCK_open in alter_close_tables, (since it now is never
        released in abort_and_upgrade_locks).
     @ sql/sql_show.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added DEBUG_SYNC for deterministic testing

New patch proposed, davi agreed to look at the locking part of the patch and correct it if needed. dlenev had some worries about deadlocks if IGNORE_FLUSH did not ignore name locks too if I understood him correctly.

merging the two functions abort_and_upgrade_lock and alter_close_tables into one function, so that LOCK_open is not taken in one function and released in another...

Also trying to create a test case where the LOCK_open glitch can be verified. (the code line for only ignore flush and not open_placeholder does fix the reported test case).

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/103601

3404 Mattias Jonsson	2010-03-17
      Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
      concurrent I_S query
      
      There were two problem:
      1) MYSQL_LOCK_IGNORE_FLUSH also ignored name locks
      2) there was a race between abort_and_upgrade_locks and
         alter_close_tables
         (i.e. remove_table_from_cache and
          close_data_files_and_morph_locks)
      
      Which allowed the table to be opened with MYSQL_LOCK_IGNORE_FLUSH flag
      resulting in renaming a partition that was already in use,
      which could cause the table to be unusable.
      
      Solution was to not allow IGNORE_FLUSH to skip waiting for
      a named locked table.
      
      And to not release the LOCK_open mutex between the
      calls to remove_table_from_cache and
      close_data_files_and_morph_locks by merging the functions
      abort_and_upgrade_locks and alter_close_tables.
     @ mysql-test/suite/parts/r/partition_debug_sync_innodb.result
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added test result
     @ mysql-test/suite/parts/t/partition_debug_sync_innodb-master.opt
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added test option
     @ mysql-test/suite/parts/t/partition_debug_sync_innodb.test
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added test file
     @ sql/authors.h
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Time to be acknowledged :)
     @ sql/ha_partition.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added DEBUG_SYNC for deterministic testing
     @ sql/mysql_priv.h
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Renamed function since merging alter_close_tables into
        abort_and_upgrade_lock.
     @ sql/sql_base.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Changed MYSQL_LOCK_IGNORE_FLUSH to not ignore name locks
        (open_placeholder).
        
        Merged alter_close_tables into abort_and_upgrade_locks
        (and added _and_close_table to the name)
        to not release LOCK_open between remove_table_from_cache
        and close_data_files_and_morph_locks.
        
        Added DEBUG_SYNC for deterministic testing.
     @ sql/sql_partition.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Removed alter_close_tables, (merged it into
        abort_and_upgrad_lock) so that LOCK_open never is released
        between remove_table_from_cache and
        close_data_files_and_morph_locks.
     @ sql/sql_show.cc
        Bug#50561: ALTER PARTITIONS does not have adequate lock, breaks with
        concurrent I_S query
        
        Added DEBUG_SYNC for deterministic testing

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/103693

3405 Mattias Jonsson	2010-03-18
      Additional fix for DEBUG_SYNC which failed for some rpl-tests,
      due to DBUG_ASSERT. (added in bug#50561)
     @ sql/sql_base.cc
        DEBUG_SYNC asserts that thd->debug_sync_control is set.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/107845

4055 Mattias Jonsson	2010-05-10 [merge]
      Manual merge of bug#50561 into mysql-pe.

Pushed to mysql-pe and mysql-5.1-bugteam

Pushed into mysql-next-mr (revid:alik@sun.com-20100524190136-egaq7e8zgkwb9aqi) (version source revid:alik@sun.com-20100512070920-xgpmqeytp0gc183c) (pib:16)

Pushed into 6.0.14-alpha (revid:alik@sun.com-20100524190941-nuudpx60if25wsvx) (version source revid:alik@sun.com-20100514054548-91z72f0mcskr84kj) (merge vers: 6.0.14-alpha) (pib:16)

Pushed into 5.5.5-m3 (revid:alik@sun.com-20100524185725-c8k5q7v60i5nix3t) (version source revid:alexey.kopytov@sun.com-20100511160250-pevxq2uoey47dw1f) (merge vers: 5.5.5-m3) (pib:16)

Documented bugfix in the 5.5.5 and 6.0.14 changelogs as follows:

        ALTER TABLE statements that cause partitions of InnoDB tables to
        be renamed or dropped (such as ALTER TABLE ... ADD PARTITION,
        ALTER TABLE ... DROP PARTITION, and ALTER TABLE ... REORGANIZE
        PARTITION) -- when run concurrently with queries against
        the INFORMATION_SCHEMA.PARTITIONS table -- could fail,
        cause the affected partitioned tables to become unusable, or
        both. This was due to the fact that the INFORMATION_SCHEMA
        database ignored the name lock imposed by the ALTER TABLE
        statement on the partitions affected. In particular, this led to
        problems with InnoDB tables, because InnoDB would accept the
        rename operation, but put it in a background queue, so that
        subsequent rename operations failed when InnoDB was unable to
        find the correct partition. Now, INFORMATION_SCHEMA honors name
        locks imposed by ongoing ALTER TABLE statements that cause
        partitions to be renamed or dropped.

        See also BUG#45808 and BUG#47343.

Status = NM -- waiting for push to 5.1 tree.

Pushed into 5.1.48 (revid:georgi.kodinov@oracle.com-20100602084411-2yu607bslbmgufl3) (version source revid:mattias.jonsson@sun.com-20100510131706-wfci7c0gb7072wwa) (merge vers: 5.1.47) (pib:16)

Also documented in the 5.1.48 changelog. Closed.

This bugfix is suggested to be the cause of the regression reported as Bug#56541.

Pushed into mysql-5.1-telco-7.0 5.1.51-ndb-7.0.20 (revid:martin.skold@mysql.com-20101014082627-jrmy9xbfbtrebw3c) (version source revid:vasil.dimov@oracle.com-20100513074652-0cvlhgkesgbb2bfh) (merge vers: 5.5.5-m3) (pib:21)

Pushed into mysql-5.1-telco-6.3 5.1.51-ndb-6.3.39 (revid:martin.skold@mysql.com-20101014083757-5qo48b86d69zjvzj) (version source revid:vasil.dimov@oracle.com-20100513074652-0cvlhgkesgbb2bfh) (merge vers: 5.5.5-m3) (pib:21)

Pushed into mysql-5.1-telco-6.2 5.1.51-ndb-6.2.19 (revid:martin.skold@mysql.com-20101014084420-y54ecj85j5we27oa) (version source revid:vasil.dimov@oracle.com-20100513074652-0cvlhgkesgbb2bfh) (merge vers: 5.5.5-m3) (pib:21)

Already documented in 5.1.48 changelog; no new changelog entries required. 

Setting back to Closed.