Skip to content

Commit 9180e86

Browse files
vuvovavuvova
committed
MDEV-16465 Invalid (old?) table or database name or hang in ha_innobase::delete_table and log semaphore wait upon concurrent DDL with foreign keys
ALTER TABLE locks the table with TL_READ_NO_INSERT, to prevent the source table modifications while it's being copied. But there's an indirect way of modifying a table, via cascade FK actions. After previous commits, an attempt to modify an FK parent table will cause FK children to be prelocked, so the table-being-altered cannot be modified by a cascade FK action, because ALTER holds a lock and prelocking will wait. But if a new FK is being added by this very ALTER, then the target table is not locked yet (it's a temporary table). So, we have to lock FK parents explicitly.
1 parent e81f101 commit 9180e86

File tree

6 files changed

+132
-9
lines changed

6 files changed

+132
-9
lines changed

‎mysql-test/suite/innodb/r/foreign-keys.result‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,39 @@ c d
5151
6 30
5252
drop table t2, t1;
5353
drop user foo;
54+
create table t1 (f1 int primary key) engine=innodb;
55+
create table t2 (f2 int primary key) engine=innodb;
56+
create table t3 (f3 int primary key, foreign key (f3) references t2(f2)) engine=innodb;
57+
insert into t1 values (1),(2),(3),(4),(5);
58+
insert into t2 values (1),(2),(3),(4),(5);
59+
insert into t3 values (1),(2),(3),(4),(5);
60+
connect con1,localhost,root;
61+
set debug_sync='alter_table_before_rename_result_table signal g1 wait_for g2';
62+
alter table t2 add constraint foreign key (f2) references t1(f1) on delete cascade on update cascade;
63+
connection default;
64+
set debug_sync='before_execute_sql_command wait_for g1';
65+
update t1 set f1 = f1 + 100000 limit 2;
66+
connect con2,localhost,root;
67+
kill query UPDATE;
68+
disconnect con2;
69+
connection default;
70+
ERROR 70100: Query execution was interrupted
71+
set debug_sync='now signal g2';
72+
connection con1;
73+
show create table t2;
74+
Table Create Table
75+
t2 CREATE TABLE `t2` (
76+
`f2` int(11) NOT NULL,
77+
PRIMARY KEY (`f2`),
78+
CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`f2`) REFERENCES `t1` (`f1`) ON DELETE CASCADE ON UPDATE CASCADE
79+
) ENGINE=InnoDB DEFAULT CHARSET=latin1
80+
disconnect con1;
81+
connection default;
82+
select * from t2 where f2 not in (select f1 from t1);
83+
f2
84+
select * from t3 where f3 not in (select f2 from t2);
85+
f3
86+
drop table t3;
87+
drop table t2;
88+
drop table t1;
89+
set debug_sync='reset';

‎mysql-test/suite/innodb/t/foreign-keys.test‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
--source include/have_innodb.inc
22
--source include/have_debug.inc
3+
--source include/have_debug_sync.inc
34

45
--enable_connect_log
56

@@ -72,3 +73,41 @@ connection default;
7273
select * from t2;
7374
drop table t2, t1;
7475
drop user foo;
76+
77+
#
78+
# MDEV-16465 Invalid (old?) table or database name or hang in ha_innobase::delete_table and log semaphore wait upon concurrent DDL with foreign keys
79+
#
80+
create table t1 (f1 int primary key) engine=innodb;
81+
create table t2 (f2 int primary key) engine=innodb;
82+
create table t3 (f3 int primary key, foreign key (f3) references t2(f2)) engine=innodb;
83+
insert into t1 values (1),(2),(3),(4),(5);
84+
insert into t2 values (1),(2),(3),(4),(5);
85+
insert into t3 values (1),(2),(3),(4),(5);
86+
connect con1,localhost,root;
87+
set debug_sync='alter_table_before_rename_result_table signal g1 wait_for g2';
88+
send alter table t2 add constraint foreign key (f2) references t1(f1) on delete cascade on update cascade;
89+
connection default;
90+
let $conn=`select connection_id()`;
91+
set debug_sync='before_execute_sql_command wait_for g1';
92+
send update t1 set f1 = f1 + 100000 limit 2;
93+
connect con2,localhost,root;
94+
let $wait_condition= select 1 from information_schema.processlist where state='Waiting for table metadata lock' and info like 'update t1 %';
95+
source include/wait_condition.inc;
96+
--replace_result $conn UPDATE
97+
eval kill query $conn;
98+
disconnect con2;
99+
connection default;
100+
error ER_QUERY_INTERRUPTED;
101+
reap;
102+
set debug_sync='now signal g2';
103+
connection con1;
104+
reap;
105+
show create table t2;
106+
disconnect con1;
107+
connection default;
108+
select * from t2 where f2 not in (select f1 from t1);
109+
select * from t3 where f3 not in (select f2 from t2);
110+
drop table t3;
111+
drop table t2;
112+
drop table t1;
113+
set debug_sync='reset';

‎sql/sql_base.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4806,8 +4806,8 @@ handle_routine(THD *thd, Query_tables_list *prelocking_ctx,
48064806
@note this can be changed to use a hash, instead of scanning the linked
48074807
list, if the performance of this function will ever become an issue
48084808
*/
4809-
static bool table_already_fk_prelocked(TABLE_LIST *tl, LEX_STRING *db,
4810-
LEX_STRING *table, thr_lock_type lock_type)
4809+
bool table_already_fk_prelocked(TABLE_LIST *tl, LEX_STRING *db,
4810+
LEX_STRING *table, thr_lock_type lock_type)
48114811
{
48124812
for (; tl; tl= tl->next_global )
48134813
{

‎sql/sql_base.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,8 @@ my_bool mysql_rm_tmp_tables(void);
148148
bool rm_temporary_table(handlerton *base, const char *path);
149149
void close_tables_for_reopen(THD *thd, TABLE_LIST **tables,
150150
const MDL_savepoint &start_of_statement_svp);
151+
bool table_already_fk_prelocked(TABLE_LIST *tl, LEX_STRING *db,
152+
LEX_STRING *table, thr_lock_type lock_type);
151153
TABLE_LIST *find_table_in_list(TABLE_LIST *table,
152154
TABLE_LIST *TABLE_LIST::*link,
153155
const char *db_name,

‎sql/sql_table.cc‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9057,8 +9057,10 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
90579057
alter_ctx.tmp_name, strlen(alter_ctx.tmp_name),
90589058
alter_ctx.tmp_name, TL_READ_NO_INSERT);
90599059
/* Table is in thd->temporary_tables */
9060-
(void) open_temporary_table(thd, &tbl);
9060+
if (open_temporary_table(thd, &tbl))
9061+
goto err_new_table_cleanup;
90619062
new_table= tbl.table;
9063+
DBUG_ASSERT(new_table);
90629064
}
90639065
else
90649066
{
@@ -9067,9 +9069,48 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
90679069
new_table= open_table_uncached(thd, new_db_type, alter_ctx.get_tmp_path(),
90689070
alter_ctx.new_db, alter_ctx.tmp_name,
90699071
true, true);
9072+
if (!new_table)
9073+
goto err_new_table_cleanup;
9074+
9075+
/*
9076+
Normally, an attempt to modify an FK parent table will cause
9077+
FK children to be prelocked, so the table-being-altered cannot
9078+
be modified by a cascade FK action, because ALTER holds a lock
9079+
and prelocking will wait.
9080+
9081+
But if a new FK is being added by this very ALTER, then the target
9082+
table is not locked yet (it's a temporary table). So, we have to
9083+
lock FK parents explicitly.
9084+
*/
9085+
if (alter_info->flags & Alter_info::ADD_FOREIGN_KEY)
9086+
{
9087+
List <FOREIGN_KEY_INFO> fk_list;
9088+
List_iterator<FOREIGN_KEY_INFO> fk_list_it(fk_list);
9089+
FOREIGN_KEY_INFO *fk;
9090+
9091+
/* tables_opened can be > 1 only for MERGE tables */
9092+
DBUG_ASSERT(tables_opened == 1);
9093+
DBUG_ASSERT(&table_list->next_global == thd->lex->query_tables_last);
9094+
9095+
new_table->file->get_foreign_key_list(thd, &fk_list);
9096+
while ((fk= fk_list_it++))
9097+
{
9098+
if (table_already_fk_prelocked(table_list, fk->referenced_db,
9099+
fk->referenced_table, TL_READ_NO_INSERT))
9100+
continue;
9101+
9102+
TABLE_LIST *tl= (TABLE_LIST *) thd->alloc(sizeof(TABLE_LIST));
9103+
tl->init_one_table_for_prelocking(fk->referenced_db->str, fk->referenced_db->length,
9104+
fk->referenced_table->str, fk->referenced_table->length,
9105+
NULL, TL_READ_NO_INSERT, false, NULL, 0,
9106+
&thd->lex->query_tables_last);
9107+
}
9108+
9109+
if (open_tables(thd, &table_list->next_global, &tables_opened, 0,
9110+
&alter_prelocking_strategy))
9111+
goto err_new_table_cleanup;
9112+
}
90709113
}
9071-
if (!new_table)
9072-
goto err_new_table_cleanup;
90739114
/*
90749115
Note: In case of MERGE table, we do not attach children. We do not
90759116
copy data for MERGE tables. Only the children have data.

‎sql/table.h‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1695,17 +1695,22 @@ struct TABLE_LIST
16951695
const char *alias_arg,
16961696
enum thr_lock_type lock_type_arg)
16971697
{
1698+
enum enum_mdl_type mdl_type;
1699+
if (lock_type_arg >= TL_WRITE_ALLOW_WRITE)
1700+
mdl_type= MDL_SHARED_WRITE;
1701+
else if (lock_type_arg == TL_READ_NO_INSERT)
1702+
mdl_type= MDL_SHARED_NO_WRITE;
1703+
else
1704+
mdl_type= MDL_SHARED_READ;
1705+
16981706
bzero((char*) this, sizeof(*this));
16991707
db= (char*) db_name_arg;
17001708
db_length= db_length_arg;
17011709
table_name= (char*) table_name_arg;
17021710
table_name_length= table_name_length_arg;
17031711
alias= (char*) (alias_arg ? alias_arg : table_name_arg);
17041712
lock_type= lock_type_arg;
1705-
mdl_request.init(MDL_key::TABLE, db, table_name,
1706-
(lock_type >= TL_WRITE_ALLOW_WRITE) ?
1707-
MDL_SHARED_WRITE : MDL_SHARED_READ,
1708-
MDL_TRANSACTION);
1713+
mdl_request.init(MDL_key::TABLE, db, table_name, mdl_type, MDL_TRANSACTION);
17091714
}
17101715

17111716
inline void init_one_table_for_prelocking(const char *db_name_arg,

0 commit comments

Comments
 (0)