MDEV-13498 DELETE with CASCADE constraints takes long time / MDEV-13246

MDEV-13498 is a performance regression that was introduced in MariaDB 10.2.2
by commit fec844ac
which introduced some Galera-specific conditions that were being
evaluated even if the write-set replication was not enabled.

MDEV-13246 Stale rows despite ON DELETE CASCADE constraint
is a correctness regression that was introduced by the same commit.

Especially the subcondition
	!(parent && que_node_get_type(parent) == QUE_NODE_UPDATE)
which is equivalent to
	!parent || que_node_get_type(parent) != QUE_NODE_UPDATE
makes little sense. If parent==NULL, the evaluation would proceed to the
std::find() expression, which would dereference parent. Because no SIGSEGV
was observed related to this, we can conclude that parent!=NULL always
holds. But then, the condition would be equivalent to
	que_node_get_type(parent) != QUE_NODE_UPDATE
which would not make sense either, because the std::find() expression
is actually assuming the opposite when casting parent to upd_node_t*.

It looks like this condition never worked properly, or that
it was never properly tested, or both.

wsrep_must_process_fk(): Helper function to check if FOREIGN KEY
constraints need to be processed. Only evaluate the costly std::find()
expression when write-set replication is enabled.

Also, rely on operator<<(std::ostream&, const id_name_t&) and
operator<<(std::ostream&, const table_name_t&) for pretty-printing
index and table names.

row_upd_sec_index_entry(): Add !wsrep_thd_is_BF() to the condition.
This is applying part of "Galera MW-369 FK fixes"
https://github.com/codership/mysql-wsrep/commit/f37b79c6dab101310a45a9e8cb23c0f98716da52
that is described by the following part of the commit comment:
    additionally: skipping wsrep_row_upd_check_foreign_constraint if thd has
    BF, essentially is applier or replaying
    This FK check would be needed only for populating parent row FK keys
    in write set, so no use for appliers

mysql-test/suite/innodb/r/foreign_key.result

@@ -169,3 +169,56 @@ SET FOREIGN_KEY_CHECKS=DEFAULT;
 LOCK TABLE staff WRITE;
 UNLOCK TABLES;
 DROP TABLES staff, store;
+SET FOREIGN_KEY_CHECKS=1;
+#
+# MDEV-13246 Stale rows despite ON DELETE CASCADE constraint
+#
+CREATE TABLE users (
+id int unsigned AUTO_INCREMENT PRIMARY KEY,
+name varchar(32) NOT NULL DEFAULT ''
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+CREATE TABLE matchmaking_groups (
+id bigint unsigned AUTO_INCREMENT PRIMARY KEY,
+host_user_id int unsigned NOT NULL UNIQUE,
+CONSTRAINT FOREIGN KEY (host_user_id) REFERENCES users (id)
+ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+CREATE TABLE matchmaking_group_users (
+matchmaking_group_id bigint unsigned NOT NULL,
+user_id int unsigned NOT NULL,
+PRIMARY KEY (matchmaking_group_id,user_id),
+UNIQUE KEY user_id (user_id),
+CONSTRAINT FOREIGN KEY (matchmaking_group_id)
+REFERENCES matchmaking_groups (id) ON DELETE CASCADE ON UPDATE CASCADE,
+CONSTRAINT FOREIGN KEY (user_id)
+REFERENCES users (id) ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+CREATE TABLE matchmaking_group_maps (
+matchmaking_group_id bigint unsigned NOT NULL,
+map_id tinyint unsigned NOT NULL,
+PRIMARY KEY (matchmaking_group_id,map_id),
+CONSTRAINT FOREIGN KEY (matchmaking_group_id)
+REFERENCES matchmaking_groups (id) ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+INSERT INTO users VALUES (NULL,'foo'),(NULL,'bar');
+INSERT INTO matchmaking_groups VALUES (10,1),(11,2);
+INSERT INTO matchmaking_group_users VALUES (10,1),(11,2);
+INSERT INTO matchmaking_group_maps VALUES (10,55),(11,66);
+BEGIN;
+UPDATE users SET name = 'qux' WHERE id = 1;
+connect  con1,localhost,root,,;
+SET innodb_lock_wait_timeout= 1;
+DELETE FROM matchmaking_groups WHERE id = 10;
+disconnect con1;
+connection default;
+COMMIT;
+SELECT * FROM matchmaking_group_users WHERE matchmaking_group_id NOT IN (SELECT id FROM matchmaking_groups);
+matchmaking_group_id	user_id
+SELECT * FROM matchmaking_group_maps WHERE matchmaking_group_id NOT IN (SELECT id FROM matchmaking_groups);
+matchmaking_group_id	map_id
+SELECT * FROM users;
+id	name
+1	qux
+2	bar
+DROP TABLE
+matchmaking_group_maps, matchmaking_group_users, matchmaking_groups, users;

mysql-test/suite/innodb/t/foreign_key.test

 --source include/have_innodb.inc
+--source include/count_sessions.inc
 
 --echo #
 --echo # Bug #19027905 ASSERT RET.SECOND DICT_CREATE_FOREIGN_CONSTRAINTS_LOW
@@ -137,3 +138,66 @@ SET FOREIGN_KEY_CHECKS=DEFAULT;
 LOCK TABLE staff WRITE;
 UNLOCK TABLES;
 DROP TABLES staff, store;
+SET FOREIGN_KEY_CHECKS=1;
+
+--echo #
+--echo # MDEV-13246 Stale rows despite ON DELETE CASCADE constraint
+--echo #
+
+CREATE TABLE users (
+  id int unsigned AUTO_INCREMENT PRIMARY KEY,
+  name varchar(32) NOT NULL DEFAULT ''
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+CREATE TABLE matchmaking_groups (
+  id bigint unsigned AUTO_INCREMENT PRIMARY KEY,
+  host_user_id int unsigned NOT NULL UNIQUE,
+  CONSTRAINT FOREIGN KEY (host_user_id) REFERENCES users (id)
+  ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+CREATE TABLE matchmaking_group_users (
+  matchmaking_group_id bigint unsigned NOT NULL,
+  user_id int unsigned NOT NULL,
+  PRIMARY KEY (matchmaking_group_id,user_id),
+  UNIQUE KEY user_id (user_id),
+  CONSTRAINT FOREIGN KEY (matchmaking_group_id)
+  REFERENCES matchmaking_groups (id) ON DELETE CASCADE ON UPDATE CASCADE,
+  CONSTRAINT FOREIGN KEY (user_id)
+  REFERENCES users (id) ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+CREATE TABLE matchmaking_group_maps (
+  matchmaking_group_id bigint unsigned NOT NULL,
+  map_id tinyint unsigned NOT NULL,
+  PRIMARY KEY (matchmaking_group_id,map_id),
+  CONSTRAINT FOREIGN KEY (matchmaking_group_id)
+  REFERENCES matchmaking_groups (id) ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+INSERT INTO users VALUES (NULL,'foo'),(NULL,'bar');
+INSERT INTO matchmaking_groups VALUES (10,1),(11,2);
+INSERT INTO matchmaking_group_users VALUES (10,1),(11,2);
+INSERT INTO matchmaking_group_maps VALUES (10,55),(11,66);
+
+BEGIN;
+UPDATE users SET name = 'qux' WHERE id = 1;
+
+--connect (con1,localhost,root,,)
+SET innodb_lock_wait_timeout= 1;
+DELETE FROM matchmaking_groups WHERE id = 10;
+--disconnect con1
+
+--connection default
+COMMIT;
+--sorted_result
+SELECT * FROM matchmaking_group_users WHERE matchmaking_group_id NOT IN (SELECT id FROM matchmaking_groups);
+--sorted_result
+SELECT * FROM matchmaking_group_maps WHERE matchmaking_group_id NOT IN (SELECT id FROM matchmaking_groups);
+--sorted_result
+SELECT * FROM users;
+
+DROP TABLE
+matchmaking_group_maps, matchmaking_group_users, matchmaking_groups, users;
+
+--source include/wait_until_count_sessions.inc

storage/innobase/row/row0upd.cc

@@ -455,6 +455,25 @@ wsrep_row_upd_check_foreign_constraints(
 
 	return(err);
 }
+
+/** Determine if a FOREIGN KEY constraint needs to be processed.
+@param[in]	node	query node
+@param[in]	trx	transaction
+@return	whether the node cannot be ignored */
+static
+bool
+wsrep_must_process_fk(const upd_node_t* node, const trx_t* trx)
+{
+	if (que_node_get_type(node->common.parent) != QUE_NODE_UPDATE
+	    || !wsrep_on(trx->mysql_thd)) {
+		return false;
+	}
+
+	const upd_cascade_t&	nodes = *static_cast<const upd_node_t*>(
+		node->common.parent)->cascade_upd_nodes;
+	const upd_cascade_t::const_iterator end = nodes.end();
+	return std::find(nodes.begin(), end, node) == end;
+}
 #endif /* WITH_WSREP */
 
 /*********************************************************************//**
@@ -2414,29 +2433,18 @@ row_upd_sec_index_entry(
 		row_ins_sec_index_entry() below */
 		if (!rec_get_deleted_flag(
 			    rec, dict_table_is_comp(index->table))) {
-
-#ifdef WITH_WSREP
-			que_node_t *parent = que_node_get_parent(node);
-#endif
 			err = btr_cur_del_mark_set_sec_rec(
 				flags, btr_cur, TRUE, thr, &mtr);
 			if (err != DB_SUCCESS) {
 				break;
 			}
 #ifdef WITH_WSREP
-			if (err == DB_SUCCESS && !referenced                  &&
-			    !(parent && que_node_get_type(parent) ==
-				QUE_NODE_UPDATE                               &&
-			      (std::find(((upd_node_t*)parent)->cascade_upd_nodes->begin(),
-                                         ((upd_node_t*)parent)->cascade_upd_nodes->end(),
-                                         node) ==
-                               ((upd_node_t*)parent)->cascade_upd_nodes->end()))  &&
-                            foreign
-                        ) {
-				ulint*	offsets =
-					rec_get_offsets(
-						rec, index, NULL, ULINT_UNDEFINED,
-						&heap);
+			if (!referenced && foreign
+			    && wsrep_must_process_fk(node, trx)
+			    && !wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
+				ulint*	offsets = rec_get_offsets(
+					rec, index, NULL, ULINT_UNDEFINED,
+					&heap);
 
 				err = wsrep_row_upd_check_foreign_constraints(
 					node, &pcur, index->table,
@@ -2450,14 +2458,14 @@ row_upd_sec_index_entry(
 				case DB_DEADLOCK:
 					if (wsrep_debug) {
 						ib::warn() << "WSREP: sec index FK check fail for deadlock"
-							   << " index " << index->name()
-							   << " table " << index->table->name.m_name;
+							   << " index " << index->name
+							   << " table " << index->table->name;
 					}
 					break;
 				default:
-					ib::error() << "WSREP: referenced FK check fail: " << err
-						    << " index " << index->name()
-						    << " table " << index->table->name.m_name;
+					ib::error() << "WSREP: referenced FK check fail: " << ut_strerr(err)
+						    << " index " << index->name
+						    << " table " << index->table->name;
 
 					break;
 				}
@@ -2651,9 +2659,6 @@ row_upd_clust_rec_by_insert(
 	dberr_t		err;
 	rec_t*		rec;
 	ulint*		offsets			= NULL;
-#ifdef WITH_WSREP
-	que_node_t *parent = que_node_get_parent(node);
-#endif
 
 	ut_ad(node);
 	ut_ad(dict_index_is_clust(index));
@@ -2741,18 +2746,8 @@ row_upd_clust_rec_by_insert(
 			if (err != DB_SUCCESS) {
 				goto err_exit;
 			}
-		}
 #ifdef WITH_WSREP
-		if (!referenced                                              &&
-		    !(parent && que_node_get_type(parent) == QUE_NODE_UPDATE &&
-                      (std::find(((upd_node_t*)parent)->cascade_upd_nodes->begin(),
-                                 ((upd_node_t*)parent)->cascade_upd_nodes->end(),
-                                 node) ==
-                       ((upd_node_t*)parent)->cascade_upd_nodes->end()))       &&
-		    foreign
-		) {
-			err = wsrep_row_upd_check_foreign_constraints(
-				node, pcur, table, index, offsets, thr, mtr);
+		} else if (foreign && wsrep_must_process_fk(node, trx)) {
 			switch (err) {
 			case DB_SUCCESS:
 			case DB_NO_REFERENCED_ROW:
@@ -2761,14 +2756,14 @@ row_upd_clust_rec_by_insert(
 			case DB_DEADLOCK:
 				if (wsrep_debug) {
 					ib::warn() << "WSREP: sec index FK check fail for deadlock"
-						   << " index " << index->name()
-						   << " table " << index->table->name.m_name;
+						   << " index " << index->name
+						   << " table " << index->table->name;
 				}
 				break;
 			default:
-				ib::error() << "WSREP: referenced FK check fail: " << err
-					    << " index " << index->name()
-					    << " table " << index->table->name.m_name;
+				ib::error() << "WSREP: referenced FK check fail: " << ut_strerr(err)
+					    << " index " << index->name
+					    << " table " << index->table->name;
 
 				break;
 			}
@@ -2776,8 +2771,8 @@ row_upd_clust_rec_by_insert(
 			if (err != DB_SUCCESS) {
 				goto err_exit;
 			}
-		}
 #endif /* WITH_WSREP */
+		}
 	}
 
 	mtr_commit(mtr);
@@ -2959,9 +2954,7 @@ row_upd_del_mark_clust_rec(
 	btr_cur_t*	btr_cur;
 	dberr_t		err;
 	rec_t*		rec;
-#ifdef WITH_WSREP
-	que_node_t *parent = que_node_get_parent(node);
-#endif
+	trx_t*		trx = thr_get_trx(thr);
 	ut_ad(node);
 	ut_ad(dict_index_is_clust(index));
 	ut_ad(node->is_delete);
@@ -2972,7 +2965,7 @@ row_upd_del_mark_clust_rec(
 	/* Store row because we have to build also the secondary index
 	entries */
 
-	row_upd_store_row(node, thr_get_trx(thr)->mysql_thd,
+	row_upd_store_row(node, trx->mysql_thd,
 			  thr->prebuilt ? thr->prebuilt->m_mysql_table : NULL);
 
 	/* Mark the clustered index record deleted; we do not have to check
@@ -2984,22 +2977,14 @@ row_upd_del_mark_clust_rec(
 		btr_cur_get_block(btr_cur), rec,
 		index, offsets, thr, node->row, mtr);
 
-	if (err == DB_SUCCESS && referenced) {
+	if (err != DB_SUCCESS) {
+	} else if (referenced) {
 		/* NOTE that the following call loses the position of pcur ! */
 
 		err = row_upd_check_references_constraints(
 			node, pcur, index->table, index, offsets, thr, mtr);
-	}
 #ifdef WITH_WSREP
-	if (err == DB_SUCCESS && !referenced                         &&
-	    !(parent && que_node_get_type(parent) == QUE_NODE_UPDATE &&
-              (std::find(((upd_node_t*)parent)->cascade_upd_nodes->begin(),
-                         ((upd_node_t*)parent)->cascade_upd_nodes->end(),
-                         node) ==
-               ((upd_node_t*)parent)->cascade_upd_nodes->end()))       &&
-	    thr_get_trx(thr)                                         &&
-	    foreign
-	) {
+	} else if (foreign && wsrep_must_process_fk(node, trx)) {
 		err = wsrep_row_upd_check_foreign_constraints(
 			node, pcur, index->table, index, offsets, thr, mtr);
 		switch (err) {
@@ -3010,19 +2995,19 @@ row_upd_del_mark_clust_rec(
 		case DB_DEADLOCK:
 			if (wsrep_debug) {
 				ib::warn() << "WSREP: sec index FK check fail for deadlock"
-					   << " index " << index->name()
-					   << " table " << index->table->name.m_name;
+					   << " index " << index->name
+					   << " table " << index->table->name;
 			}
 			break;
 		default:
-			ib::error() << "WSREP: referenced FK check fail: " << err
-				    << " index " << index->name()
-				    << " table " << index->table->name.m_name;
+			ib::error() << "WSREP: referenced FK check fail: " << ut_strerr(err)
+				    << " index " << index->name
+				    << " table " << index->table->name;
 
 			break;
 		}
-	}
 #endif /* WITH_WSREP */
+	}
 
 	mtr_commit(mtr);