Commit c3c53926 authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-26554: Races between INSERT on child and DDL on parent table

The SQL layer never acquires metadata locks (MDL) on the tables
that the tables that DML statement accesses is modifying.

However, the storage engine must access the parent table in order to
ensure that the child table will not refer to a non-existing record
in the parent table.

During certain DDL operations, the InnoDB table metadata (dict_table_t)
may be be freed and rebuilt. This would cause a race condition with
a concurrent INSERT that is attempting to report a FOREIGN KEY violation.

We work around the insufficient MDL during DML by acquiring exclusive
InnoDB table locks on all child tables during DDL. To avoid deadlocks,
we will follow the following order of acquisition:

1. tables whose REFERENCES clauses point to the current table
2. the current table that is being subjected to DDL
3. mysql.innodb_table_stats
4. mysql.innodb_index_stats
5. the InnoDB dictionary tables (SYS_TABLES and so on)
6. exclusive dict_sys.latch
parent 59fe6a8a
......@@ -408,8 +408,8 @@ INSERT INTO t1 VALUES (1,2);
CREATE TABLE x AS SELECT * FROM t1;
ERROR XAE07: XAER_RMFAIL: The command cannot be executed when global transaction is in the ACTIVE state
connect con1,localhost,root,,test;
SET foreign_key_checks= OFF, innodb_lock_wait_timeout= 1;
SET lock_wait_timeout=5;
SET foreign_key_checks= OFF, innodb_lock_wait_timeout= 0;
SET lock_wait_timeout=2;
ALTER TABLE t1 ADD FOREIGN KEY f (a) REFERENCES t1 (pk), LOCK=EXCLUSIVE;
ERROR HY000: Lock wait timeout exceeded; try restarting transaction
disconnect con1;
......@@ -491,7 +491,7 @@ BEGIN;
UPDATE users SET name = 'qux' WHERE id = 1;
connect con1,localhost,root;
connection con1;
SET innodb_lock_wait_timeout= 1;
SET innodb_lock_wait_timeout= 0;
DELETE FROM matchmaking_groups WHERE id = 10;
connection default;
COMMIT;
......@@ -531,9 +531,10 @@ connection con1;
BEGIN;
UPDATE t2 SET f = 11 WHERE id = 1;
connection default;
SET innodb_lock_wait_timeout= 1;
SET innodb_lock_wait_timeout= 0;
DELETE FROM t1 WHERE id = 1;
ERROR HY000: Lock wait timeout exceeded; try restarting transaction
SET innodb_lock_wait_timeout= 1;
connection con1;
COMMIT;
connection default;
......@@ -897,3 +898,30 @@ create or replace table t2 (pk int primary key, a varchar(4096) unique, foreign
ERROR HY000: Can't create table `test`.`t2` (errno: 150 "Foreign key constraint is incorrectly formed")
drop table t1;
# End of 10.5 tests
#
# MDEV-26554 Table-rebuilding DDL on parent table causes crash
# for INSERT into child table
#
CREATE TABLE parent(a INT PRIMARY KEY) ENGINE=InnoDB;
CREATE TABLE child(a INT PRIMARY KEY REFERENCES parent(a)) ENGINE=InnoDB;
connect con1, localhost, root,,;
BEGIN;
INSERT INTO child SET a=1;
ERROR 23000: Cannot add or update a child row: a foreign key constraint fails (`test`.`child`, CONSTRAINT `child_ibfk_1` FOREIGN KEY (`a`) REFERENCES `parent` (`a`))
connection default;
SET innodb_lock_wait_timeout=0, foreign_key_checks=0;
TRUNCATE TABLE parent;
ERROR HY000: Lock wait timeout exceeded; try restarting transaction
ALTER TABLE parent FORCE, ALGORITHM=COPY;
ERROR HY000: Lock wait timeout exceeded; try restarting transaction
ALTER TABLE parent FORCE, ALGORITHM=INPLACE;
ERROR HY000: Lock wait timeout exceeded; try restarting transaction
ALTER TABLE parent ADD COLUMN b INT, ALGORITHM=INSTANT;
ERROR HY000: Lock wait timeout exceeded; try restarting transaction
disconnect con1;
TRUNCATE TABLE parent;
ALTER TABLE parent FORCE, ALGORITHM=COPY;
ALTER TABLE parent FORCE, ALGORITHM=INPLACE;
ALTER TABLE parent ADD COLUMN b INT, ALGORITHM=INSTANT;
DROP TABLE child, parent;
# End of 10.6 tests
......@@ -75,7 +75,7 @@ DROP TABLE t1;
Warnings:
Warning 1932 Table 'test.t1' doesn't exist in engine
DROP TABLE t2,t3;
FOUND 5 /\[ERROR\] InnoDB: Table test/t1 in InnoDB data dictionary contains invalid flags\. SYS_TABLES\.TYPE=1 SYS_TABLES\.MIX_LEN=511\b/ in mysqld.1.err
FOUND 6 /\[ERROR\] InnoDB: Table test/t1 in InnoDB data dictionary contains invalid flags\. SYS_TABLES\.TYPE=1 SYS_TABLES\.MIX_LEN=511\b/ in mysqld.1.err
# restart
ib_buffer_pool
ib_logfile0
......
......@@ -43,16 +43,19 @@ SET DEBUG_SYNC='foreign_constraint_check_for_ins SIGNAL fk WAIT_FOR go';
INSERT INTO child SET a=5;
connection default;
SET DEBUG_SYNC='now WAIT_FOR fk';
SET foreign_key_checks=0;
SET foreign_key_checks=0, innodb_lock_wait_timeout=0;
TRUNCATE TABLE parent;
ERROR HY000: Lock wait timeout exceeded; try restarting transaction
SET DEBUG_SYNC='now SIGNAL go';
connection dml;
ERROR 23000: Cannot add or update a child row: a foreign key constraint fails (`test`.`child`, CONSTRAINT `child_ibfk_1` FOREIGN KEY (`a`) REFERENCES `parent` (`a`) ON UPDATE CASCADE)
SELECT * FROM parent;
a
3
5
SELECT * FROM child;
a
3
5
disconnect dml;
connection default;
SET DEBUG_SYNC = RESET;
......
......@@ -411,8 +411,8 @@ INSERT INTO t1 VALUES (1,2);
--error ER_XAER_RMFAIL
CREATE TABLE x AS SELECT * FROM t1;
--connect (con1,localhost,root,,test)
SET foreign_key_checks= OFF, innodb_lock_wait_timeout= 1;
SET lock_wait_timeout=5;
SET foreign_key_checks= OFF, innodb_lock_wait_timeout= 0;
SET lock_wait_timeout=2;
--error ER_LOCK_WAIT_TIMEOUT
ALTER TABLE t1 ADD FOREIGN KEY f (a) REFERENCES t1 (pk), LOCK=EXCLUSIVE;# Cleanup
--disconnect con1
......@@ -506,7 +506,7 @@ UPDATE users SET name = 'qux' WHERE id = 1;
connect (con1,localhost,root);
--connection con1
SET innodb_lock_wait_timeout= 1;
SET innodb_lock_wait_timeout= 0;
DELETE FROM matchmaking_groups WHERE id = 10;
--connection default
......@@ -544,9 +544,10 @@ BEGIN;
UPDATE t2 SET f = 11 WHERE id = 1;
--connection default
SET innodb_lock_wait_timeout= 1;
SET innodb_lock_wait_timeout= 0;
--error ER_LOCK_WAIT_TIMEOUT
DELETE FROM t1 WHERE id = 1;
SET innodb_lock_wait_timeout= 1;
--connection con1
COMMIT;
......@@ -902,4 +903,34 @@ drop table t1;
--echo # End of 10.5 tests
--echo #
--echo # MDEV-26554 Table-rebuilding DDL on parent table causes crash
--echo # for INSERT into child table
--echo #
CREATE TABLE parent(a INT PRIMARY KEY) ENGINE=InnoDB;
CREATE TABLE child(a INT PRIMARY KEY REFERENCES parent(a)) ENGINE=InnoDB;
connect (con1, localhost, root,,);
BEGIN;
--error ER_NO_REFERENCED_ROW_2
INSERT INTO child SET a=1;
connection default;
SET innodb_lock_wait_timeout=0, foreign_key_checks=0;
--error ER_LOCK_WAIT_TIMEOUT
TRUNCATE TABLE parent;
--error ER_LOCK_WAIT_TIMEOUT
ALTER TABLE parent FORCE, ALGORITHM=COPY;
--error ER_LOCK_WAIT_TIMEOUT
ALTER TABLE parent FORCE, ALGORITHM=INPLACE;
--error ER_LOCK_WAIT_TIMEOUT
ALTER TABLE parent ADD COLUMN b INT, ALGORITHM=INSTANT;
disconnect con1;
TRUNCATE TABLE parent;
ALTER TABLE parent FORCE, ALGORITHM=COPY;
ALTER TABLE parent FORCE, ALGORITHM=INPLACE;
ALTER TABLE parent ADD COLUMN b INT, ALGORITHM=INSTANT;
DROP TABLE child, parent;
--echo # End of 10.6 tests
--source include/wait_until_count_sessions.inc
......@@ -52,12 +52,12 @@ send INSERT INTO child SET a=5;
connection default;
SET DEBUG_SYNC='now WAIT_FOR fk';
SET foreign_key_checks=0;
SET foreign_key_checks=0, innodb_lock_wait_timeout=0;
--error ER_LOCK_WAIT_TIMEOUT
TRUNCATE TABLE parent;
SET DEBUG_SYNC='now SIGNAL go';
connection dml;
--error ER_NO_REFERENCED_ROW_2
reap;
SELECT * FROM parent;
SELECT * FROM child;
......
......@@ -13645,7 +13645,6 @@ static dberr_t innobase_rename_table(trx_t *trx, const char *from,
DEBUG_SYNC_C("innodb_rename_table_ready");
trx_start_if_not_started(trx, true);
ut_ad(trx->will_lock);
error = row_rename_table_for_mysql(norm_from, norm_to, trx, use_fk);
......@@ -13782,7 +13781,23 @@ int ha_innobase::truncate()
dict_table_t *table_stats = nullptr, *index_stats = nullptr;
MDL_ticket *mdl_table = nullptr, *mdl_index = nullptr;
dberr_t error = lock_table_for_trx(ib_table, trx, LOCK_X);
dberr_t error = DB_SUCCESS;
dict_sys.freeze(SRW_LOCK_CALL);
for (const dict_foreign_t* f : ib_table->referenced_set) {
if (dict_table_t* child = f->foreign_table) {
error = lock_table_for_trx(child, trx, LOCK_X);
if (error != DB_SUCCESS) {
break;
}
}
}
dict_sys.unfreeze();
if (error == DB_SUCCESS) {
error = lock_table_for_trx(ib_table, trx, LOCK_X);
}
const bool fts = error == DB_SUCCESS
&& ib_table->flags2 & (DICT_TF2_FTS_HAS_DOC_ID | DICT_TF2_FTS);
......@@ -13945,6 +13960,26 @@ ha_innobase::rename_table(
dberr_t error = DB_SUCCESS;
if (dict_table_t::is_temporary_name(norm_from)) {
/* There is no need to lock any FOREIGN KEY child tables. */
} else if (dict_table_t *table = dict_table_open_on_name(
norm_from, false, DICT_ERR_IGNORE_FK_NOKEY)) {
dict_sys.freeze(SRW_LOCK_CALL);
for (const dict_foreign_t* f : table->referenced_set) {
if (dict_table_t* child = f->foreign_table) {
error = lock_table_for_trx(child, trx, LOCK_X);
if (error != DB_SUCCESS) {
break;
}
}
}
dict_sys.unfreeze();
if (error == DB_SUCCESS) {
error = lock_table_for_trx(table, trx, LOCK_X);
}
table->release();
}
if (strcmp(norm_from, TABLE_STATS_NAME)
&& strcmp(norm_from, INDEX_STATS_NAME)
&& strcmp(norm_to, TABLE_STATS_NAME)
......@@ -13966,7 +14001,7 @@ ha_innobase::rename_table(
dict_sys.unfreeze();
}
if (table_stats && index_stats
if (error == DB_SUCCESS && table_stats && index_stats
&& !strcmp(table_stats->name.m_name, TABLE_STATS_NAME)
&& !strcmp(index_stats->name.m_name, INDEX_STATS_NAME) &&
!(error = lock_table_for_trx(table_stats, trx, LOCK_X))) {
......
......@@ -10847,12 +10847,24 @@ ha_innobase::commit_inplace_alter_table(
for (inplace_alter_handler_ctx** pctx = ctx_array; *pctx; pctx++) {
auto ctx = static_cast<ha_innobase_inplace_ctx*>(*pctx);
dberr_t error = DB_SUCCESS;
if (new_clustered && ctx->old_table->fts) {
ut_ad(!ctx->old_table->fts->add_wq);
fts_optimize_remove_table(ctx->old_table);
}
dict_sys.freeze(SRW_LOCK_CALL);
for (auto f : ctx->old_table->referenced_set) {
if (dict_table_t* child = f->foreign_table) {
error = lock_table_for_trx(child, trx, LOCK_X);
if (error != DB_SUCCESS) {
break;
}
}
}
dict_sys.unfreeze();
if (ctx->new_table->fts) {
ut_ad(!ctx->new_table->fts->add_wq);
fts_optimize_remove_table(ctx->new_table);
......@@ -10863,9 +10875,12 @@ ha_innobase::commit_inplace_alter_table(
transaction is holding locks on the table while we
change the table definition. Any recovered incomplete
transactions would be holding InnoDB locks only, not MDL. */
if (error == DB_SUCCESS) {
error = lock_table_for_trx(ctx->new_table, trx,
LOCK_X);
}
if (dberr_t error = lock_table_for_trx(ctx->new_table, trx,
LOCK_X)) {
if (error != DB_SUCCESS) {
lock_fail:
my_error_innodb(
error, table_share->table_name.str, 0);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment