List:Commits« Previous MessageNext Message »
From:marko.makela Date:May 25 2012 11:43am
Subject:bzr push into mysql-trunk-wl6255 branch (marko.makela:3864 to 3865)
Bug#14117641 WL#6255
View as plain text  
 3865 Marko Mäkelä	2012-05-25
      Bug#14117641 (possible fix) and a WL#6255 optimization
      
      Allow the non-online InnoDB in-place DDL operations with LOCK=SHARED.
      
      prepare_inplace_alter_table_dict(): When creating indexes and
      an online operation has not been requested, acquire S-lock on the table.
      Previously, we acquired an X-lock, and only when the table was being rebuilt.
      Failure to lock the table might explain Bug#14117641 if there was a
      foreign key CASCADE or SET NULL constraint referencing the table.

    modified:
      mysql-test/r/innodb_mysql_sync.result
      mysql-test/suite/innodb/r/innodb-alter.result
      mysql-test/suite/innodb/t/innodb-alter.test
      mysql-test/t/innodb_mysql_sync.test
      storage/innobase/handler/handler0alter.cc
 3864 Marko Mäkelä	2012-05-24
      row_undo_mod_remove_clust_low(): Enable a debug assertion and
      add comments on DB_TRX_ID and the delete-mark flag.

    modified:
      storage/innobase/row/row0umod.cc
=== modified file 'mysql-test/r/innodb_mysql_sync.result'
--- a/mysql-test/r/innodb_mysql_sync.result	revid:marko.makela@stripped
+++ b/mysql-test/r/innodb_mysql_sync.result	revid:marko.makela@oracle.com-20120525113924-23jckoyd4n0wkpu8
@@ -115,7 +115,7 @@ SET DEBUG_SYNC= "now SIGNAL query";
 # Connection default
 # Reaping: ALTER TABLE db1.t1 ADD INDEX(value)
 DROP DATABASE db1;
-# Test 2: Primary index (implicit), should block reads.
+# Test 2: Primary index (implicit), should block writes.
 CREATE TABLE t1(a INT NOT NULL, b INT NOT NULL) engine=innodb;
 SET DEBUG_SYNC= "alter_table_inplace_after_lock_downgrade SIGNAL manage WAIT_FOR query";
 # Sending:
@@ -123,17 +123,18 @@ ALTER TABLE t1 ADD UNIQUE INDEX(a);
 # Connection con1
 SET DEBUG_SYNC= "now WAIT_FOR manage";
 USE test;
-# Sending:
 SELECT * FROM t1;
+a	b
+# Sending:
+UPDATE t1 SET a=NULL;
 # Connection con2
 # Waiting for SELECT to be blocked by the metadata lock on t1
 SET DEBUG_SYNC= "now SIGNAL query";
 # Connection default
 # Reaping: ALTER TABLE t1 ADD UNIQUE INDEX(a)
 # Connection con1
-# Reaping: SELECT * FROM t1
-a	b
-# Test 3: Primary index (explicit), should block reads.
+# Reaping: UPDATE t1 SET a=NULL
+# Test 3: Primary index (explicit), should block writes.
 # Connection default
 ALTER TABLE t1 DROP INDEX a;
 SET DEBUG_SYNC= "alter_table_inplace_after_lock_downgrade SIGNAL manage WAIT_FOR query";
@@ -141,16 +142,17 @@ SET DEBUG_SYNC= "alter_table_inplace_aft
 ALTER TABLE t1 ADD PRIMARY KEY (a);
 # Connection con1
 SET DEBUG_SYNC= "now WAIT_FOR manage";
-# Sending:
 SELECT * FROM t1;
+a	b
+# Sending:
+UPDATE t1 SET a=NULL;
 # Connection con2
 # Waiting for SELECT to be blocked by the metadata lock on t1
 SET DEBUG_SYNC= "now SIGNAL query";
 # Connection default
 # Reaping: ALTER TABLE t1 ADD PRIMARY KEY (a)
 # Connection con1
-# Reaping: SELECT * FROM t1
-a	b
+# Reaping: UPDATE t1 SET a=NULL
 # Test 4: Secondary unique index, should not block reads.
 # Connection default
 SET DEBUG_SYNC= "alter_table_inplace_after_lock_downgrade SIGNAL manage WAIT_FOR query";

=== modified file 'mysql-test/suite/innodb/r/innodb-alter.result'
--- a/mysql-test/suite/innodb/r/innodb-alter.result	revid:marko.makela@stripped
+++ b/mysql-test/suite/innodb/r/innodb-alter.result	revid:marko.makela@stripped
@@ -414,13 +414,13 @@ ID	FOR_COL_NAME	REF_COL_NAME	POS
 CREATE TABLE t1o LIKE t1;
 ALTER TABLE t1 ADD FULLTEXT INDEX (ct),
 CHANGE c1 pk INT, ALTER c2 SET DEFAULT 42, RENAME TO tt,
-ALGORITHM=INPLACE, LOCK=SHARED;
+ALGORITHM=INPLACE, LOCK=NONE;
 ERROR 42000: This version of MySQL doesn't yet support 'ALTER TABLE t1 ADD FULLTEXT INDEX (ct),
 CHANGE c1 pk INT, ALTER c2 SET DEFAULT 42, RENAME TO tt,
-ALGORITHM=INPLACE, LOCK=SHARED'
+ALGORITHM=INPLACE, LOCK=NONE'
 ALTER TABLE t1 ADD FULLTEXT INDEX (ct),
 CHANGE c1 pk INT, ALTER c2 SET DEFAULT 42, RENAME TO tt,
-ALGORITHM=INPLACE, LOCK=EXCLUSIVE;
+ALGORITHM=INPLACE, LOCK=SHARED;
 Warnings:
 Warning	124	InnoDB rebuilding table to add column FTS_DOC_ID
 SELECT i.NAME,i.POS,i.MTYPE,i.PRTYPE,i.LEN

=== modified file 'mysql-test/suite/innodb/t/innodb-alter.test'
--- a/mysql-test/suite/innodb/t/innodb-alter.test	revid:marko.makela@strippedq0fuzwiksj4yx
+++ b/mysql-test/suite/innodb/t/innodb-alter.test	revid:marko.makela@strippedoyd4n0wkpu8
@@ -180,16 +180,16 @@ ALTER TABLE t1 DROP INDEX c2, CHANGE c3
 CREATE TABLE t1o LIKE t1;
 
 # This will implicitly add a DOC_ID column, copying the table while
-# holding an exclusive lock. The LOCK=SHARED should thus fail.
+# holding a lock. The LOCK=NONE should thus fail.
 --error ER_NOT_SUPPORTED_YET
 ALTER TABLE t1 ADD FULLTEXT INDEX (ct),
 CHANGE c1 pk INT, ALTER c2 SET DEFAULT 42, RENAME TO tt,
-ALGORITHM=INPLACE, LOCK=SHARED;
+ALGORITHM=INPLACE, LOCK=NONE;
 
-# Retry with LOCK=EXCLUSIVE.
+# Retry with LOCK=SHARED.
 ALTER TABLE t1 ADD FULLTEXT INDEX (ct),
 CHANGE c1 pk INT, ALTER c2 SET DEFAULT 42, RENAME TO tt,
-ALGORITHM=INPLACE, LOCK=EXCLUSIVE;
+ALGORITHM=INPLACE, LOCK=SHARED;
 
 # The output should be empty, because index->id was reassigned.
 -- source suite/innodb/include/innodb_dict.inc

=== modified file 'mysql-test/t/innodb_mysql_sync.test'
--- a/mysql-test/t/innodb_mysql_sync.test	revid:marko.makela@strippedq0fuzwiksj4yx
+++ b/mysql-test/t/innodb_mysql_sync.test	revid:marko.makela@strippedpu8
@@ -186,7 +186,7 @@ connection default;
 --reap
 DROP DATABASE db1;
 
---echo # Test 2: Primary index (implicit), should block reads.
+--echo # Test 2: Primary index (implicit), should block writes.
 
 CREATE TABLE t1(a INT NOT NULL, b INT NOT NULL) engine=innodb;
 SET DEBUG_SYNC= "alter_table_inplace_after_lock_downgrade SIGNAL manage WAIT_FOR query";
@@ -197,15 +197,16 @@ SET DEBUG_SYNC= "alter_table_inplace_aft
 connection con1;
 SET DEBUG_SYNC= "now WAIT_FOR manage";
 USE test;
+SELECT * FROM t1;
 --echo # Sending:
---send SELECT * FROM t1
+--send UPDATE t1 SET a=NULL
 
 --echo # Connection con2
 connection con2;
 --echo # Waiting for SELECT to be blocked by the metadata lock on t1
 let $wait_condition= SELECT COUNT(*)= 1 FROM information_schema.processlist
   WHERE state= 'Waiting for table metadata lock'
-  AND info='SELECT * FROM t1';
+  AND info='UPDATE t1 SET a=NULL';
 --source include/wait_condition.inc
 SET DEBUG_SYNC= "now SIGNAL query";
 
@@ -216,10 +217,10 @@ connection default;
 
 --echo # Connection con1
 connection con1;
---echo # Reaping: SELECT * FROM t1
+--echo # Reaping: UPDATE t1 SET a=NULL
 --reap
 
---echo # Test 3: Primary index (explicit), should block reads.
+--echo # Test 3: Primary index (explicit), should block writes.
 
 --echo # Connection default
 connection default;
@@ -231,15 +232,16 @@ SET DEBUG_SYNC= "alter_table_inplace_aft
 --echo # Connection con1
 connection con1;
 SET DEBUG_SYNC= "now WAIT_FOR manage";
+SELECT * FROM t1;
 --echo # Sending:
---send SELECT * FROM t1
+--send UPDATE t1 SET a=NULL
 
 --echo # Connection con2
 connection con2;
 --echo # Waiting for SELECT to be blocked by the metadata lock on t1
 let $wait_condition= SELECT COUNT(*)= 1 FROM information_schema.processlist
   WHERE state= 'Waiting for table metadata lock'
-  AND info='SELECT * FROM t1';
+  AND info='UPDATE t1 SET a=NULL';
 --source include/wait_condition.inc
 SET DEBUG_SYNC= "now SIGNAL query";
 
@@ -250,7 +252,7 @@ connection default;
 
 --echo # Connection con1
 connection con1;
---echo # Reaping: SELECT * FROM t1
+--echo # Reaping: UPDATE t1 SET a=NULL
 --reap
 
 --echo # Test 4: Secondary unique index, should not block reads.

=== modified file 'storage/innobase/handler/handler0alter.cc'
--- a/storage/innobase/handler/handler0alter.cc	revid:marko.makela@stripped4123850-ycrq0fuzwiksj4yx
+++ b/storage/innobase/handler/handler0alter.cc	revid:marko.makela@stripped924-23jckoyd4n0wkpu8
@@ -183,7 +183,7 @@ innobase_need_rebuild(
 by ALTER TABLE and holding data used during in-place alter.
 
 @retval HA_ALTER_INPLACE_NOT_SUPPORTED	Not supported
-@retval HA_ALTER_INPLACE_EXCLUSIVE_LOCK	Supported, but requires X-lock
+@retval HA_ALTER_INPLACE_SHARED_LOCK	Supported, but requires S-lock
 @retval HA_ALTER_INPLACE_NO_LOCK Supported
 @retval HA_ALTER_INPLACE_NO_LOCK_AFTER_PREPARE	Supported, prepare phase
 (any transactions that have modified the table must commit or roll back
@@ -277,16 +277,16 @@ ha_innobase::check_if_supported_inplace_
 
 	prebuilt->trx->will_lock++;
 
-	/* Rebuilding the clustered index requires an exclusive lock on the
+	/* Rebuilding the clustered index requires a shared lock on the
 	table during the whole copying operation. */
 	if (innobase_need_rebuild(ha_alter_info)) {
-		DBUG_RETURN(HA_ALTER_INPLACE_EXCLUSIVE_LOCK);
+		DBUG_RETURN(HA_ALTER_INPLACE_SHARED_LOCK);
 	}
 
 	if (ha_alter_info->handler_flags
 	    & (Alter_inplace_info::ADD_INDEX
 	       | Alter_inplace_info::ADD_UNIQUE_INDEX)) {
-		/* Building a full-text index requires an exclusive lock. */
+		/* Building a full-text index requires a shared lock. */
 
 		for (uint i = 0; i < ha_alter_info->index_add_count; i++) {
 			const KEY* key =
@@ -298,7 +298,7 @@ ha_innobase::check_if_supported_inplace_
 						  | HA_PACK_KEY
 						  | HA_GENERATED_KEY
 						  | HA_BINARY_PACK_KEY)));
-				DBUG_RETURN(HA_ALTER_INPLACE_EXCLUSIVE_LOCK);
+				DBUG_RETURN(HA_ALTER_INPLACE_SHARED_LOCK);
 			}
 		}
 	}
@@ -1443,8 +1443,8 @@ prepare_inplace_alter_table_dict(
 	DBUG_ASSERT(!add_fts_doc_id || new_clustered);
 
 	/* A primary key index cannot be created online. The table
-	should be exclusively locked in this case. It should also
-	be exclusively locked when a full-text index is being created. */
+	should be locked in this case. It should also be locked when a
+	full-text index is being created. */
 	DBUG_ASSERT(!!new_clustered
 		    == ((ha_alter_info->handler_flags
 			 & INNOBASE_INPLACE_REBUILD)
@@ -1461,13 +1461,20 @@ prepare_inplace_alter_table_dict(
 	the data dictionary will be locked in crash recovery. */
 	trx_set_dict_operation(trx, TRX_DICT_OP_INDEX);
 
-	const bool exclusive = ha_alter_info->alter_info->requested_lock
-		== Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE;
+	const bool locked = ha_alter_info->alter_info->requested_lock
+		== Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE
+		|| ha_alter_info->alter_info->requested_lock
+		== Alter_info::ALTER_TABLE_LOCK_SHARED;
+
+	/* For now, rebuilding the clustered index requires a lock. */
+	ut_ad(!new_clustered || locked
+	      || ha_alter_info->alter_info->requested_lock
+	      == Alter_info::ALTER_TABLE_LOCK_DEFAULT);
 
 	/* Acquire a lock on the table before creating any indexes. */
-	if (new_clustered) {
+	if (new_clustered || locked) {
 		error = row_merge_lock_table(
-			user_trx, indexed_table, LOCK_X);
+			user_trx, indexed_table, LOCK_S);
 
 		if (error != DB_SUCCESS) {
 
@@ -1685,9 +1692,9 @@ col_fail:
 
 		/* If only online ALTER TABLE operations have been
 		requested, allocate a modification log. If the table
-		will be exclusively locked anyway, the modification
-		log is unnecessary. */
-		if (!exclusive && !num_fts_index
+		will be locked anyway, the modification log is
+		unnecessary. */
+		if (!locked && !num_fts_index
 		    && !innobase_need_rebuild(ha_alter_info)
 		    && !user_table->ibd_file_missing
 		    && !user_table->tablespace_discarded) {
@@ -1779,11 +1786,11 @@ op_ok:
 	ut_a(trx->lock.n_active_thrs == 0);
 
 	if (new_clustered) {
-		/* A clustered index is to be built.  Acquire an exclusive
+		/* A clustered index is to be built.  Acquire a shared
 		table lock also on the table that is being created. */
 		DBUG_ASSERT(indexed_table != user_table);
 
-		error = row_merge_lock_table(user_trx, indexed_table, LOCK_X);
+		error = row_merge_lock_table(user_trx, indexed_table, LOCK_S);
 	}
 
 error_handling:
@@ -1802,7 +1809,7 @@ error_handling:
 			add_index, add_key_nums, n_add_index,
 			drop_index, n_drop_index,
 			drop_foreign, n_drop_foreign,
-			!exclusive && !new_clustered && !num_fts_index,
+			!locked && !new_clustered && !num_fts_index,
 			heap, trx, indexed_table);
 		DBUG_RETURN(false);
 	case DB_TABLESPACE_ALREADY_EXISTS:
@@ -2285,14 +2292,16 @@ index_needed:
 
 	if (!(ha_alter_info->handler_flags & INNOBASE_INPLACE_CREATE)) {
 		if (heap) {
-			const bool exclusive
+			const bool locked
 				= ha_alter_info->alter_info->requested_lock
-				== Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE;
+				== Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE
+				|| ha_alter_info->alter_info->requested_lock
+				== Alter_info::ALTER_TABLE_LOCK_SHARED;
 			ha_alter_info->handler_ctx
 				= new ha_innobase_inplace_ctx(
 					NULL, NULL, 0,
 					drop_index, n_drop_index,
-					drop_fk, n_drop_fk, !exclusive,
+					drop_fk, n_drop_fk, !locked,
 					heap, NULL, indexed_table);
 		}
 

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk-wl6255 branch (marko.makela:3864 to 3865)Bug#14117641 WL#6255marko.makela25 May