MDEV-36639 innodb_snapshot_isolation=1 gives error for not committed row changes

Set solution is to check if transaction, which modified a record, is still active in lock_clust_rec_read_check_and_lock(). if yes, then just request a lock. If no, then, depending on if the current transaction read view can see the changes, return eighter DB_RECORD_CHANGED or request a lock. We can do the check in lock_clust_rec_read_check_and_lock() because transaction tries to set a lock on the record which cursor points to after transaction resuming and cursor position restoring. If the lock already exists, then we don't request the lock again. But for the current commit it's important that lock_clust_rec_read_check_and_lock() will be invoked again for the same record, so we can do the check again after transaction, which modified a record, was committed or rolled back. MDEV-33802(4aa9291) is partially reverted. If some transaction holds implicit lock on some record and transaction with snapshot isolation level requests conflicting lock on the same record, it should be blocked instead of returning DB_RECORD_CHANGED to have ability to continue execution when implicit lock owner is rolled back. The construction -------------------------------------------------------------------------- let $wait_condition= select count(*) = 1 from information_schema.processlist where state = 'Updating' and info = 'UPDATE t SET b = 2 WHERE a'; --source include/wait_condition.inc -------------------------------------------------------------------------- is not reliable enought to make sure transaction is blocked in test case, the test failed sporadically with -------------------------------------------------------------------------- ./mtr --max-test-fail=1 --parallel=96 lock_isolation{,,,,,,,}{,,,}{,,} \ --repeat=500 -------------------------------------------------------------------------- command. That's why it was replaced with debug sync-points. Reviewed by: Marko Mäkelä
2025-07-21 23:50:01 +00:00 · 2025-04-22 15:49:53 +03:00
parent 4bedb222a8
commit 47e687b109
3 changed files with 207 additions and 82 deletions
--- a/mysql-test/suite/innodb/r/lock_isolation.result
+++ b/mysql-test/suite/innodb/r/lock_isolation.result
@ -1,3 +1,6 @@
+connect disable_purging,localhost,root;
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+connection default;
 #
 # MDEV-26642 Weird SELECT view when a record is
 #            modified to the same value by two transactions
@ -52,15 +55,17 @@ DROP TABLE t;
 # MDEV-26643 Inconsistent behaviors of UPDATE under
 #            READ UNCOMMITTED and READ COMMITTED isolation level
 #
-CREATE TABLE t(a INT, b INT) ENGINE=InnoDB;
+CREATE TABLE t(a INT, b INT) ENGINE=InnoDB STATS_PERSISTENT=0;
 INSERT INTO t VALUES(NULL, 1), (2, 2);
 SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
 BEGIN;
 UPDATE t SET a = 10;
 connection consistent;
 SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
 UPDATE t SET b = 20 WHERE a;
 connection default;
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 COMMIT;
 connection consistent;
 SELECT * FROM t;
@ -74,8 +79,10 @@ BEGIN;
 UPDATE t SET a = 10;
 connection consistent;
 SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
 UPDATE t SET b = 20 WHERE a;
 connection default;
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 COMMIT;
 connection consistent;
 SELECT * FROM t;
@ -89,8 +96,10 @@ BEGIN;
 UPDATE t SET a = 10;
 connection con_weird;
 SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
 UPDATE t SET b = 20 WHERE a;
 connection default;
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 SELECT * FROM t;
 a	b
 10	1
@ -113,8 +122,10 @@ UPDATE t SET b = 3;
 connection consistent;
 SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
 BEGIN;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
 UPDATE t SET b = 2 WHERE a;
 connection default;
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 UPDATE t SET a = 1;
 COMMIT;
 connection consistent;
@ -128,20 +139,25 @@ DROP TABLE t;
 #
 # MDEV-33802 Weird read view after ROLLBACK of other transactions
 #
-CREATE TABLE t(a INT PRIMARY KEY, b INT UNIQUE) ENGINE=InnoDB;
-INSERT INTO t SET a=1;
-BEGIN;
-INSERT INTO t SET a=2;
+CREATE TABLE t(a INT PRIMARY KEY, b INT UNIQUE) ENGINE=InnoDB STATS_PERSISTENT=0;
 connection consistent;
 START TRANSACTION WITH CONSISTENT SNAPSHOT;
+connection default;
+INSERT INTO t SET a=1;
+connection consistent;
 SAVEPOINT sp1;
 SELECT * FROM t FORCE INDEX (b) FOR UPDATE;
 ERROR HY000: Record has changed since last read in table 't'
 SAVEPOINT sp1;
+connection default;
+BEGIN;
+INSERT INTO t SET a=2;
 connection con_weird;
 START TRANSACTION WITH CONSISTENT SNAPSHOT;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
 SELECT * FROM t FORCE INDEX (b) FOR UPDATE;
 connection default;
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 ROLLBACK;
 connection con_weird;
 a	b
@ -149,12 +165,74 @@ a	b
 SELECT * FROM t FORCE INDEX (b) FOR UPDATE;
 a	b
 1	NULL
+COMMIT;
 disconnect con_weird;
 connection consistent;
 SELECT * FROM t FORCE INDEX (b) FOR UPDATE;
 a	b
 1	NULL
-disconnect consistent;
+COMMIT;
 connection default;
+TRUNCATE TABLE t;
+#
+# MDEV-36639 innodb_snapshot_isolation=1 gives error for not comitted row changes
+#
+INSERT INTO t VALUES (1,1),(2,2);
+connection default;
+# Case 1: Transaction A modifies a record, transaction B with snapshot
+# isolation level is blocked by A, then A is committed.
+# Expected behaviour: B gets ER_CHECKREAD.
+BEGIN;
+UPDATE t SET b=3 WHERE a = 1;
+connection consistent;
+SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
+SELECT * FROM t;
+a	b
+1	1
+2	2
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
+SELECT * FROM t WHERE a=1 FOR UPDATE;
+connection default;
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
+COMMIT;
+connection consistent;
+ERROR HY000: Record has changed since last read in table 't'
+# Case 2: Transaction A modifies a record, transaction B with snapshot
+# isolation level is blocked by A, then A is rolled back.
+# Expected behaviour: B continues execution.
+connection default;
+BEGIN;
+UPDATE t SET b=4 WHERE a=1;
+connection consistent;
+BEGIN;
+SELECT * FROM t;
+a	b
+2	2
+1	3
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
+SELECT * FROM t WHERE a=1 FOR UPDATE;
+connection default;
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
+ROLLBACK;
+connection consistent;
+a	b
+1	3
+ROLLBACK;
+# Case 3: Transaction B with snapshot isolation level started with
+# consistent snapshot. Transaction A modifies a record and is committed.
+# Both B tries to read modified by A record.
+# Expected behavior: B gets ER_CHECKREAD.
+connection consistent;
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+connection default;
+UPDATE t SET b=4 WHERE a=1;
+connection consistent;
+SELECT * FROM t WHERE a=1 FOR UPDATE;
+ERROR HY000: Record has changed since last read in table 't'
+disconnect consistent;
+disconnect disable_purging;
+connection default;
+SET DEBUG_SYNC="RESET";
 DROP TABLE t;
 # End of 10.6 tests
--- a/mysql-test/suite/innodb/t/lock_isolation.test
+++ b/mysql-test/suite/innodb/t/lock_isolation.test
@ -1,9 +1,16 @@
 --source include/have_innodb.inc
+--source include/count_sessions.inc
+--source include/have_debug.inc
+--source include/have_debug_sync.inc

 --disable_query_log
 call mtr.add_suppression("InnoDB: Transaction was aborted due to ");
 --enable_query_log

+--connect disable_purging,localhost,root
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+
+--connection default
 --echo #
 --echo # MDEV-26642 Weird SELECT view when a record is
 --echo #            modified to the same value by two transactions
@ -41,22 +48,18 @@ DROP TABLE t;
 --echo #            READ UNCOMMITTED and READ COMMITTED isolation level
 --echo #

-CREATE TABLE t(a INT, b INT) ENGINE=InnoDB;
+CREATE TABLE t(a INT, b INT) ENGINE=InnoDB STATS_PERSISTENT=0;
 INSERT INTO t VALUES(NULL, 1), (2, 2);
 SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
 BEGIN; UPDATE t SET a = 10;

 --connection consistent
 SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
 --send UPDATE t SET b = 20 WHERE a

 --connection default
-let $wait_condition=
-  select count(*) = 1 from information_schema.processlist
-  where state = 'Updating'
-  and info = 'UPDATE t SET b = 20 WHERE a';
--source include/wait_condition.inc
-
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 COMMIT;

 --connection consistent
@ -70,14 +73,11 @@ BEGIN; UPDATE t SET a = 10;

 --connection consistent
 SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
 --send UPDATE t SET b = 20 WHERE a

 --connection default
-let $wait_condition=
-  select count(*) = 1 from information_schema.processlist
-  where info = 'UPDATE t SET b = 20 WHERE a';
--source include/wait_condition.inc
-
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 COMMIT;

 --connection consistent
@ -91,15 +91,11 @@ BEGIN; UPDATE t SET a = 10;

 --connection con_weird
 SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
 send UPDATE t SET b = 20 WHERE a;

 --connection default
-let $wait_condition=
-  select count(*) = 1 from information_schema.processlist
-  where state = 'Updating'
-  and info = 'UPDATE t SET b = 20 WHERE a';
--source include/wait_condition.inc
-
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 SELECT * FROM t;
 COMMIT;

@ -123,14 +119,11 @@ SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
 BEGIN;
 # As semi-consistent read is disabled for innodb_snapshot_isolation=ON, the
 # following UPDATE must be blocked on the first record.
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
 --send UPDATE t SET b = 2 WHERE a

 --connection default
-let $wait_condition=
-  select count(*) = 1 from information_schema.processlist
-  where state = 'Updating' and info = 'UPDATE t SET b = 2 WHERE a';
--source include/wait_condition.inc
-
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 UPDATE t SET a = 1;
 COMMIT;
 --connection consistent
@ -149,13 +142,15 @@ DROP TABLE t;
 --echo # MDEV-33802 Weird read view after ROLLBACK of other transactions
 --echo #

-CREATE TABLE t(a INT PRIMARY KEY, b INT UNIQUE) ENGINE=InnoDB;
-INSERT INTO t SET a=1;
-
-BEGIN; INSERT INTO t SET a=2;
+CREATE TABLE t(a INT PRIMARY KEY, b INT UNIQUE) ENGINE=InnoDB STATS_PERSISTENT=0;

 --connection consistent
 START TRANSACTION WITH CONSISTENT SNAPSHOT;
+
+--connection default
+INSERT INTO t SET a=1;
+
+--connection consistent
 SAVEPOINT sp1;
 --disable_ps2_protocol
 --error ER_CHECKREAD
@ -163,29 +158,100 @@ SELECT * FROM t FORCE INDEX (b) FOR UPDATE;
 --enable_ps2_protocol
 SAVEPOINT sp1;

+--connection default
+BEGIN; INSERT INTO t SET a=2;
+
 --connection con_weird
 START TRANSACTION WITH CONSISTENT SNAPSHOT;
-send
-SELECT * FROM t FORCE INDEX (b) FOR UPDATE;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
+--send SELECT * FROM t FORCE INDEX (b) FOR UPDATE

 --connection default
-let $wait_condition=
-  select count(*) = 1 from information_schema.processlist
-  where state = 'Sending data'
-  and info LIKE 'SELECT * FROM t %';
--source include/wait_condition.inc
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
 ROLLBACK;

 --connection con_weird
 --reap
 SELECT * FROM t FORCE INDEX (b) FOR UPDATE;
+COMMIT;
 --disconnect con_weird

 --connection consistent
 SELECT * FROM t FORCE INDEX (b) FOR UPDATE;
--disconnect consistent
+COMMIT;

 --connection default
+TRUNCATE TABLE t;
+
+--echo #
+--echo # MDEV-36639 innodb_snapshot_isolation=1 gives error for not comitted row changes
+--echo #
+INSERT INTO t VALUES (1,1),(2,2);
+
+--connection default
+--echo # Case 1: Transaction A modifies a record, transaction B with snapshot
+--echo # isolation level is blocked by A, then A is committed.
+--echo # Expected behaviour: B gets ER_CHECKREAD.
+BEGIN;
+UPDATE t SET b=3 WHERE a = 1;
+
+--connection consistent
+SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
+SELECT * FROM t;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
+--send SELECT * FROM t WHERE a=1 FOR UPDATE
+
+--connection default
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
+COMMIT;
+
+--connection consistent
+--error ER_CHECKREAD
+--reap
+
+--echo # Case 2: Transaction A modifies a record, transaction B with snapshot
+--echo # isolation level is blocked by A, then A is rolled back.
+--echo # Expected behaviour: B continues execution.
+
+--connection default
+BEGIN;
+UPDATE t SET b=4 WHERE a=1;
+
+--connection consistent
+BEGIN;
+SELECT * FROM t;
+SET DEBUG_SYNC="lock_wait_before_suspend SIGNAL select_blocked";
+--send SELECT * FROM t WHERE a=1 FOR UPDATE
+
+--connection default
+SET DEBUG_SYNC="now WAIT_FOR select_blocked";
+ROLLBACK;
+
+--connection consistent
+--reap
+ROLLBACK;
+
+--echo # Case 3: Transaction B with snapshot isolation level started with
+--echo # consistent snapshot. Transaction A modifies a record and is committed.
+--echo # Both B tries to read modified by A record.
+--echo # Expected behavior: B gets ER_CHECKREAD.
+
+--connection consistent
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+
+--connection default
+UPDATE t SET b=4 WHERE a=1;
+
+--connection consistent
+--error ER_CHECKREAD
+SELECT * FROM t WHERE a=1 FOR UPDATE;
+--disconnect consistent
+--disconnect disable_purging
+
+--connection default
+SET DEBUG_SYNC="RESET";
 DROP TABLE t;

+--source include/wait_until_count_sessions.inc
 --echo # End of 10.6 tests
--- a/storage/innobase/lock/lock0lock.cc
+++ b/storage/innobase/lock/lock0lock.cc
@ -6052,17 +6052,10 @@ lock_clust_rec_modify_check_and_lock(
 	for it */

 	trx_t *trx = thr_get_trx(thr);
-	if (const trx_t *owner =
-	    lock_rec_convert_impl_to_expl<true>(trx, *block,
-						rec, index, offsets)) {
-		if (owner == trx) {
-			/* We already hold an exclusive lock. */
-			return DB_SUCCESS;
-		}
-
-		if (trx->snapshot_isolation && trx->read_view.is_open()) {
-			return DB_RECORD_CHANGED;
-		}
+	if (lock_rec_convert_impl_to_expl<true>(trx, *block,
+						rec, index, offsets) == trx) {
+		/* We already hold an exclusive lock. */
+		return DB_SUCCESS;
 	}

 	err = lock_rec_lock(true, LOCK_X | LOCK_REC_NOT_GAP,
@ -6224,19 +6217,11 @@ lock_sec_rec_read_check_and_lock(
 		return DB_SUCCESS;
 	}

-	if (page_rec_is_supremum(rec)) {
-	} else if (const trx_t *owner =
-		   lock_rec_convert_impl_to_expl<false>(trx, *block,
-							rec, index, offsets)) {
-		if (owner == trx) {
-			if (gap_mode == LOCK_REC_NOT_GAP) {
-				/* We already hold an exclusive lock. */
-				return DB_SUCCESS;
-			}
-		} else if (trx->snapshot_isolation
-			   && trx->read_view.is_open()) {
-			return DB_RECORD_CHANGED;
-		}
+	if (!page_rec_is_supremum(rec)
+	    && lock_rec_convert_impl_to_expl<false>(trx, *block, rec, index,
+						    offsets) == trx
+	    && gap_mode == LOCK_REC_NOT_GAP) {
+		return DB_SUCCESS;
 	}

 #ifdef WITH_WSREP
@ -6316,28 +6301,24 @@ lock_clust_rec_read_check_and_lock(
 	trx_t *trx = thr_get_trx(thr);
 	if (lock_table_has(trx, index->table, LOCK_X)
 	    || heap_no == PAGE_HEAP_NO_SUPREMUM) {
-	} else if (const trx_t *owner =
-		   lock_rec_convert_impl_to_expl<true>(trx, *block,
-						       rec, index, offsets)) {
-		if (owner == trx) {
-			if (gap_mode == LOCK_REC_NOT_GAP) {
-				/* We already hold an exclusive lock. */
-				return DB_SUCCESS;
-			}
-		} else if (trx->snapshot_isolation
-			   && trx->read_view.is_open()) {
-			return DB_RECORD_CHANGED;
-		}
+	} else if (lock_rec_convert_impl_to_expl<true>(trx, *block, rec, index,
+						       offsets) == trx
+	    && gap_mode == LOCK_REC_NOT_GAP) {
+		/* We already hold an exclusive lock. */
+		return DB_SUCCESS;
 	}

 	if (heap_no > PAGE_HEAP_NO_SUPREMUM && gap_mode != LOCK_GAP
            && trx->snapshot_isolation
-	    && trx->read_view.is_open()
-	    && !trx->read_view.changes_visible(
-		trx_read_trx_id(rec + row_trx_id_offset(rec, index)))
-	    && IF_WSREP(!(trx->is_wsrep()
+	    && trx->read_view.is_open()) {
+		trx_id_t trx_id= trx_read_trx_id(rec +
+						 row_trx_id_offset(rec, index));
+		if (!trx_sys.is_registered(trx, trx_id)
+		    && !trx->read_view.changes_visible(trx_id)
+		    && IF_WSREP(!(trx->is_wsrep()
 			&& wsrep_thd_skip_locking(trx->mysql_thd)), true)) {
-		return DB_RECORD_CHANGED;
+			return DB_RECORD_CHANGED;
+		}
 	}

 	dberr_t err = lock_rec_lock(false, gap_mode | mode,