From: Dmitry Lenev Date: May 26 2011 3:03pm Subject: bzr push into mysql-5.1 branch (Dmitry.Lenev:3630 to 3631) Bug#11762012 List-Archive: http://lists.mysql.com/commits/138226 X-Bug: 11762012 Message-Id: <20110526150346.F38BB74067D@bandersnatch> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3631 Dmitry Lenev 2011-05-26 Fix for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK". Attempt to update an InnoDB temporary table under LOCK TABLES led to assertion failure in both debug and production builds if this temporary table was explicitly locked for READ. The same scenario works fine for MyISAM temporary tables. The assertion failure was caused by discrepancy between lock that was requested on the rows of temporary table at LOCK TABLES time and by update operation. Since SQL-layer requested a read-lock at LOCK TABLES time InnoDB engine assumed that upcoming statements which are going to be executed under LOCK TABLES will only read table and therefore should acquire only S-lock. An update operation broken this assumption by requesting X-lock. Possible approaches to fixing this problem are: 1) Skip locking of temporary tables as locking doesn't make any sense for connection-local objects. 2) Prohibit changing of temporary table locked by LOCK TABLES ... READ. Unfortunately both of these approaches have drawbacks which make them unviable for stable versions of server. So this patch takes another approach and changes code in such way that LOCK TABLES for a temporary table will always request write lock. In 5.1 version of this patch switch from read lock to write lock is done inside of InnoDBs handler methods as doing it on SQL-layer causes compatibility troubles with FLUSH TABLES WITH READ LOCK. @ mysql-test/suite/innodb/r/innodb_mysql.result Added test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK". @ mysql-test/suite/innodb/t/innodb_mysql.test Added test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK". @ mysql-test/suite/innodb_plugin/r/innodb_mysql.result Added test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK". @ mysql-test/suite/innodb_plugin/t/innodb_mysql.test Added test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK". @ storage/innobase/handler/ha_innodb.cc Assume that a temporary table locked by LOCK TABLES can be updated even if it was only locked for read and therefore an X-lock should be always requested for such tables. @ storage/innodb_plugin/handler/ha_innodb.cc Assume that a temporary table locked by LOCK TABLES can be updated even if it was only locked for read and therefore an X-lock should be always requested for such tables. modified: mysql-test/suite/innodb/r/innodb_mysql.result mysql-test/suite/innodb/t/innodb_mysql.test mysql-test/suite/innodb_plugin/r/innodb_mysql.result mysql-test/suite/innodb_plugin/t/innodb_mysql.test storage/innobase/handler/ha_innodb.cc storage/innodb_plugin/handler/ha_innodb.cc 3630 Sven Sandberg 2011-05-26 BUG#12574820: binlog.binlog_tmp_table timing out in daily and weekly trunk run Problem: MYSQL_BIN_LOG::reset_logs acquires mutexes in wrong order. The correct order is first LOCK_thread_count and then LOCK_log. This function does it the other way around. This leads to deadlock when run in parallel with a thread that takes the two locks in correct order. For example, a thread that disconnects will take the locks in the correct order. Fix: change order of the locks in MYSQL_BIN_LOG::reset_logs: first LOCK_thread_count and then LOCK_log. @ mysql-test/suite/binlog/r/binlog_reset_master.result added result file @ mysql-test/suite/binlog/t/binlog_reset_master.test Added test case that demonstrates deadlock because of wrong mutex order. The deadlock is between two threads: - RESET MASTER acquires mutexes in wrong order. - client thread shutdown code acquires mutexes in right order. Actually, this test case does not produce deadlock in 5.1, probably the client thread shutdown code does not hold both mutexes at the same time. However, the bug existed in 5.1 (mutexes are taken in the wrong order) so we push the test case to 5.1 too, to prevent future regressions. @ sql/log.cc Change mutex acquisition to the correct order: first LOCK_thread_count, then LOCK_log. @ sql/mysqld.cc Add debug code to synchronize test case. added: mysql-test/suite/binlog/r/binlog_reset_master.result mysql-test/suite/binlog/t/binlog_reset_master.test modified: sql/log.cc sql/mysqld.cc === modified file 'mysql-test/suite/innodb/r/innodb_mysql.result' --- a/mysql-test/suite/innodb/r/innodb_mysql.result 2010-12-08 14:21:03 +0000 +++ b/mysql-test/suite/innodb/r/innodb_mysql.result 2011-05-26 13:14:47 +0000 @@ -2639,7 +2639,6 @@ COUNT(*) 1537 SET SESSION sort_buffer_size = DEFAULT; DROP TABLE t1; -End of 5.1 tests # # Test for bug #39932 "create table fails if column for FK is in different # case than in corr index". @@ -2668,3 +2667,20 @@ DROP TABLE IF EXISTS t1, t2; CREATE TABLE t1 (a INT, INDEX(a)) engine=innodb; ALTER TABLE t1 RENAME TO t2, DISABLE KEYS; DROP TABLE IF EXISTS t1, t2; +# +# Test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE:: +# UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK". +# +DROP TABLE IF EXISTS t1; +CREATE TEMPORARY TABLE t1 (c int) ENGINE = InnoDB; +INSERT INTO t1 VALUES (1); +LOCK TABLES t1 READ; +# Even though temporary table was locked for READ we +# still allow writes to it to be compatible with MyISAM. +# This is possible since due to fact that temporary tables +# are specific to connection and therefore locking for them +# is irrelevant. +UPDATE t1 SET c = 5; +UNLOCK TABLES; +DROP TEMPORARY TABLE t1; +End of 5.1 tests === modified file 'mysql-test/suite/innodb/t/innodb_mysql.test' --- a/mysql-test/suite/innodb/t/innodb_mysql.test 2010-12-20 11:58:33 +0000 +++ b/mysql-test/suite/innodb/t/innodb_mysql.test 2011-05-26 13:14:47 +0000 @@ -868,9 +868,6 @@ SET SESSION sort_buffer_size = DEFAULT; DROP TABLE t1; ---echo End of 5.1 tests - - --echo # --echo # Test for bug #39932 "create table fails if column for FK is in different --echo # case than in corr index". @@ -900,3 +897,25 @@ CREATE TABLE t1 (a INT, INDEX(a)) engine ALTER TABLE t1 RENAME TO t2, DISABLE KEYS; DROP TABLE IF EXISTS t1, t2; --enable_warnings + + +--echo # +--echo # Test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE:: +--echo # UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK". +--echo # +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings +CREATE TEMPORARY TABLE t1 (c int) ENGINE = InnoDB; +INSERT INTO t1 VALUES (1); +LOCK TABLES t1 READ; +--echo # Even though temporary table was locked for READ we +--echo # still allow writes to it to be compatible with MyISAM. +--echo # This is possible since due to fact that temporary tables +--echo # are specific to connection and therefore locking for them +--echo # is irrelevant. +UPDATE t1 SET c = 5; +UNLOCK TABLES; +DROP TEMPORARY TABLE t1; + +--echo End of 5.1 tests === modified file 'mysql-test/suite/innodb_plugin/r/innodb_mysql.result' --- a/mysql-test/suite/innodb_plugin/r/innodb_mysql.result 2010-11-23 10:18:47 +0000 +++ b/mysql-test/suite/innodb_plugin/r/innodb_mysql.result 2011-05-26 13:14:47 +0000 @@ -2438,4 +2438,20 @@ COUNT(*) 1537 SET SESSION sort_buffer_size = DEFAULT; DROP TABLE t1; +# +# Test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE:: +# UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK". +# +DROP TABLE IF EXISTS t1; +CREATE TEMPORARY TABLE t1 (c int) ENGINE = InnoDB; +INSERT INTO t1 VALUES (1); +LOCK TABLES t1 READ; +# Even though temporary table was locked for READ we +# still allow writes to it to be compatible with MyISAM. +# This is possible since due to fact that temporary tables +# are specific to connection and therefore locking for them +# is irrelevant. +UPDATE t1 SET c = 5; +UNLOCK TABLES; +DROP TEMPORARY TABLE t1; End of 5.1 tests === modified file 'mysql-test/suite/innodb_plugin/t/innodb_mysql.test' --- a/mysql-test/suite/innodb_plugin/t/innodb_mysql.test 2010-11-23 10:18:47 +0000 +++ b/mysql-test/suite/innodb_plugin/t/innodb_mysql.test 2011-05-26 13:14:47 +0000 @@ -689,4 +689,24 @@ SET SESSION sort_buffer_size = DEFAULT; DROP TABLE t1; + +--echo # +--echo # Test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE:: +--echo # UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK". +--echo # +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings +CREATE TEMPORARY TABLE t1 (c int) ENGINE = InnoDB; +INSERT INTO t1 VALUES (1); +LOCK TABLES t1 READ; +--echo # Even though temporary table was locked for READ we +--echo # still allow writes to it to be compatible with MyISAM. +--echo # This is possible since due to fact that temporary tables +--echo # are specific to connection and therefore locking for them +--echo # is irrelevant. +UPDATE t1 SET c = 5; +UNLOCK TABLES; +DROP TEMPORARY TABLE t1; + --echo End of 5.1 tests === modified file 'storage/innobase/handler/ha_innodb.cc' --- a/storage/innobase/handler/ha_innodb.cc 2011-04-11 13:40:28 +0000 +++ b/storage/innobase/handler/ha_innodb.cc 2011-05-26 13:14:47 +0000 @@ -7325,10 +7325,18 @@ ha_innobase::external_lock( reset_template(prebuilt); - if (lock_type == F_WRLCK) { + if (lock_type == F_WRLCK + || (table->s->tmp_table + && thd_sql_command(thd) == SQLCOM_LOCK_TABLES)) { /* If this is a SELECT, then it is in UPDATE TABLE ... - or SELECT ... FOR UPDATE */ + or SELECT ... FOR UPDATE + + For temporary tables which are locked for READ by LOCK TABLES + updates are still allowed by SQL-layer. In order to accomodate + for such a situation we always request X-lock for such table + at LOCK TABLES time. + */ prebuilt->select_lock_type = LOCK_X; prebuilt->stored_select_lock_type = LOCK_X; } === modified file 'storage/innodb_plugin/handler/ha_innodb.cc' --- a/storage/innodb_plugin/handler/ha_innodb.cc 2011-04-11 13:40:28 +0000 +++ b/storage/innodb_plugin/handler/ha_innodb.cc 2011-05-26 13:14:47 +0000 @@ -8642,10 +8642,18 @@ ha_innobase::external_lock( reset_template(prebuilt); - if (lock_type == F_WRLCK) { + if (lock_type == F_WRLCK + || (table->s->tmp_table + && thd_sql_command(thd) == SQLCOM_LOCK_TABLES)) { /* If this is a SELECT, then it is in UPDATE TABLE ... - or SELECT ... FOR UPDATE */ + or SELECT ... FOR UPDATE + + For temporary tables which are locked for READ by LOCK TABLES + updates are still allowed by SQL-layer. In order to accomodate + for such a situation we always request X-lock for such table + at LOCK TABLES time. + */ prebuilt->select_lock_type = LOCK_X; prebuilt->stored_select_lock_type = LOCK_X; } No bundle (reason: useless for push emails).