MDEV-34784 unhandled FK dependency with DML vs DDL by sjaakola · Pull Request #5041 · MariaDB/server

sjaakola · 2026-05-05T04:52:58Z

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.

gemini-code-assist

Code Review

This pull request addresses MDEV-34784 by ensuring that both parent and child foreign key dependencies are correctly identified and recorded in the write set, preventing issues with parallel applying in Galera clusters. Additionally, it updates InnoDB's lock reporting logic to correctly distinguish between record and table locks during debug output. Review feedback suggests refactoring duplicated logic in wsrep_mysqld.cc and lock0lock.cc to improve maintainability, correcting inconsistent indentation, and removing redundant debug messages.

gemini-code-assist · 2026-05-05T04:56:15Z

      {
-        WSREP_DEBUG("appended fkey %s", f_key_info->referenced_table->str);
-        keys->push_back(wsrep_prepare_key_for_toi(
+	FOREIGN_KEY_INFO *f_key_info;
+	List<FOREIGN_KEY_INFO> f_key_list;
+
+	table->table->file->get_foreign_key_list(thd, &f_key_list);
+	List_iterator_fast<FOREIGN_KEY_INFO> 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 */
+      {
+	FOREIGN_KEY_INFO *f_key_info;
+	List<FOREIGN_KEY_INFO> f_key_list;
+
+	table->table->file->get_parent_foreign_key_list(thd, &f_key_list);
+	List_iterator_fast<FOREIGN_KEY_INFO> it(f_key_list);
+	WSREP_DEBUG("scanning child FK keys ");
+	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));
+	}
+	WSREP_DEBUG("scanned child FK keys ");
+
      }


The logic for finding FK parents and FK children is almost identical, leading to significant code duplication. This can be refactored to improve maintainability. For example, you could declare variables like f_key_info and f_key_list once and reuse them, clearing the list between calls.

Additionally, the new code blocks use tabs for indentation, which is inconsistent with the surrounding code that uses spaces. Please ensure consistent formatting throughout the file.

gemini-code-assist · 2026-05-05T04:56:15Z

+            f_key_info->foreign_db->str, f_key_info->foreign_table->str,
+            wsrep::key::shared));
+	}
+	WSREP_DEBUG("scanned child FK keys ");


This debug message appears to be redundant. There is already a WSREP_DEBUG message before the loop (scanning child FK keys) and one inside the loop for each key. This closing message adds unnecessary noise to the logs and can be removed.

gemini-code-assist · 2026-05-05T04:56:15Z

+  const lock_t *wait_lock= bf_trx->lock.wait_lock;
+  if (!wait_lock->is_table()) {
+    lock_rec_print(stderr, wait_lock, mtr);
+  } else {
+    lock_table_print(stderr, wait_lock);
+  }
+
  WSREP_ERROR("victim holding lock: ");
-  lock_rec_print(stderr, victim_lock, mtr);
+  if (!victim_lock->is_table()) {
+    lock_rec_print(stderr, victim_lock, mtr);
+  } else {
+    lock_table_print(stderr, victim_lock);
+  }


The logic to print lock details is duplicated for the waiting lock and the victim lock. This can be simplified by extracting the logic into a helper lambda to avoid repetition and improve readability.

auto print_lock_details = [&](const lock_t* lock) { if (!lock->is_table()) { lock_rec_print(stderr, lock, mtr); } else { lock_table_print(stderr, lock); } }; print_lock_details(bf_trx->lock.wait_lock); WSREP_ERROR("victim holding lock: "); print_lock_details(victim_lock);

janlindstrom

Please remove commented code and you could address AI review also. If I correctly remember using lock_rec_print is not safe to call as it expects page to be latched and that might not be always true.

janlindstrom · 2026-05-05T06:46:07Z

      {
-        WSREP_DEBUG("appended fkey %s", f_key_info->referenced_table->str);
-        keys->push_back(wsrep_prepare_key_for_toi(
+        //FOREIGN_KEY_INFO *f_key_info;


Remove commented code.

janlindstrom · 2026-05-05T06:46:20Z

+
+      /* find FK children */
+      {
+        //FOREIGN_KEY_INFO *f_key_info;


Remove commented code.

janlindstrom · 2026-05-06T08:50:04Z

 		    << " that has lock ";
-	lock_rec_print(stderr, lock, mtr);
+	if (!lock->is_table()) {
+		lock_rec_print(stderr, lock, mtr);


Is this safe i.e. is page always latched?

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.

gemini-code-assist Bot reviewed May 5, 2026

View reviewed changes

sjaakola force-pushed the 10.11-MDEV-34784 branch from 26d443d to 1202f23 Compare May 5, 2026 06:39

gkodinov added the MariaDB Corporation label May 5, 2026

janlindstrom requested changes May 6, 2026

View reviewed changes

sjaakola force-pushed the 10.11-MDEV-34784 branch from 1202f23 to 11aa4de Compare May 6, 2026 17:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

MDEV-34784 unhandled FK dependency with DML vs DDL#5041

MDEV-34784 unhandled FK dependency with DML vs DDL#5041
sjaakola wants to merge 1 commit into10.11from
10.11-MDEV-34784

sjaakola commented May 5, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 5, 2026

Uh oh!

gemini-code-assist Bot May 5, 2026

Uh oh!

gemini-code-assist Bot May 5, 2026

Uh oh!

janlindstrom left a comment

Uh oh!

janlindstrom May 5, 2026

Uh oh!

janlindstrom May 5, 2026

Uh oh!

janlindstrom May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

sjaakola commented May 5, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

janlindstrom left a comment

Choose a reason for hiding this comment

Uh oh!

janlindstrom May 5, 2026

Choose a reason for hiding this comment

Uh oh!

janlindstrom May 5, 2026

Choose a reason for hiding this comment

Uh oh!

janlindstrom May 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants