Fix Bug#21293: Consider transactions that had edited non-transactional

tables heavier than ones that had not. This helps killing the "right"
transaction in case of a deadlock.

Approved by:	Heikki

handler/ha_innodb.cc

@@ -50,6 +50,7 @@ removing -DMYSQL_SERVER from ../Makefile.am as well. */
 #define thd_killed(thd) (thd)->killed
 #define thd_slave_thread(thd) (thd)->slave_thread
 #define thd_query(thd) (&(thd)->query)
+#define thd_non_transactional_update(thd) ((thd)->no_trans_update.all)
 #define mysql_bin_log_file_name() mysql_bin_log.get_log_fname()
 #define mysql_bin_log_file_pos() mysql_bin_log.get_log_file()->pos_in_file
 /*#define mysql_tmpfile() fileno(tmpfile())/* BUGGY: leaks memory, Bug #3998 */
@@ -505,6 +506,23 @@ innobase_release_stat_resources(
 	}
 }
 
+/**********************************************************************
+Returns true if the transaction this thread is processing has edited
+non-transactional tables. Used by the deadlock detector when deciding
+which transaction to rollback in case of a deadlock - we try to avoid
+rolling back transactions that have edited non-transactional tables. */
+
+extern "C"
+ibool
+thd_has_edited_nontrans_tables(
+/*===========================*/
+			/* out: true if non-transactional tables have
+			been edited */
+	void*	thd)	/* in: thread handle (THD*) */
+{
+	return((ibool) thd_non_transactional_update((THD*) thd));
+}
+
 /************************************************************************
 Obtain the InnoDB transaction of a MySQL thread. */
 inline

include/ha_prototypes.h

@@ -35,4 +35,17 @@ thd_is_replication_slave_thread(
 			/* out: true if thd is the replication thread */
 	void*	thd);	/* in: thread handle (THD*) */
 
+/**********************************************************************
+Returns true if the transaction this thread is processing has edited
+non-transactional tables. Used by the deadlock detector when deciding
+which transaction to rollback in case of a deadlock - we try to avoid
+rolling back transactions that have edited non-transactional tables. */
+
+ibool
+thd_has_edited_nontrans_tables(
+/*===========================*/
+			/* out: true if non-transactional tables have
+			been edited */
+	void*	thd);	/* in: thread handle (THD*) */
+
 #endif

include/trx0trx.h

@@ -374,7 +374,8 @@ trx_is_interrupted(
 /***********************************************************************
 Compares the "weight" (or size) of two transactions. The weight of one
 transaction is estimated as the number of altered rows + the number of
-locked rows. */
+locked rows. Transactions that have edited non-transactional tables are
+considered heavier than ones that have not. */
 
 int
 trx_weight_cmp(

mysql-test/innodb_trx_weight.inc

+-- connect (con1,localhost,,,)
+-- connect (con2,localhost,,,)
+
+-- connection con1
+SET autocommit=0;
+SELECT * FROM t1 FOR UPDATE;
+-- if ($con1_extra_sql_present) {
+	-- eval $con1_extra_sql
+-- }
+
+-- connection con2
+SET autocommit=0;
+SELECT * FROM t2 FOR UPDATE;
+-- if ($con2_extra_sql_present) {
+	-- eval $con2_extra_sql
+-- }
+
+-- if ($con1_should_be_rolledback) {
+	-- connection con1
+	-- send
+	INSERT INTO t2 VALUES (0);
+
+	-- connection con2
+	INSERT INTO t1 VALUES (0);
+	ROLLBACK;
+
+	-- connection con1
+	-- error ER_LOCK_DEADLOCK
+	-- reap
+-- }
+# else
+-- if (!$con1_should_be_rolledback) {
+	-- connection con2
+	-- send
+	INSERT INTO t1 VALUES (0);
+
+	-- connection con1
+	INSERT INTO t2 VALUES (0);
+	ROLLBACK;
+
+	-- connection con2
+	-- error ER_LOCK_DEADLOCK
+	-- reap
+-- }
+
+-- connection default
+
+DELETE FROM t5_nontrans;
+
+-- disconnect con1
+-- disconnect con2

mysql-test/innodb_trx_weight.result

 SET storage_engine=InnoDB;
-DROP TABLE IF EXISTS t1, t2, t3;
-CREATE TABLE t1 (a INT);
-CREATE TABLE t2 (a INT);
-CREATE TABLE t3 (a INT);
-INSERT INTO t1 VALUES (1);
-INSERT INTO t2 VALUES (1);
-INSERT INTO t3 VALUES (1), (2), (3);
-SET autocommit=0;
-SELECT * FROM t1 FOR UPDATE;
-a
-1
-SET autocommit=0;
-SELECT * FROM t2 FOR UPDATE;
-a
-1
-SELECT * FROM t3 FOR UPDATE;
-a
-1
-2
-3
-INSERT INTO t2 VALUES (0);
-INSERT INTO t1 VALUES (0);
-Got one of the listed errors
-DROP TABLE t1, t2, t3;

mysql-test/innodb_trx_weight.test

@@ -2,56 +2,101 @@
 # Ensure that the number of locks (SELECT FOR UPDATE for example) is
 # added to the number of altered rows when choosing the smallest
 # transaction to kill as a victim when a deadlock is detected.
+# Also transactions what had edited non-transactional tables should
+# be heavier than ones that had not.
 #
 
 -- source include/have_innodb.inc
+
 SET storage_engine=InnoDB;
 
+# we do not really care about what gets output-ed, we are only
+# interested in getting the deadlock resolved according to our
+# expectations
+-- disable_query_log
+-- disable_result_log
+
 -- disable_warnings
-DROP TABLE IF EXISTS t1, t2, t3;
+DROP TABLE IF EXISTS t1, t2, t3, t4, t5_nontrans;
 -- enable_warnings
 
 # we will create a simple deadlock with t1, t2 and two connections
 CREATE TABLE t1 (a INT);
 CREATE TABLE t2 (a INT);
-# auxiliary table with a bulk of rows which will be locked by the second
+
+# auxiliary table with a bulk of rows which will be locked by a
 # transaction to increase its weight
 CREATE TABLE t3 (a INT);
 
+# auxiliary empty table which will be inserted by a
+# transaction to increase its weight
+CREATE TABLE t4 (a INT);
+
+# auxiliary non-transactional table which will be edited by a
+# transaction to tremendously increase its weight
+CREATE TABLE t5_nontrans (a INT) ENGINE=MyISAM;
+
 INSERT INTO t1 VALUES (1);
 INSERT INTO t2 VALUES (1);
-INSERT INTO t3 VALUES (1), (2), (3);
+# insert a lot of rows in t3
+INSERT INTO t3 VALUES (1);
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
+INSERT INTO t3 SELECT * FROM t3;
 
--- connect (con1,localhost,,,)
--- connect (con2,localhost,,,)
+# test locking weight
 
--- connection con1
-SET autocommit=0;
-SELECT * FROM t1 FOR UPDATE;
+-- let $con1_extra_sql =
+-- let $con1_extra_sql_present = 0
+-- let $con2_extra_sql = SELECT * FROM t3 FOR UPDATE
+-- let $con2_extra_sql_present = 1
+-- let $con1_should_be_rolledback = 1
+-- source include/innodb_trx_weight.inc
 
--- connection con2
-SET autocommit=0;
-SELECT * FROM t2 FOR UPDATE;
-SELECT * FROM t3 FOR UPDATE;
+-- let $con1_extra_sql = INSERT INTO t4 VALUES (1), (1)
+-- let $con1_extra_sql_present = 1
+-- let $con2_extra_sql = SELECT * FROM t3 FOR UPDATE
+-- let $con2_extra_sql_present = 1
+-- let $con1_should_be_rolledback = 1
+-- source include/innodb_trx_weight.inc
 
--- connection con1
--- send
-INSERT INTO t2 VALUES (0);
+-- let $con1_extra_sql = INSERT INTO t4 VALUES (1), (1), (1), (1), (1), (1)
+-- let $con1_extra_sql_present = 1
+-- let $con2_extra_sql = SELECT * FROM t3 FOR UPDATE
+-- let $con2_extra_sql_present = 1
+-- let $con1_should_be_rolledback = 0
+-- source include/innodb_trx_weight.inc
 
--- connection con2
-INSERT INTO t1 VALUES (0);
+# test weight when non-transactional tables are edited
 
--- connection con1
-# we do not want this test to fail if the very descriptive text
-# "ERROR 40001: Deadlock found when trying to get lock; try restarting transaction"
-# gets changed someday, so force mysqltest to emit
-# "Got one of the listed errors"
--- error ER_LOCK_DEADLOCK,ER_LOCK_DEADLOCK
--- reap
+-- let $con1_extra_sql = INSERT INTO t4 VALUES (1), (1), (1)
+-- let $con1_extra_sql_present = 1
+-- let $con2_extra_sql =
+-- let $con2_extra_sql_present = 0
+-- let $con1_should_be_rolledback = 0
+-- source include/innodb_trx_weight.inc
 
--- connection default
+-- let $con1_extra_sql = INSERT INTO t4 VALUES (1), (1), (1)
+-- let $con1_extra_sql_present = 1
+-- let $con2_extra_sql = INSERT INTO t5_nontrans VALUES (1)
+-- let $con2_extra_sql_present = 1
+-- let $con1_should_be_rolledback = 1
+-- source include/innodb_trx_weight.inc
 
--- disconnect con1
--- disconnect con2
+-- let $con1_extra_sql = INSERT INTO t4 VALUES (1), (1), (1)
+-- let $con1_extra_sql = $con1_extra_sql; INSERT INTO t5_nontrans VALUES (1)
+-- let $con1_extra_sql_present = 1
+-- let $con2_extra_sql = INSERT INTO t5_nontrans VALUES (1)
+-- let $con2_extra_sql_present = 1
+-- let $con1_should_be_rolledback = 0
+-- source include/innodb_trx_weight.inc
 
-DROP TABLE t1, t2, t3;
+DROP TABLE t1, t2, t3, t4, t5_nontrans;

trx/trx0trx.c

@@ -1776,7 +1776,8 @@ trx_print(
 /***********************************************************************
 Compares the "weight" (or size) of two transactions. The weight of one
 transaction is estimated as the number of altered rows + the number of
-locked rows. */
+locked rows. Transactions that have edited non-transactional tables are
+considered heavier than ones that have not. */
 
 int
 trx_weight_cmp(
@@ -1785,6 +1786,42 @@ trx_weight_cmp(
 	trx_t*	a,	/* in: the first transaction to be compared */
 	trx_t*	b)	/* in: the second transaction to be compared */
 {
+	ibool	a_notrans_edit;
+	ibool	b_notrans_edit;
+
+	/* If mysql_thd is NULL for a transaction we assume that it has
+	not edited non-transactional tables. */
+
+	a_notrans_edit = a->mysql_thd != NULL
+	    && thd_has_edited_nontrans_tables(a->mysql_thd);
+
+	b_notrans_edit = b->mysql_thd != NULL
+	    && thd_has_edited_nontrans_tables(b->mysql_thd);
+
+	if (a_notrans_edit && !b_notrans_edit) {
+
+		return(1);
+	}
+
+	if (!a_notrans_edit && b_notrans_edit) {
+
+		return(-1);
+	}
+
+	/* Either both had edited non-transactional tables or both had
+	not, we fall back to comparing the number of altered/locked
+	rows. */
+
+#if 0
+	fprintf(stderr,
+		"%s TRX_WEIGHT(a): %lld+%lu, TRX_WEIGHT(b): %lld+%lu\n",
+		__func__,
+		ut_conv_dulint_to_longlong(a->undo_no),
+		UT_LIST_GET_LEN(a->trx_locks),
+		ut_conv_dulint_to_longlong(b->undo_no),
+		UT_LIST_GET_LEN(b->trx_locks));
+#endif
+
 #define TRX_WEIGHT(t)	\
 	ut_dulint_add((t)->undo_no, UT_LIST_GET_LEN((t)->trx_locks))