List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:November 25 2010 2:50pm
Subject:bzr commit into mysql-trunk-bugfixing branch (jon.hauglid:3395) Bug#42230
View as plain text  
#At file:///export/home/x/mysql-trunk-bugfixing-altertable/ based on revid:magnus.blaudd@stripped

 3395 Jon Olav Hauglid	2010-11-25
      Bug #42230 during add index, cannot do queries on storage engines
                 that implement add_index
      
      The problem was that ALTER TABLE blocked reads on an InnoDB table
      while adding a secondary index, even if this was not needed. It is
      only needed for the final step where the .frm file is updated.
      
      The reason queries were blocked, was that ALTER TABLE upgraded the
      metadata lock from MDL_SHARED_NO_WRITE (which blocks writes) to
      MDL_EXCLUSIVE (which blocks all accesses) before index creation.
      
      The way the server handles index creation, is that storage engines
      publish their capabilities to the server and the server determines
      which of the following three ways this can be handled: 1) build a
      new version of the table; 2) change the existing table but with
      exclusive metadata lock; 3) change the existing table but without
      metadata lock upgrade.
      
      For InnoDB and secondary index creation, option 3) should have been
      selected. However this failed for two reasons. First, InnoDB did
      not publish this capability properly. This patch fixes the problem
      by adding HA_ONLINE_ADD_INDEX to InnoDB's list of alter table flags.
      (The already published HA_ONLINE_ADD_INDEX_NO_WRITES flag only
      indicates that InnoDB support option 2).
      
      Second, the ALTER TABLE code failed to made proper use of the
      information supplied by the storage engine. A variable
      need_lock_for_indexes was set accordingly, but was not later used.
      This patch fixes this problem by only doing metadata lock upgrade
      before index creation/deletion if this variable has been set.
      
      Test case added to innodb_mysql_sync.test.

    modified:
      mysql-test/r/innodb_mysql_sync.result
      mysql-test/t/innodb_mysql_sync.test
      sql/sql_table.cc
      storage/innobase/handler/ha_innodb.cc
      storage/innobase/handler/handler0alter.cc
=== modified file 'mysql-test/r/innodb_mysql_sync.result'
--- a/mysql-test/r/innodb_mysql_sync.result	2010-06-25 07:07:18 +0000
+++ b/mysql-test/r/innodb_mysql_sync.result	2010-11-25 14:50:08 +0000
@@ -66,3 +66,23 @@ SELECT ((@id := id) - id) FROM t2;
 KILL @id;
 SET DEBUG_SYNC= "now SIGNAL killed";
 DROP TABLE t1, t2;
+SET DEBUG_SYNC= "RESET";
+#
+# Bug#42230 during add index, cannot do queries on storage engines
+#           that implement add_index
+#
+DROP DATABASE IF EXISTS db1;
+CREATE DATABASE db1;
+CREATE TABLE db1.t1(id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, value INT) engine=innodb;
+INSERT INTO db1.t1(value) VALUES (1), (2);
+SET DEBUG_SYNC= "alter_table_manage_keys SIGNAL manage WAIT_FOR query";
+ALTER TABLE db1.t1 ADD INDEX(value);
+SET DEBUG_SYNC= "now WAIT_FOR manage";
+USE db1;
+SELECT * FROM t1;
+id	value
+1	1
+2	2
+SET DEBUG_SYNC= "now SIGNAL query";
+DROP DATABASE db1;
+SET DEBUG_SYNC= "RESET";

=== modified file 'mysql-test/t/innodb_mysql_sync.test'
--- a/mysql-test/t/innodb_mysql_sync.test	2010-06-25 07:07:18 +0000
+++ b/mysql-test/t/innodb_mysql_sync.test	2010-11-25 14:50:08 +0000
@@ -104,6 +104,37 @@ SELECT ((@id := id) - id) FROM t2;
 KILL @id;
 SET DEBUG_SYNC= "now SIGNAL killed";
 DROP TABLE t1, t2;
+disconnect con1;
+SET DEBUG_SYNC= "RESET";
+
+
+--echo #
+--echo # Bug#42230 during add index, cannot do queries on storage engines
+--echo #           that implement add_index
+--echo #
+
+--disable_warnings
+DROP DATABASE IF EXISTS db1;
+--enable_warnings
+
+CREATE DATABASE db1;
+CREATE TABLE db1.t1(id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, value INT) engine=innodb;
+INSERT INTO db1.t1(value) VALUES (1), (2);
+SET DEBUG_SYNC= "alter_table_manage_keys SIGNAL manage WAIT_FOR query";
+--send ALTER TABLE db1.t1 ADD INDEX(value)
+
+connect(con1,localhost,root);
+SET DEBUG_SYNC= "now WAIT_FOR manage";
+# Neither of these two statements should be blocked
+USE db1;
+SELECT * FROM t1;
+SET DEBUG_SYNC= "now SIGNAL query";
+
+connection default;
+--reap
+DROP DATABASE db1;
+disconnect con1;
+SET DEBUG_SYNC= "RESET";
 
 
 # Check that all connections opened by test cases in this file are really

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2010-11-19 09:15:30 +0000
+++ b/sql/sql_table.cc	2010-11-25 14:50:08 +0000
@@ -6606,10 +6606,11 @@ bool mysql_alter_table(THD *thd,char *ne
   }
   else
   {
-    if (!table->s->tmp_table &&
+    if (!table->s->tmp_table && need_lock_for_indexes &&
         wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))
       goto err_new_table_cleanup;
     thd_proc_info(thd, "manage keys");
+    DEBUG_SYNC(thd, "alter_table_manage_keys");
     alter_table_manage_keys(table, table->file->indexes_are_disabled(),
                             alter_info->keys_onoff);
     error= trans_commit_stmt(thd);

=== modified file 'storage/innobase/handler/ha_innodb.cc'
--- a/storage/innobase/handler/ha_innodb.cc	2010-11-17 11:30:54 +0000
+++ b/storage/innobase/handler/ha_innodb.cc	2010-11-25 14:50:08 +0000
@@ -2758,6 +2758,7 @@ innobase_alter_table_flags(
 	uint	flags)
 {
 	return(HA_ONLINE_ADD_INDEX_NO_WRITES
+		| HA_ONLINE_ADD_INDEX
 		| HA_ONLINE_DROP_INDEX_NO_WRITES
 		| HA_ONLINE_ADD_UNIQUE_INDEX_NO_WRITES
 		| HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES

=== modified file 'storage/innobase/handler/handler0alter.cc'
--- a/storage/innobase/handler/handler0alter.cc	2010-11-11 06:08:37 +0000
+++ b/storage/innobase/handler/handler0alter.cc	2010-11-25 14:50:08 +0000
@@ -1009,7 +1009,7 @@ convert_error:
 						    user_thd);
 	}
 
-	ut_a(innodb_table->n_ref_count == 1);
+	ut_a(!new_primary || innodb_table->n_ref_count == 1);
 
 	mem_heap_free(heap);
 	trx_commit_for_mysql(trx);


Attachment: [text/bzr-bundle] bzr/jon.hauglid@oracle.com-20101125145008-kucw9wlahxn21grx.bundle
Thread
bzr commit into mysql-trunk-bugfixing branch (jon.hauglid:3395) Bug#42230Jon Olav Hauglid25 Nov