BUG#25082593 FOREIGN KEY VALIDATION DOESN'T NEED TO ACQUIRE GAP LOCK …

…IN READ COMMITTED

Problem :
---------
This bug is filed from the base replication bug#25040331 where the
slave thread times out while INSERT operation waits on GAP lock taken
during Foreign Key validation.

The primary reason for the lock wait is because the statements are
getting replayed in different order. However, we also observed
two things ...

1. The slave thread could always use "Read Committed" isolation for
row level replication.

2. It is not necessary to have GAP locks in "READ Committed" isolation
level in innodb.

This bug is filed to address point(2) to avoid taking GAP locks during
Foreign Key validation.

Solution :
----------
Innodb is primarily designed for "Repeatable Read" and the GAP lock
behaviour is default. For "Read Committed" isolation, we have special
handling in row_search_mvcc to avoid taking the GAP lock while
scanning records.

While looking for Foreign Key, the code is following the default
behaviour taking GAP locks. The suggested fix is to avoid GAP
locking during FK validation similar to normal search operation
(row_search_mvcc) for "Read Committed" isolation level.

Reviewed-by: Sunny Bains <sunny.bains@oracle.com>

RB: 14526

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

@@ -9,3 +9,23 @@ PARTITION BY HASH (c1) PARTITIONS 15;
 DROP TABLE t1;
 SET GLOBAL innodb_change_buffering_debug=0;
 SET GLOBAL innodb_limit_optimistic_insert_debug=0;
+#
+# Bug#25082593 FOREIGN KEY VALIDATION DOESN'T NEED
+# TO ACQUIRE GAP LOCK IN READ COMMITTED
+#
+SET GLOBAL innodb_limit_optimistic_insert_debug=2;
+CREATE TABLE t1(col1 INT PRIMARY KEY) ENGINE=INNODB;
+CREATE TABLE t2(col1 INT PRIMARY KEY, col2 INT NOT NULL,
+FOREIGN KEY(col2) REFERENCES t1(col1)) ENGINE=INNODB;
+INSERT INTO t1 VALUES(1), (3), (4);
+connect  con1,localhost,root;
+SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
+START TRANSACTION;
+INSERT INTO t2 VALUES(1, 3);
+connection default;
+START TRANSACTION;
+INSERT INTO t1 VALUES(2);
+disconnect con1;
+SET GLOBAL innodb_limit_optimistic_insert_debug=0;
+DROP TABLE t2;
+DROP TABLE t1;

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

@@ -15,3 +15,35 @@ DROP TABLE t1;
 
 SET GLOBAL innodb_change_buffering_debug=0;
 SET GLOBAL innodb_limit_optimistic_insert_debug=0;
+
+--echo #
+--echo # Bug#25082593 FOREIGN KEY VALIDATION DOESN'T NEED
+--echo # TO ACQUIRE GAP LOCK IN READ COMMITTED
+--echo #
+
+SET GLOBAL innodb_limit_optimistic_insert_debug=2;
+
+CREATE TABLE t1(col1 INT PRIMARY KEY) ENGINE=INNODB;
+
+CREATE TABLE t2(col1 INT PRIMARY KEY, col2 INT NOT NULL,
+FOREIGN KEY(col2) REFERENCES t1(col1)) ENGINE=INNODB;
+
+INSERT INTO t1 VALUES(1), (3), (4);
+
+connect (con1,localhost,root);
+
+SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
+START TRANSACTION;
+INSERT INTO t2 VALUES(1, 3);
+
+connection default;
+START TRANSACTION;
+
+INSERT INTO t1 VALUES(2);
+
+disconnect con1;
+
+SET GLOBAL innodb_limit_optimistic_insert_debug=0;
+
+DROP TABLE t2;
+DROP TABLE t1;

storage/innobase/row/row0ins.cc

@@ -1596,6 +1596,10 @@ row_ins_check_foreign_constraint(
 	ulint		offsets_[REC_OFFS_NORMAL_SIZE];
 	ulint*		offsets		= offsets_;
 
+	bool		skip_gap_lock;
+
+	skip_gap_lock = (trx->isolation_level <= TRX_ISO_READ_COMMITTED);
+
 	DBUG_ENTER("row_ins_check_foreign_constraint");
 
 	rec_offs_init(offsets_);
@@ -1725,6 +1729,11 @@ row_ins_check_foreign_constraint(
 
 		if (page_rec_is_supremum(rec)) {
 
+			if (skip_gap_lock) {
+
+				continue;
+			}
+
 			err = row_ins_set_shared_rec_lock(LOCK_ORDINARY, block,
 							  rec, check_index,
 							  offsets, thr);
@@ -1740,10 +1749,17 @@ row_ins_check_foreign_constraint(
 		cmp = cmp_dtuple_rec(entry, rec, offsets);
 
 		if (cmp == 0) {
+
+			ulint	lock_type;
+
+			lock_type = skip_gap_lock
+				? LOCK_REC_NOT_GAP
+				: LOCK_ORDINARY;
+
 			if (rec_get_deleted_flag(rec,
 						 rec_offs_comp(offsets))) {
 				err = row_ins_set_shared_rec_lock(
-					LOCK_ORDINARY, block,
+					lock_type, block,
 					rec, check_index, offsets, thr);
 				switch (err) {
 				case DB_SUCCESS_LOCKED_REC:
@@ -1824,9 +1840,13 @@ row_ins_check_foreign_constraint(
 		} else {
 			ut_a(cmp < 0);
 
-			err = row_ins_set_shared_rec_lock(
-				LOCK_GAP, block,
-				rec, check_index, offsets, thr);
+			err = DB_SUCCESS;
+
+			if (!skip_gap_lock) {
+				err = row_ins_set_shared_rec_lock(
+					LOCK_GAP, block,
+					rec, check_index, offsets, thr);
+			}
 
 			switch (err) {
 			case DB_SUCCESS_LOCKED_REC: