Skip to content

Commit 00a2596

Browse files
sjaakolasjaakola
committed
MDEV-34784 unhandled FK dependency with DML vs DDL
Certain DDL statements (e.g. ALTER TABLE) require innodb table lock on tables having foreign key constraint reference to the table under DDL execution. This dependency is not added in write set key information. However, tables being referenced to will be added in the key information, so the table locking domain of DDL is only partially recorded. One harmful consequence of this missing dependency information happens when a DML modifies a FK child table's row, which has NULL in the FK referencing column. In such situation, the FK reference cannot be followed during DDL execution, and there will be no FK parent table keys recorded in the write set. Parallel applying (or multi-master access) of such DML and DDL on the FK parent table will cause applying conflicts. This scenario is presented in a new mtr test added in this commit. The commit has a fix for the DDL FK dependency handling by adding all FK child table names in the write set key information. The commit has also fixes for innodb lock0lock.cc error logging to report lock connflicts of table and record locks correctly.
1 parent b89cb5d commit 00a2596

4 files changed

Lines changed: 140 additions & 8 deletions

File tree

‎mysql-test/suite/galera/r/MDEV-34784.result‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
connection node_2;
2+
connection node_1;
3+
CREATE TABLE parent (
4+
id INT PRIMARY KEY,
5+
KEY (id)
6+
) ENGINE=InnoDB;
7+
CREATE TABLE child (
8+
id INT PRIMARY KEY,
9+
parent_id INT DEFAULT NULL,
10+
FOREIGN KEY (parent_id)
11+
REFERENCES parent(id)
12+
) ENGINE=InnoDB;
13+
connection node_2;
14+
SET SESSION wsrep_sync_wait = 0;
15+
SET GLOBAL wsrep_slave_threads=2;
16+
SET GLOBAL DEBUG_DBUG = "d,sync.wsrep_apply_toi";
17+
connection node_1;
18+
ALTER TABLE parent ADD COLUMN (j INT);
19+
connection node_2;
20+
SET DEBUG_SYNC = "now WAIT_FOR sync.wsrep_apply_toi_reached";
21+
connection node_1;
22+
INSERT INTO child(id) values (1);
23+
connection node_2;
24+
SET DEBUG_SYNC = "now SIGNAL signal.wsrep_apply_toi";
25+
SET SESSION wsrep_sync_wait = DEFAULT;
26+
SET DEBUG_SYNC = "RESET";
27+
SET GLOBAL DEBUG_DBUG = '';
28+
SET GLOBAL wsrep_slave_threads = DEFAULT;
29+
connection node_1;
30+
DROP TABLE child;
31+
DROP TABLE parent;

‎mysql-test/suite/galera/t/MDEV-34784.test‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
#
2+
# Testing that foreign key constraint dependencies are correctly
3+
# recorded in write set preventing harmfull parallel applying.
4+
#
5+
# In this scenario foreign reference column in child table is left
6+
# NULL in a DML, and therefore the FK parent table is not appended
7+
# in the key set. DDL execution on parent table will require table
8+
# lock on FK child table
9+
#
10+
11+
--source include/galera_cluster.inc
12+
--source include/have_debug.inc
13+
--source include/have_debug_sync.inc
14+
--source include/galera_have_debug_sync.inc
15+
16+
CREATE TABLE parent (
17+
id INT PRIMARY KEY,
18+
KEY (id)
19+
) ENGINE=InnoDB;
20+
21+
CREATE TABLE child (
22+
id INT PRIMARY KEY,
23+
parent_id INT DEFAULT NULL,
24+
FOREIGN KEY (parent_id)
25+
REFERENCES parent(id)
26+
) ENGINE=InnoDB;
27+
28+
29+
# Set up sync point for TOI applying in node 2
30+
--connection node_2
31+
SET SESSION wsrep_sync_wait = 0;
32+
SET GLOBAL wsrep_slave_threads=2;
33+
34+
SET GLOBAL DEBUG_DBUG = "d,sync.wsrep_apply_toi";
35+
36+
# replicate ALTER
37+
--connection node_1
38+
ALTER TABLE parent ADD COLUMN (j INT);
39+
40+
# wait for ALTER to reach applying state
41+
--connection node_2
42+
SET DEBUG_SYNC = "now WAIT_FOR sync.wsrep_apply_toi_reached";
43+
44+
#
45+
# Expect the INSERT to depend on the ALTER,
46+
# therefore it should wait for the ALTER to
47+
# finish before it can be applied.
48+
# If bug is present, the wait condition will
49+
# timeout
50+
#
51+
--let $expected_apply_waits = `SELECT VARIABLE_VALUE+1 FROM information_schema.global_status WHERE VARIABLE_NAME = 'wsrep_apply_waits'`
52+
53+
--connection node_1
54+
# note, the FK column will have NULL value
55+
INSERT INTO child(id) values (1);
56+
57+
# check that the INSERT depends on ALTER and waits for ALTER to complete first
58+
--connection node_2
59+
--let $wait_condition = SELECT VARIABLE_VALUE = $expected_apply_waits FROM information_schema.global_status WHERE VARIABLE_NAME = 'wsrep_apply_waits'
60+
--source include/wait_condition.inc
61+
SET DEBUG_SYNC = "now SIGNAL signal.wsrep_apply_toi";
62+
SET SESSION wsrep_sync_wait = DEFAULT;
63+
64+
SET DEBUG_SYNC = "RESET";
65+
SET GLOBAL DEBUG_DBUG = '';
66+
SET GLOBAL wsrep_slave_threads = DEFAULT;
67+
68+
--connection node_1
69+
DROP TABLE child;
70+
DROP TABLE parent;
71+

‎sql/wsrep_mysqld.cc‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1844,14 +1844,30 @@ bool wsrep_append_fk_parent_table(THD *thd, TABLE_LIST *tables,
18441844
FOREIGN_KEY_INFO *f_key_info;
18451845
List<FOREIGN_KEY_INFO> f_key_list;
18461846

1847-
table->table->file->get_foreign_key_list(thd, &f_key_list);
1848-
List_iterator_fast<FOREIGN_KEY_INFO> it(f_key_list);
1849-
while ((f_key_info= it++))
1847+
/* find FK parents */
18501848
{
1851-
WSREP_DEBUG("appended fkey %s", f_key_info->referenced_table->str);
1852-
keys->push_back(wsrep_prepare_key_for_toi(
1849+
table->table->file->get_foreign_key_list(thd, &f_key_list);
1850+
List_iterator_fast<FOREIGN_KEY_INFO> it(f_key_list);
1851+
while ((f_key_info= it++))
1852+
{
1853+
WSREP_DEBUG("appended parent FK key %s", f_key_info->referenced_table->str);
1854+
keys->push_back(wsrep_prepare_key_for_toi(
18531855
f_key_info->referenced_db->str, f_key_info->referenced_table->str,
18541856
wsrep::key::shared));
1857+
}
1858+
}
1859+
1860+
/* find FK children */
1861+
{
1862+
table->table->file->get_parent_foreign_key_list(thd, &f_key_list);
1863+
List_iterator_fast<FOREIGN_KEY_INFO> it(f_key_list);
1864+
while ((f_key_info= it++))
1865+
{
1866+
WSREP_DEBUG("appended child FK key %s", f_key_info->foreign_table->str);
1867+
keys->push_back(wsrep_prepare_key_for_toi(
1868+
f_key_info->foreign_db->str, f_key_info->foreign_table->str,
1869+
wsrep::key::shared));
1870+
}
18551871
}
18561872
}
18571873
}

‎storage/innobase/lock/lock0lock.cc‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,11 @@ static void wsrep_assert_valid_bf_bf_wait(const lock_t *lock, const trx_t *trx,
567567
<< " index: "
568568
<< lock->index->name()
569569
<< " that has lock ";
570-
lock_rec_print(stderr, lock, mtr);
570+
if (!lock->is_table()) {
571+
lock_rec_print(stderr, lock, mtr);
572+
} else {
573+
lock_table_print(stderr, lock);
574+
}
571575

572576
ib::error() << "WSREP state: ";
573577

@@ -1068,10 +1072,20 @@ void wsrep_report_error(const lock_t* victim_lock, const trx_t *bf_trx)
10681072
// should not execute concurrently
10691073
mtr_t mtr;
10701074
WSREP_ERROR("BF request is not compatible with victim");
1075+
1076+
auto print_lock_details = [&](const lock_t* lock) {
1077+
if (!lock->is_table()) {
1078+
lock_rec_print(stderr, lock, mtr);
1079+
} else {
1080+
lock_table_print(stderr, lock);
1081+
}
1082+
};
1083+
10711084
WSREP_ERROR("BF requesting lock: ");
1072-
lock_rec_print(stderr, bf_trx->lock.wait_lock, mtr);
1085+
print_lock_details(bf_trx->lock.wait_lock);
1086+
10731087
WSREP_ERROR("victim holding lock: ");
1074-
lock_rec_print(stderr, victim_lock, mtr);
1088+
print_lock_details(victim_lock);
10751089
}
10761090
#endif /* WITH_DEBUG */
10771091

0 commit comments

Comments
 (0)