Backport into build-200906240007-5.1.34sp1

> ------------------------------------------------------------
> revno: 2857.1.1
> revision-id: satya.bn@sun.com-20090415114608-26b21dtx3doeidcc
> parent: davi.arnaut@sun.com-20090414120532-9a34lwlk105z8log
> committer: Satya B <satya.bn@sun.com>
> branch nick: mysql-5.1-bugteam-innodb
> timestamp: Wed 2009-04-15 17:16:08 +0530
> message:
>   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 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 (prebuilt->new_rec_locks >= 2) {
+			/* Restore the cursor position and find the record
+			in the clustered index. */
 
-			goto func_exit;
-		}
+			if (!has_latches_on_recs) {
+				btr_pcur_restore_position(BTR_SEARCH_LEAF,
+							  clust_pcur, &mtr);
 			}
 
+			rec = btr_pcur_get_rec(clust_pcur);
 			index = btr_pcur_get_btr_cur(clust_pcur)->index;
+		}
 
-	if (index != NULL && trx_new_rec_locks_contain(trx, index)) {
+		/* If the record has been modified by this
+		transaction, do not unlock it. */
+		ut_a(index->type & DICT_CLUSTERED);
 
-		mtr_start(&mtr);
+		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_;
 
-		/* Restore the cursor position and find the record */
+			*offsets_ = (sizeof offsets_) / sizeof *offsets_;
+			offsets = rec_get_offsets(rec, index, offsets,
+						  ULINT_UNDEFINED, &heap);
 
-		if (!has_latches_on_recs) {
-			btr_pcur_restore_position(BTR_SEARCH_LEAF, clust_pcur,
-						  &mtr);
+			rec_trx_id = row_get_rec_trx_id(rec, index, offsets);
+
+			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);
 
+			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);
 }