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#6255 | marko.makela | 25 May |
