From 11aa4deb1a57e901b9aed14203e64baf95e7b7ff Mon Sep 17 00:00:00 2001 From: sjaakola Date: Mon, 4 May 2026 09:58:44 +0300 Subject: [PATCH] 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. --- mysql-test/suite/galera/r/MDEV-34784.result | 31 +++++++++ mysql-test/suite/galera/t/MDEV-34784.test | 71 +++++++++++++++++++++ sql/wsrep_mysqld.cc | 26 ++++++-- storage/innobase/lock/lock0lock.cc | 20 +++++- 4 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 mysql-test/suite/galera/r/MDEV-34784.result create mode 100644 mysql-test/suite/galera/t/MDEV-34784.test diff --git a/mysql-test/suite/galera/r/MDEV-34784.result b/mysql-test/suite/galera/r/MDEV-34784.result new file mode 100644 index 0000000000000..c4e0e7d6a6b4a --- /dev/null +++ b/mysql-test/suite/galera/r/MDEV-34784.result @@ -0,0 +1,31 @@ +connection node_2; +connection node_1; +CREATE TABLE parent ( +id INT PRIMARY KEY, +KEY (id) +) ENGINE=InnoDB; +CREATE TABLE child ( +id INT PRIMARY KEY, +parent_id INT DEFAULT NULL, +FOREIGN KEY (parent_id) +REFERENCES parent(id) +) ENGINE=InnoDB; +connection node_2; +SET SESSION wsrep_sync_wait = 0; +SET GLOBAL wsrep_slave_threads=2; +SET GLOBAL DEBUG_DBUG = "d,sync.wsrep_apply_toi"; +connection node_1; +ALTER TABLE parent ADD COLUMN (j INT); +connection node_2; +SET DEBUG_SYNC = "now WAIT_FOR sync.wsrep_apply_toi_reached"; +connection node_1; +INSERT INTO child(id) values (1); +connection node_2; +SET DEBUG_SYNC = "now SIGNAL signal.wsrep_apply_toi"; +SET SESSION wsrep_sync_wait = DEFAULT; +SET DEBUG_SYNC = "RESET"; +SET GLOBAL DEBUG_DBUG = ''; +SET GLOBAL wsrep_slave_threads = DEFAULT; +connection node_1; +DROP TABLE child; +DROP TABLE parent; diff --git a/mysql-test/suite/galera/t/MDEV-34784.test b/mysql-test/suite/galera/t/MDEV-34784.test new file mode 100644 index 0000000000000..025ea049ab9dc --- /dev/null +++ b/mysql-test/suite/galera/t/MDEV-34784.test @@ -0,0 +1,71 @@ +# +# Testing that foreign key constraint dependencies are correctly +# recorded in write set preventing harmfull parallel applying. +# +# In this scenario foreign reference column in child table is left +# NULL in a DML, and therefore the FK parent table is not appended +# in the key set. DDL execution on parent table will require table +# lock on FK child table +# + +--source include/galera_cluster.inc +--source include/have_debug.inc +--source include/have_debug_sync.inc +--source include/galera_have_debug_sync.inc + +CREATE TABLE parent ( + id INT PRIMARY KEY, + KEY (id) +) ENGINE=InnoDB; + +CREATE TABLE child ( + id INT PRIMARY KEY, + parent_id INT DEFAULT NULL, + FOREIGN KEY (parent_id) + REFERENCES parent(id) +) ENGINE=InnoDB; + + +# Set up sync point for TOI applying in node 2 +--connection node_2 +SET SESSION wsrep_sync_wait = 0; +SET GLOBAL wsrep_slave_threads=2; + +SET GLOBAL DEBUG_DBUG = "d,sync.wsrep_apply_toi"; + +# replicate ALTER +--connection node_1 +ALTER TABLE parent ADD COLUMN (j INT); + +# wait for ALTER to reach applying state +--connection node_2 +SET DEBUG_SYNC = "now WAIT_FOR sync.wsrep_apply_toi_reached"; + +# +# Expect the INSERT to depend on the ALTER, +# therefore it should wait for the ALTER to +# finish before it can be applied. +# If bug is present, the wait condition will +# timeout +# +--let $expected_apply_waits = `SELECT VARIABLE_VALUE+1 FROM information_schema.global_status WHERE VARIABLE_NAME = 'wsrep_apply_waits'` + +--connection node_1 +# note, the FK column will have NULL value +INSERT INTO child(id) values (1); + +# check that the INSERT depends on ALTER and waits for ALTER to complete first +--connection node_2 +--let $wait_condition = SELECT VARIABLE_VALUE = $expected_apply_waits FROM information_schema.global_status WHERE VARIABLE_NAME = 'wsrep_apply_waits' +--source include/wait_condition.inc +SET DEBUG_SYNC = "now SIGNAL signal.wsrep_apply_toi"; +SET SESSION wsrep_sync_wait = DEFAULT; + +SET DEBUG_SYNC = "RESET"; +SET GLOBAL DEBUG_DBUG = ''; +SET GLOBAL wsrep_slave_threads = DEFAULT; + +--connection node_1 +DROP TABLE child; +DROP TABLE parent; + diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc index bffbd51635f1c..2dd9911908b6d 100644 --- a/sql/wsrep_mysqld.cc +++ b/sql/wsrep_mysqld.cc @@ -1844,14 +1844,30 @@ bool wsrep_append_fk_parent_table(THD *thd, TABLE_LIST *tables, FOREIGN_KEY_INFO *f_key_info; List f_key_list; - table->table->file->get_foreign_key_list(thd, &f_key_list); - List_iterator_fast it(f_key_list); - while ((f_key_info= it++)) + /* find FK parents */ { - WSREP_DEBUG("appended fkey %s", f_key_info->referenced_table->str); - keys->push_back(wsrep_prepare_key_for_toi( + table->table->file->get_foreign_key_list(thd, &f_key_list); + List_iterator_fast it(f_key_list); + while ((f_key_info= it++)) + { + WSREP_DEBUG("appended parent FK key %s", f_key_info->referenced_table->str); + keys->push_back(wsrep_prepare_key_for_toi( f_key_info->referenced_db->str, f_key_info->referenced_table->str, wsrep::key::shared)); + } + } + + /* find FK children */ + { + table->table->file->get_parent_foreign_key_list(thd, &f_key_list); + List_iterator_fast it(f_key_list); + while ((f_key_info= it++)) + { + WSREP_DEBUG("appended child FK key %s", f_key_info->foreign_table->str); + keys->push_back(wsrep_prepare_key_for_toi( + f_key_info->foreign_db->str, f_key_info->foreign_table->str, + wsrep::key::shared)); + } } } } diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 22db8d84af9d6..6522debc0e380 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -567,7 +567,11 @@ static void wsrep_assert_valid_bf_bf_wait(const lock_t *lock, const trx_t *trx, << " index: " << lock->index->name() << " that has lock "; - lock_rec_print(stderr, lock, mtr); + if (!lock->is_table()) { + lock_rec_print(stderr, lock, mtr); + } else { + lock_table_print(stderr, lock); + } ib::error() << "WSREP state: "; @@ -1068,10 +1072,20 @@ void wsrep_report_error(const lock_t* victim_lock, const trx_t *bf_trx) // should not execute concurrently mtr_t mtr; WSREP_ERROR("BF request is not compatible with victim"); + + auto print_lock_details = [&](const lock_t* lock) { + if (!lock->is_table()) { + lock_rec_print(stderr, lock, mtr); + } else { + lock_table_print(stderr, lock); + } + }; + WSREP_ERROR("BF requesting lock: "); - lock_rec_print(stderr, bf_trx->lock.wait_lock, mtr); + print_lock_details(bf_trx->lock.wait_lock); + WSREP_ERROR("victim holding lock: "); - lock_rec_print(stderr, victim_lock, mtr); + print_lock_details(victim_lock); } #endif /* WITH_DEBUG */