Applying InnoDB snashot 5.1-ss4699, part 1. Fixes BUG#39320 and other

problems

1) BUG#39320 - innodb crash in file btr/btr0pcur.c line 217 with 
               innodb_locks_unsafe_for_binlog

2) Fixes bug in multi-table semi consistent reads.

3) Fixes email address from dev@innodb.com to innodb_dev_ww@oracle.com

4) Fixes warning message generated by main.innodb test


Detailed revision comments:

r4399 | marko | 2009-03-12 09:38:05 +0200 (Thu, 12 Mar 2009) | 5 lines
branches/5.1: row_sel_get_clust_rec_for_mysql(): Store the cursor position
also for unlock_row().  (Bug #39320)

rb://96 approved by Heikki Tuuri.

r4400 | marko | 2009-03-12 10:06:44 +0200 (Thu, 12 Mar 2009) | 8 lines
branches/5.1: Fix a bug in multi-table semi-consistent reads.
Remember the acquired record locks per table handle (row_prebuilt_t)
rather than per transaction (trx_t), so that unlock_row should successfully
unlock all non-matching rows in multi-table operations.
This deficiency was found while investigating Bug #39320.

rb://94 approved by Heikki Tuuri.

r4481 | marko | 2009-03-19 15:01:48 +0200 (Thu, 19 Mar 2009) | 6 lines
branches/5.1: row_unlock_for_mysql(): Do not unlock records that were
modified by the current transaction.  This bug was introduced or unmasked
in r4400.

rb://97 approved by Heikki Tuuri

r4573 | vasil | 2009-03-30 14:17:13 +0300 (Mon, 30 Mar 2009) | 4 lines
branches/5.1:

Fix email address from dev@innodb.com to innodb_dev_ww@oracle.com

r4574 | vasil | 2009-03-30 14:27:08 +0300 (Mon, 30 Mar 2009) | 38 lines
branches/5.1:

Restore the state of INNODB_THREAD_CONCURRENCY to silence this warning:

  TEST                                      RESULT   TIME (ms)
  ------------------------------------------------------------
  
  worker[1] Using MTR_BUILD_THREAD 250, with reserved ports 12500..12509
  main.innodb                              [ pass ]   8803
  
  MTR's internal check of the test case 'main.innodb' failed.
  This means that the test case does not preserve the state that existed
  before the test case was executed.  Most likely the test case did not
  do a proper clean-up.
  This is the diff of the states of the servers before and after the
  test case was executed:
  mysqltest: Logging to '/tmp/autotest.sh-20090330_033000-5.1.5Hg8CY/mysql-5.1/mysql-test/var/tmp/check-mysqld_1.log'.
  mysqltest: Results saved in '/tmp/autotest.sh-20090330_033000-5.1.5Hg8CY/mysql-5.1/mysql-test/var/tmp/check-mysqld_1.result'.
  mysqltest: Connecting to server localhost:12500 (socket /tmp/autotest.sh-20090330_033000-5.1.5Hg8CY/mysql-5.1/mysql-test/var/tmp/mysqld.1.sock) as 'root', connection 'default', attempt 0 ...
  mysqltest: ... Connected.
  mysqltest: Start processing test commands from './include/check-testcase.test' ...
  mysqltest: ... Done processing test commands.
  --- /tmp/autotest.sh-20090330_033000-5.1.5Hg8CY/mysql-5.1/mysql-test/var/tmp/check-mysqld_1.result	2009-03-30 14:12:31.000000000 +0300
  +++ /tmp/autotest.sh-20090330_033000-5.1.5Hg8CY/mysql-5.1/mysql-test/var/tmp/check-mysqld_1.reject	2009-03-30 14:12:41.000000000 +0300
  @@ -99,7 +99,7 @@
   INNODB_SUPPORT_XA	ON
   INNODB_SYNC_SPIN_LOOPS	20
   INNODB_TABLE_LOCKS	ON
  -INNODB_THREAD_CONCURRENCY	8
  +INNODB_THREAD_CONCURRENCY	16
   INNODB_THREAD_SLEEP_DELAY	10000
   INSERT_ID	0
   INTERACTIVE_TIMEOUT	28800
  
  mysqltest: Result content mismatch
  
  not ok

r4576 | vasil | 2009-03-30 16:25:10 +0300 (Mon, 30 Mar 2009) | 4 lines
branches/5.1:

Revert a change to Makefile.am that I committed accidentally in c4574.

mysql-test/r/innodb-semi-consistent.result

@@ -38,3 +38,10 @@ a
 11
 7
 drop table t1;
+create table t1 (a int, b int) engine=myisam;
+create table t2 (c int, d int, key (c)) engine=innodb;
+insert into t1 values (1,1);
+insert into t2 values (1,2);
+set session transaction isolation level read committed;
+delete from t1 using t1 join t2 on t1.a = t2.c where t2.d in (1);
+drop table t1, t2;

mysql-test/t/innodb-semi-consistent.test

@@ -53,3 +53,16 @@ drop table t1;
 connection default;
 disconnect a;
 disconnect b;
+
+# Bug 39320
+create table t1 (a int, b int) engine=myisam;
+create table t2 (c int, d int, key (c)) engine=innodb;
+insert into t1 values (1,1);
+insert into t2 values (1,2);
+connect (a,localhost,root,,);
+connection a;
+set session transaction isolation level read committed;
+delete from t1 using t1 join t2 on t1.a = t2.c where t2.d in (1);
+connection default;
+disconnect a;
+drop table t1, t2;

mysql-test/t/innodb.test

@@ -6,7 +6,9 @@
 # Use innodb_mysql.[test|result] files instead.                       #
 #                                                                     #
 # If nevertheless you need to make some changes here, please, forward #
-# your commit message To: dev@innodb.com Cc: dev-innodb@mysql.com     #
+# your commit message                                                 #
+# To: innodb_dev_ww@oracle.com                                        #
+# Cc: dev-innodb@mysql.com                                            #
 # (otherwise your changes may be erased).                             #
 #                                                                     #
 #######################################################################
@@ -17,6 +19,10 @@
 # Small basic test with ignore
 #
 
+-- disable_query_log
+SET @innodb_thread_concurrency_orig = @@innodb_thread_concurrency;
+-- enable_query_log
+
 --disable_warnings
 drop table if exists t1,t2,t3,t4;
 drop database if exists mysqltest;
@@ -2524,6 +2530,8 @@ DROP TABLE bug35537;
 DISCONNECT c1;
 CONNECTION default;
 
+SET GLOBAL innodb_thread_concurrency = @innodb_thread_concurrency_orig;
+
 #######################################################################
 #                                                                     #
 # Please, DO NOT TOUCH this file as well as the innodb.result file.   #
@@ -2532,7 +2540,9 @@ CONNECTION default;
 # Use innodb_mysql.[test|result] files instead.                       #
 #                                                                     #
 # If nevertheless you need to make some changes here, please, forward #
-# your commit message To: dev@innodb.com Cc: dev-innodb@mysql.com     #
+# your commit message                                                 #
+# To: innodb_dev_ww@oracle.com                                        #
+# Cc: dev-innodb@mysql.com                                            #
 # (otherwise your changes may be erased).                             #
 #                                                                     #
 #######################################################################

storage/innobase/include/row0mysql.h

@@ -656,6 +656,21 @@ struct row_prebuilt_struct {
 					This eliminates lock waits in some
 					cases; note that this breaks
 					serializability. */
+	ulint		new_rec_locks;	/* normally 0; if
+					srv_locks_unsafe_for_binlog is
+					TRUE or session is using READ
+					COMMITTED isolation level, in a
+					cursor search, if we set a new
+					record lock on an index, this is
+					incremented; this is used in
+					releasing the locks under the
+					cursors if we are performing an
+					UPDATE and we determine after
+					retrieving the row that it does
+					not need to be locked; thus,
+					these can be used to implement a
+					'mini-rollback' that releases
+					the latest record locks */
 	ulint		mysql_prefix_len;/* byte offset of the end of
 					the last requested column */
 	ulint		mysql_row_len;	/* length in bytes of a row in the

storage/innobase/include/trx0trx.h

@@ -21,34 +21,6 @@ Created 3/26/1996 Heikki Tuuri
 
 extern ulint	trx_n_mysql_transactions;
 
-/*****************************************************************
-Resets the new record lock info in a transaction struct. */
-UNIV_INLINE
-void
-trx_reset_new_rec_lock_info(
-/*========================*/
-	trx_t*	trx);	/* in: transaction struct */
-/*****************************************************************
-Registers that we have set a new record lock on an index. We only have space
-to store 2 indexes! If this is called to store more than 2 indexes after
-trx_reset_new_rec_lock_info(), then this function does nothing. */
-UNIV_INLINE
-void
-trx_register_new_rec_lock(
-/*======================*/
-	trx_t*		trx,	/* in: transaction struct */
-	dict_index_t*	index);	/* in: trx sets a new record lock on this
-				index */
-/*****************************************************************
-Checks if trx has set a new record lock on an index. */
-UNIV_INLINE
-ibool
-trx_new_rec_locks_contain(
-/*======================*/
-				/* out: TRUE if trx has set a new record lock
-				on index */
-	trx_t*		trx,	/* in: transaction struct */
-	dict_index_t*	index);	/* in: index */
 /************************************************************************
 Releases the search latch if trx has reserved it. */
 
@@ -527,20 +499,6 @@ struct trx_struct{
 	lock_t*		auto_inc_lock;	/* possible auto-inc lock reserved by
 					the transaction; note that it is also
 					in the lock list trx_locks */
-	dict_index_t*	new_rec_locks[2];/* these are normally NULL; if
-					srv_locks_unsafe_for_binlog is TRUE
-					or session is using READ COMMITTED
-					isolation level,
-					in a cursor search, if we set a new
-					record lock on an index, this is set
-					to point to the index; this is
-					used in releasing the locks under the
-					cursors if we are performing an UPDATE
-					and we determine after retrieving
-					the row that it does not need to be
-					locked; thus, these can be used to
-					implement a 'mini-rollback' that
-					releases the latest record locks */
 	UT_LIST_NODE_T(trx_t)
 			trx_list;	/* list of transactions */
 	UT_LIST_NODE_T(trx_t)

storage/innobase/include/trx0trx.ic

@@ -38,61 +38,3 @@ trx_start_if_not_started_low(
 		trx_start_low(trx, ULINT_UNDEFINED);
 	}
 }
-
-/*****************************************************************
-Resets the new record lock info in a transaction struct. */
-UNIV_INLINE
-void
-trx_reset_new_rec_lock_info(
-/*========================*/
-	trx_t*	trx)	/* in: transaction struct */
-{
-	trx->new_rec_locks[0] = NULL;
-	trx->new_rec_locks[1] = NULL;
-}
-
-/*****************************************************************
-Registers that we have set a new record lock on an index. We only have space
-to store 2 indexes! If this is called to store more than 2 indexes after
-trx_reset_new_rec_lock_info(), then this function does nothing. */
-UNIV_INLINE
-void
-trx_register_new_rec_lock(
-/*======================*/
-	trx_t*		trx,	/* in: transaction struct */
-	dict_index_t*	index)	/* in: trx sets a new record lock on this
-				index */
-{
-	if (trx->new_rec_locks[0] == NULL) {
-		trx->new_rec_locks[0] = index;
-
-		return;
-	}
-
-	if (trx->new_rec_locks[0] == index) {
-
-		return;
-	}
-
-	if (trx->new_rec_locks[1] != NULL) {
-
-		return;
-	}
-
-	trx->new_rec_locks[1] = index;
-}
-
-/*****************************************************************
-Checks if trx has set a new record lock on an index. */
-UNIV_INLINE
-ibool
-trx_new_rec_locks_contain(
-/*======================*/
-				/* out: TRUE if trx has set a new record lock
-				on index */
-	trx_t*		trx,	/* in: transaction struct */
-	dict_index_t*	index)	/* in: index */
-{
-	return(trx->new_rec_locks[0] == index
-	       || trx->new_rec_locks[1] == index);
-}

storage/innobase/lock/lock0lock.c

@@ -1970,12 +1970,6 @@ lock_rec_lock_fast(
 	if (lock == NULL) {
 		if (!impl) {
 			lock_rec_create(mode, rec, index, trx);
-
-			if (srv_locks_unsafe_for_binlog
-			    || trx->isolation_level
-			    == TRX_ISO_READ_COMMITTED) {
-				trx_register_new_rec_lock(trx, index);
-			}
 		}
 
 		return(TRUE);
@@ -1999,11 +1993,6 @@ lock_rec_lock_fast(
 
 		if (!lock_rec_get_nth_bit(lock, heap_no)) {
 			lock_rec_set_nth_bit(lock, heap_no);
-			if (srv_locks_unsafe_for_binlog
-			    || trx->isolation_level
-			    == TRX_ISO_READ_COMMITTED) {
-				trx_register_new_rec_lock(trx, index);
-			}
 		}
 	}
 
@@ -2058,22 +2047,12 @@ lock_rec_lock_slow(
 		enough already granted on the record, we have to wait. */
 
 		err = lock_rec_enqueue_waiting(mode, rec, index, thr);
-
-		if (srv_locks_unsafe_for_binlog
-		    || trx->isolation_level == TRX_ISO_READ_COMMITTED) {
-			trx_register_new_rec_lock(trx, index);
-		}
 	} else {
 		if (!impl) {
 			/* Set the requested lock on the record */
 
 			lock_rec_add_to_queue(LOCK_REC | mode, rec, index,
 					      trx);
-			if (srv_locks_unsafe_for_binlog
-			    || trx->isolation_level
-			    == TRX_ISO_READ_COMMITTED) {
-				trx_register_new_rec_lock(trx, index);
-			}
 		}
 
 		err = DB_SUCCESS;

storage/innobase/row/row0mysql.c

@@ -1476,12 +1476,9 @@ row_unlock_for_mysql(
 					and clust_pcur, and we do not need to
 					reposition the cursors. */
 {
-	dict_index_t*	index;
 	btr_pcur_t*	pcur		= prebuilt->pcur;
 	btr_pcur_t*	clust_pcur	= prebuilt->clust_pcur;
 	trx_t*		trx		= prebuilt->trx;
-	rec_t*		rec;
-	mtr_t		mtr;
 
 	ut_ad(prebuilt && trx);
 	ut_ad(trx->mysql_thread_id == os_thread_get_curr_id());
@@ -1501,9 +1498,12 @@ row_unlock_for_mysql(
 
 	trx->op_info = "unlock_row";
 
-	index = btr_pcur_get_btr_cur(pcur)->index;
+	if (prebuilt->new_rec_locks >= 1) {
 
-	if (index != NULL && trx_new_rec_locks_contain(trx, index)) {
+		rec_t*		rec;
+		dict_index_t*	index;
+		dulint		rec_trx_id;
+		mtr_t		mtr;
 
 		mtr_start(&mtr);
 
@@ -1514,43 +1514,64 @@ row_unlock_for_mysql(
 		}
 
 		rec = btr_pcur_get_rec(pcur);
+		index = btr_pcur_get_btr_cur(pcur)->index;
 
-		lock_rec_unlock(trx, rec, prebuilt->select_lock_type);
-
-		mtr_commit(&mtr);
+		if (prebuilt->new_rec_locks >= 2) {
+			/* Restore the cursor position and find the record
+			in the clustered index. */
 
-		/* If the search was done through the clustered index, then
-		we have not used clust_pcur at all, and we must NOT try to
-		reset locks on clust_pcur. The values in clust_pcur may be
-		garbage! */
-
-		if (index->type & DICT_CLUSTERED) {
+			if (!has_latches_on_recs) {
+				btr_pcur_restore_position(BTR_SEARCH_LEAF,
+							  clust_pcur, &mtr);
+			}
 
-			goto func_exit;
+			rec = btr_pcur_get_rec(clust_pcur);
+			index = btr_pcur_get_btr_cur(clust_pcur)->index;
 		}
-	}
 
-	index = btr_pcur_get_btr_cur(clust_pcur)->index;
+		/* If the record has been modified by this
+		transaction, do not unlock it. */
+		ut_a(index->type & DICT_CLUSTERED);
 
-	if (index != NULL && trx_new_rec_locks_contain(trx, index)) {
+		if (index->trx_id_offset) {
+			rec_trx_id = trx_read_trx_id(rec
+						     + index->trx_id_offset);
+		} else {
+			mem_heap_t*	heap			= NULL;
+			ulint	offsets_[REC_OFFS_NORMAL_SIZE];
+			ulint*	offsets				= offsets_;
 
-		mtr_start(&mtr);
+			*offsets_ = (sizeof offsets_) / sizeof *offsets_;
+			offsets = rec_get_offsets(rec, index, offsets,
+						  ULINT_UNDEFINED, &heap);
 
-		/* Restore the cursor position and find the record */
+			rec_trx_id = row_get_rec_trx_id(rec, index, offsets);
 
-		if (!has_latches_on_recs) {
-			btr_pcur_restore_position(BTR_SEARCH_LEAF, clust_pcur,
-						  &mtr);
+			if (UNIV_LIKELY_NULL(heap)) {
+				mem_heap_free(heap);
+			}
 		}
 
-		rec = btr_pcur_get_rec(clust_pcur);
+		if (ut_dulint_cmp(rec_trx_id, trx->id) != 0) {
+			/* We did not update the record: unlock it */
+
+			rec = btr_pcur_get_rec(pcur);
+			index = btr_pcur_get_btr_cur(pcur)->index;
 
-		lock_rec_unlock(trx, rec, prebuilt->select_lock_type);
+			lock_rec_unlock(trx, rec, prebuilt->select_lock_type);
+
+			if (prebuilt->new_rec_locks >= 2) {
+				rec = btr_pcur_get_rec(clust_pcur);
+				index = btr_pcur_get_btr_cur(clust_pcur)->index;
+
+				lock_rec_unlock(trx, rec,
+						prebuilt->select_lock_type);
+			}
+		}
 
 		mtr_commit(&mtr);
 	}
 
-func_exit:
 	trx->op_info = "";
 
 	return(DB_SUCCESS);

storage/innobase/row/row0sel.c

@@ -2901,8 +2901,9 @@ row_sel_get_clust_rec_for_mysql(
 func_exit:
 	*out_rec = clust_rec;
 
-	if (prebuilt->select_lock_type == LOCK_X) {
-		/* We may use the cursor in update: store its position */
+	if (prebuilt->select_lock_type != LOCK_NONE) {
+		/* We may use the cursor in update or in unlock_row():
+		store its position */
 
 		btr_pcur_store_position(prebuilt->clust_pcur, mtr);
 	}
@@ -3303,13 +3304,7 @@ row_search_for_mysql(
 	is set or session is using a READ COMMITED isolation level. Then
 	we are able to remove the record locks set here on an individual
 	row. */
-
-	if ((srv_locks_unsafe_for_binlog
-	     || trx->isolation_level == TRX_ISO_READ_COMMITTED)
-	    && prebuilt->select_lock_type != LOCK_NONE) {
-
-		trx_reset_new_rec_lock_info(trx);
-	}
+	prebuilt->new_rec_locks = 0;
 
 	/*-------------------------------------------------------------*/
 	/* PHASE 1: Try to pop the row from the prefetch cache */
@@ -3951,6 +3946,12 @@ no_gap_lock:
 		switch (err) {
 			rec_t*	old_vers;
 		case DB_SUCCESS:
+			if (srv_locks_unsafe_for_binlog
+			    || trx->isolation_level == TRX_ISO_READ_COMMITTED) {
+				/* Note that a record of
+				prebuilt->index was locked. */
+				prebuilt->new_rec_locks = 1;
+			}
 			break;
 		case DB_LOCK_WAIT:
 			if (UNIV_LIKELY(prebuilt->row_read_type
@@ -3981,7 +3982,7 @@ no_gap_lock:
 			if (UNIV_LIKELY(trx->wait_lock != NULL)) {
 				lock_cancel_waiting_and_release(
 					trx->wait_lock);
-				trx_reset_new_rec_lock_info(trx);
+				prebuilt->new_rec_locks = 0;
 			} else {
 				mutex_exit(&kernel_mutex);
 
@@ -3993,6 +3994,9 @@ no_gap_lock:
 							  ULINT_UNDEFINED,
 							  &heap);
 				err = DB_SUCCESS;
+				/* Note that a record of
+				prebuilt->index was locked. */
+				prebuilt->new_rec_locks = 1;
 				break;
 			}
 			mutex_exit(&kernel_mutex);
@@ -4142,6 +4146,15 @@ requires_clust_rec:
 			goto next_rec;
 		}
 
+		if ((srv_locks_unsafe_for_binlog
+		     || trx->isolation_level == TRX_ISO_READ_COMMITTED)
+		    && prebuilt->select_lock_type != LOCK_NONE) {
+			/* Note that both the secondary index record
+			and the clustered index record were locked. */
+			ut_ad(prebuilt->new_rec_locks == 1);
+			prebuilt->new_rec_locks = 2;
+		}
+
 		if (UNIV_UNLIKELY(rec_get_deleted_flag(clust_rec, comp))) {
 
 			/* The record is delete marked: we can skip it */
@@ -4267,13 +4280,7 @@ next_rec:
 		prebuilt->row_read_type = ROW_READ_TRY_SEMI_CONSISTENT;
 	}
 	did_semi_consistent_read = FALSE;
-
-	if (UNIV_UNLIKELY(srv_locks_unsafe_for_binlog
-			  || trx->isolation_level == TRX_ISO_READ_COMMITTED)
-	    && prebuilt->select_lock_type != LOCK_NONE) {
-
-		trx_reset_new_rec_lock_info(trx);
-	}
+	prebuilt->new_rec_locks = 0;
 
 	/*-------------------------------------------------------------*/
 	/* PHASE 5: Move the cursor to the next index record */
@@ -4379,7 +4386,7 @@ lock_wait_or_error:
 			rec_loop we will again try to set a lock, and
 			new_rec_lock_info in trx will be right at the end. */
 
-			trx_reset_new_rec_lock_info(trx);
+			prebuilt->new_rec_locks = 0;
 		}
 
 		mode = pcur->search_mode;

storage/innobase/trx/trx0trx.c

@@ -192,8 +192,6 @@ trx_create(
 
 	trx->n_autoinc_rows = 0;
 
-	trx_reset_new_rec_lock_info(trx);
-
 	return(trx);
 }