MDEV-11168: InnoDB: Failing assertion: !other_lock || wsrep_thd_is_BF…

…(lock->trx->mysql_thd, FALSE) || wsrep_thd_is_BF(other_lock->trx->mysql_thd, FALSE) Merged pull request: Fix error in lock_has_higher_priority #266 #266 Added test case.
MariaDB · Dec 2, 2016 · 2fd3af4 · 2fd3af4
1 parent dbdef41
commit 2fd3af4
Show file tree

Hide file tree

Showing 5 changed files with 229 additions and 35 deletions.
diff --git a/mysql-test/suite/innodb/r/innodb-lock-schedule-algorithm.result b/mysql-test/suite/innodb/r/innodb-lock-schedule-algorithm.result
@@ -0,0 +1,89 @@
+CREATE TABLE t1 (i1 INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1),(2);
+CREATE TABLE t2 (i2 int) ENGINE=MyISAM;
+BEGIN;
+DELETE FROM t1;
+BEGIN;
+INSERT INTO t2 VALUES (1),(2);
+SELECT * from t1;
+i1
+1
+2
+UPDATE t1 SET i1 = 1;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+COMMIT;
+COMMIT;
+SELECT * FROM t1;
+i1
+SELECT * FROM t2;
+i2
+1
+2
+DROP TABLE t1, t2;
+CREATE TABLE t1 (i1 INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1),(2);
+CREATE TABLE t2 (i2 int) ENGINE=MyISAM;
+BEGIN;
+DELETE FROM t1;
+BEGIN;
+INSERT INTO t2 VALUES (1),(2);
+SELECT * FROM t1;
+i1
+1
+2
+UPDATE t1 SET i1 = 1;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+COMMIT;
+COMMIT;
+SELECT * FROM t1;
+i1
+SELECT * FROM t2;
+i2
+1
+2
+DROP TABLE t1, t2;
+# "restart: --loose-innodb-lock-schedule-algorithm=FCFS"
+CREATE TABLE t1 (i1 INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1),(2);
+CREATE TABLE t2 (i2 int) ENGINE=MyISAM;
+BEGIN;
+DELETE FROM t1;
+BEGIN;
+INSERT INTO t2 VALUES (1),(2);
+SELECT * from t1;
+i1
+1
+2
+UPDATE t1 SET i1 = 1;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+COMMIT;
+COMMIT;
+SELECT * FROM t1;
+i1
+SELECT * FROM t2;
+i2
+1
+2
+DROP TABLE t1, t2;
+CREATE TABLE t1 (i1 INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1),(2);
+CREATE TABLE t2 (i2 int) ENGINE=MyISAM;
+BEGIN;
+DELETE FROM t1;
+BEGIN;
+INSERT INTO t2 VALUES (1),(2);
+SELECT * FROM t1;
+i1
+1
+2
+UPDATE t1 SET i1 = 1;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+COMMIT;
+COMMIT;
+SELECT * FROM t1;
+i1
+SELECT * FROM t2;
+i2
+1
+2
+DROP TABLE t1, t2;
diff --git a/mysql-test/suite/innodb/t/innodb-lock-schedule-algorithm.opt b/mysql-test/suite/innodb/t/innodb-lock-schedule-algorithm.opt
@@ -0,0 +1,2 @@
+--loose-innodb-lock-wait-timeout=1
+--loose-innodb-lock-schedule-algorithm=VATS
diff --git a/mysql-test/suite/innodb/t/innodb-lock-schedule-algorithm.test b/mysql-test/suite/innodb/t/innodb-lock-schedule-algorithm.test
@@ -0,0 +1,106 @@
+--source include/have_innodb.inc
+
+CREATE TABLE t1 (i1 INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1),(2);
+
+CREATE TABLE t2 (i2 int) ENGINE=MyISAM;
+BEGIN;
+DELETE FROM t1;
+
+--connect (con1,localhost,root,,test)
+connection con1;
+BEGIN;
+INSERT INTO t2 VALUES (1),(2);
+SELECT * from t1;
+--error 1205
+UPDATE t1 SET i1 = 1;
+COMMIT;
+
+connection default;
+COMMIT;
+SELECT * FROM t1;
+SELECT * FROM t2;
+DROP TABLE t1, t2;
+disconnect con1;
+
+CREATE TABLE t1 (i1 INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1),(2);
+
+CREATE TABLE t2 (i2 int) ENGINE=MyISAM;
+BEGIN;
+DELETE FROM t1;
+
+--connect (con1,localhost,root,,test)
+connection con1;
+BEGIN;
+INSERT INTO t2 VALUES (1),(2);
+SELECT * FROM t1;
+--error 1205
+UPDATE t1 SET i1 = 1;
+
+connection default;
+COMMIT;
+
+connection con1;
+COMMIT;
+
+connection default;
+SELECT * FROM t1;
+SELECT * FROM t2;
+DROP TABLE t1, t2;
+disconnect con1;
+
+--echo # "restart: --loose-innodb-lock-schedule-algorithm=FCFS"
+--let $restart_parameters=--loose_innodb_lock_schedule_algorithm=FCFS
+-- source include/restart_mysqld.inc
+
+CREATE TABLE t1 (i1 INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1),(2);
+
+CREATE TABLE t2 (i2 int) ENGINE=MyISAM;
+BEGIN;
+DELETE FROM t1;
+
+--connect (con1,localhost,root,,test)
+connection con1;
+BEGIN;
+INSERT INTO t2 VALUES (1),(2);
+SELECT * from t1;
+--error 1205
+UPDATE t1 SET i1 = 1;
+COMMIT;
+
+connection default;
+COMMIT;
+SELECT * FROM t1;
+SELECT * FROM t2;
+DROP TABLE t1, t2;
+disconnect con1;
+
+CREATE TABLE t1 (i1 INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1),(2);
+
+CREATE TABLE t2 (i2 int) ENGINE=MyISAM;
+BEGIN;
+DELETE FROM t1;
+
+--connect (con1,localhost,root,,test)
+connection con1;
+BEGIN;
+INSERT INTO t2 VALUES (1),(2);
+SELECT * FROM t1;
+--error 1205
+UPDATE t1 SET i1 = 1;
+
+connection default;
+COMMIT;
+
+connection con1;
+COMMIT;
+
+connection default;
+SELECT * FROM t1;
+SELECT * FROM t2;
+DROP TABLE t1, t2;
+disconnect con1;
+
diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
@@ -2267,8 +2267,6 @@ lock_rec_create(
 /*********************************************************************//**
 Check if lock1 has higher priority than lock2.
 NULL has lowest priority.
-Respect the preference of the upper server layer to reduce conflict
-during in-order parallel replication.
 If neither of them is wait lock, the first one has higher priority.
 If only one of them is a wait lock, it has lower priority.
 Otherwise, the one with an older transaction has higher priority.
@@ -2282,22 +2280,13 @@ has_higher_priority(
 		return false;
 	} else if (lock2 == NULL) {
 		return true;
-	}
-	// Ask the upper server layer if any of the two trx should be prefered.
-	int preference = thd_deadlock_victim_preference(lock1->trx->mysql_thd, lock2->trx->mysql_thd);
-	if (preference == -1) {
-		// lock1 is preferred as a victim, so lock2 has higher priority
-		return false;
-	} else if (preference == 1) {
-		// lock2 is preferred as a victim, so lock1 has higher priority
-		return true;
-	}
-	// No preference. Compre them by wait mode and trx age.
-	if (!lock_get_wait(lock1)) {
-		return true;
-	} else if (!lock_get_wait(lock2)) {
-		return false;
-	}
+    }
+    // No preference. Compre them by wait mode and trx age.
+    if (!lock_get_wait(lock1)) {
+        return true;
+    } else if (!lock_get_wait(lock2)) {
+        return false;
+    }
 	return lock1->trx->start_time_micro <= lock2->trx->start_time_micro;
 }
 
@@ -6652,18 +6641,28 @@ lock_rec_queue_validate(
 
 		if (!lock_rec_get_gap(lock) && !lock_get_wait(lock)) {
 
-#ifndef WITH_WSREP
 			enum lock_mode	mode;
 
 			if (lock_get_mode(lock) == LOCK_S) {
 				mode = LOCK_X;
 			} else {
 				mode = LOCK_S;
 			}
-			ut_a(!lock_rec_other_has_expl_req(
-				mode, 0, 0, block, heap_no, lock->trx));
+
+			const lock_t*	other_lock
+				= lock_rec_other_has_expl_req(
+					mode, 0, 0, block, heap_no,
+					lock->trx);
+#ifdef WITH_WSREP
+			ut_a(!other_lock
+			     || wsrep_thd_is_BF(lock->trx->mysql_thd, FALSE)
+			     || wsrep_thd_is_BF(other_lock->trx->mysql_thd, FALSE));
+
+#else
+			ut_a(!other_lock);
 #endif /* WITH_WSREP */
 
+
 		} else if (lock_get_wait(lock) && !lock_rec_get_gap(lock)
 				   && innodb_lock_schedule_algorithm == INNODB_LOCK_SCHEDULE_ALGORITHM_FCFS) {
 			// If using VATS, it's possible that a wait lock is inserted to a place in the list

diff --git a/storage/xtradb/lock/lock0lock.cc b/storage/xtradb/lock/lock0lock.cc
@@ -2057,8 +2057,6 @@ wsrep_print_wait_locks(
 /*********************************************************************//**
 Check if lock1 has higher priority than lock2.
 NULL has lowest priority.
-Respect the preference of the upper server layer to reduce conflict
-during in-order parallel replication.
 If neither of them is wait lock, the first one has higher priority.
 If only one of them is a wait lock, it has lower priority.
 Otherwise, the one with an older transaction has higher priority.
@@ -2073,15 +2071,6 @@ has_higher_priority(
 	} else if (lock2 == NULL) {
 		return true;
 	}
-	// Ask the upper server layer if any of the two trx should be prefered.
-	int preference = thd_deadlock_victim_preference(lock1->trx->mysql_thd, lock2->trx->mysql_thd);
-	if (preference == -1) {
-		// lock1 is preferred as a victim, so lock2 has higher priority
-		return false;
-	} else if (preference == 1) {
-		// lock2 is preferred as a victim, so lock1 has higher priority
-		return true;
-	}
 	// No preference. Compre them by wait mode and trx age.
 	if (!lock_get_wait(lock1)) {
 		return true;
@@ -6713,7 +6702,6 @@ lock_rec_queue_validate(
 
 		if (!lock_rec_get_gap(lock) && !lock_get_wait(lock)) {
 
-#ifndef WITH_WSREP
 			enum lock_mode	mode;
 
 
@@ -6722,8 +6710,18 @@ lock_rec_queue_validate(
 			} else {
 				mode = LOCK_S;
 			}
-			ut_a(!lock_rec_other_has_expl_req(
-			        mode, 0, 0, block, heap_no, lock->trx->id));
+
+			const lock_t*	other_lock
+				= lock_rec_other_has_expl_req(
+					mode, 0, 0, block, heap_no,
+					lock->trx->id);
+#ifdef WITH_WSREP
+			ut_a(!other_lock
+			     || wsrep_thd_is_BF(lock->trx->mysql_thd, FALSE)
+			     || wsrep_thd_is_BF(other_lock->trx->mysql_thd, FALSE));
+
+#else
+			ut_a(!other_lock);
 #endif /* WITH_WSREP */
 
 		} else if (lock_get_wait(lock) && !lock_rec_get_gap(lock)