Description:
PROBLEM
=======
Two concurrent transactions updating DIFFERENT rows of a parent table, each
triggering an FK ON UPDATE CASCADE, can silently miss child rows in the
second transaction's cascade. Result: the child table is left in a state
that violates the foreign key relationship, with no error reported. Because
cascaded row changes are not written to the binary log (replica applies the
parent row event and fires its own cascade), the replica typically ends up
with the CORRECT data while the master has the corrupted data -> silent
master/replica divergence.
TESTED VERSIONS
===============
Reproduced on 8.0.30 (both READ COMMITTED and REPEATABLE READ).
Code inspection: the same logic exists in 8.0.22 ... 8.4.9 (row0ins.cc,
row_ins_check_foreign_constraint). MySQL 9.x cannot be tested this way
because FK referencing columns must now be UNIQUE, which prevents the
non-unique reference required by this scenario.
ROOT CAUSE ANALYSIS
===================
row_ins_check_foreign_constraint() (storage/innobase/row/row0ins.cc) scans
the child index with pcur.open(..., PAGE_CUR_GE, ...) and walks forward,
looking for rows matching the OLD key value, then performs the cascaded
update per matching row.
The scan is a "current read" of the B-tree, so it can only see secondary
index entries that have already been PHYSICALLY inserted by concurrent
transactions. There is no lock that serializes the scan against a
concurrent cascaded INSERT of new matching entries:
- With the skip_gap_lock optimization introduced by
Bug#25082593 "FOREIGN KEY VALIDATION DOESN'T NEED TO ACQUIRE GAP LOCK IN
READ COMMITTED" (fixed in 5.7.18 / 8.0.1, see also Bug#82400), in READ
COMMITTED the scan end takes no LOCK_GAP, supremum is skipped and
delete-marked matches are locked LOCK_REC_NOT_GAP only.
- In REPEATABLE READ the gap locks taken by the scan (LOCK_GAP at the scan
end, next-key on delete-marked records) still do NOT cover the position
where the concurrent transaction inserts its new entries, so the race
can be hit under RR as well (verified).
If transaction B's cascade scan is positioned BEFORE transaction A inserts
the new secondary index entries produced by A's own cascade of the same key
value, B never sees those rows, and the two transactions have completely
disjoint lock sets (different parent clustered rows, different child
clustered rows, different index ranges), so neither blocks, no deadlock is
detected, and both commit successfully.
This appears to be an unintended consequence of Bug#25082593, which was
aimed only at removing "unnecessary" gap locks to fix MTS replica hangs
(Bug#82400); the concurrency-correctness of cascaded referential actions
was not re-evaluated. Bug#89094 ("Data inconsistency on master after
upgrading to 5.7.19", closed as Can't repeat) shows the same symptom shape
(master missing cascaded child-row changes, replica consistent) right after
the version where Bug#25082593 was integrated, and may be the same
underlying race.
REPLICATION IMPACT
==================
Verified on 8.0.30: the binary log contains only the parent-table row
events (no child-table row events for the cascaded changes), which matches
the documented behavior ("cascading deletes are handled internally by the
InnoDB storage engine, which means that none of the changes are logged").
The replica re-fires the cascade when applying the parent row event, with
different timing, and typically produces the complete/correct cascade, so
master and replica diverge.
How to repeat:
Use three concurrent client sessions. Reproduces in both READ COMMITTED and
REPEATABLE READ on 8.0.30 (any innodb_lock_wait_timeout >= ~30s works,
default 50s is fine).
-- Session 0 (setup):
CREATE DATABASE fktest;
USE fktest;
CREATE TABLE sbtest1(id INT PRIMARY KEY, k INT, KEY k_1(k)) ENGINE=InnoDB;
CREATE TABLE t1(id INT PRIMARY KEY, k INT, KEY k_1(k),
CONSTRAINT fkey_t FOREIGN KEY(k) REFERENCES sbtest1(k)
ON UPDATE CASCADE) ENGINE=InnoDB;
INSERT INTO sbtest1 VALUES (9,456),(509,456),(643,456),(261,457),(544,457),(999,458);
INSERT INTO t1 VALUES (9,456),(509,456),(643,456),(261,457),(544,457),(999,458);
-- Session 1: take an X lock on the clustered record of the FIRST child row
-- that session A's cascade will touch (t1.id=9), and hold it:
SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
BEGIN;
SELECT id,k FROM t1 WHERE id=9 FOR UPDATE;
-- Session 2 (A): update parent row id=643 (k: 456 -> 457). Its cascade
-- updates t1 rows with k=456 and BLOCKS on the X lock above, BEFORE it
-- inserts any new t1.k_1 entry with k=457:
SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
UPDATE sbtest1 SET k=k+1 WHERE id=643; -- blocks (this is intended)
-- Session 3 (B): update a DIFFERENT parent row id=544 (k: 457 -> 458).
-- B's cascade scans t1.k_1 for k=457. A's new entries are not there yet,
-- so B only sees the two native k=457 rows (id=261 and id=544) and is not
-- blocked by anything. It returns in ~0.01 s:
SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
UPDATE sbtest1 SET k=k+1 WHERE id=544; -- returns immediately
-- Session 1: release the lock:
ROLLBACK;
-- Session 2 (A) unblocks and finishes its cascade (t1 k=456 rows -> 457).
-- Verify:
SELECT id,k FROM t1 ORDER BY id;
Observed (WRONG, 8.0.30):
id k
9 457 <- should be 458 (missed by B's cascade)
261 458
509 457 <- should be 458 (missed)
544 458
643 457 <- should be 458 (missed)
999 458
SELECT COUNT(*) FROM t1 WHERE k=457; -- returns 3, expected 0
Control test: executing the same two UPDATEs sequentially (without
session 1's lock) yields the correct result (all five rows at k=458), so
the corruption only occurs under the interleaving above.
The same divergence appears on a replica: the binlog only contains the two
parent row UPDATEs, the replica fires its own cascade for each one after
the previous transaction is committed, and ends up with the correct data
(all rows at 458) -> master and replica disagree on 3 rows.
Suggested fix:
In row_ins_check_foreign_constraint() (storage/innobase/row/row0ins.cc),
do not apply the skip_gap_lock optimization (Bug#25082593) when the scan
is performed to find child rows for a referential action
(check_ref == false, i.e. the parent-side scan that drives
cascade/set-null). In that path the scan must take gap locks (next-key
locks on the scanned range, LOCK_GAP on the record where the scan ends)
so that a concurrent transaction cannot insert new matching child index
entries into the scan range while the cascade is being performed, which
would serialize concurrent cascades via lock waits / deadlock resolution.
The skip_gap_lock optimization can be kept for the check_ref == true path
(child-side insert validating existence of the parent row), which is the
case the original Bug#25082593 was about.
