bzr push into mysql-trunk-innodb branch (marko.makela:3436 to 3439)

[marko.makela]

 3439 Marko Mäkelä	2011-01-19
      Add consistency checks to starting a transaction.
      
      trx_start_if_not_started(), trx_start_if_not_started_xa():
      Validate trx->state.
      
      Approved by Sunny Bains on IRC.

    modified:
      storage/innobase/trx/trx0trx.c
 3438 Marko Mäkelä	2011-01-19
      Consistently protect trx_sys->view_list with trx_sys->read_view_mutex.
      
      trx_sys_close(): Use trx_sys->read_view_mutex instead of trx_sys->lock.
      This should not have affected correctness, but was inconsistent with the
      documentation of trx_sys->read_view_mutex.
      
      trx_purge_dml_delay(): Note why the read is dirty.
      
      Approved by Sunny Bains on IRC.

    modified:
      storage/innobase/trx/trx0purge.c
      storage/innobase/trx/trx0sys.c
 3437 Marko Mäkelä	2011-01-19
      lock_get_src_table(): Fix a race that was introduced in WL#5458.
      
      We need to protect trx->lock.trx_locks with trx->mutex, because other
      threads might want to convert one of our implicit locks to an explicit one.
      
      Approved by Sunny Bains on IRC.

    modified:
      storage/innobase/lock/lock0lock.c
 3436 Marko Mäkelä	2011-01-19
      Non-functional change: Add const qualifiers or in/out comments
      to the locking functions.
      This is part of the WL#5458 clean-up.

    modified:
      storage/innobase/include/lock0lock.h
      storage/innobase/lock/lock0lock.c
=== modified file 'storage/innobase/lock/lock0lock.c'
--- a/storage/innobase/lock/lock0lock.c	revid:marko.makela@strippedpof21kl3
+++ b/storage/innobase/lock/lock0lock.c	revid:marko.makela@stripped
@@ -692,9 +692,19 @@ lock_get_src_table(
 	dict_table_t*	src;
 	lock_t*		lock;
 
+	ut_ad(!lock_mutex_own());
+
 	src = NULL;
 	*mode = LOCK_NONE;
 
+	/* The trx mutex protects the trx_locks for our purposes.
+	Other transactions could want to convert one of our implicit
+	record locks to an explicit one. For that, they would need our
+	trx mutex. Waiting locks can be removed while only holding
+	lock_sys->mutex, but this is a running transaction and cannot
+	thus be holding any waiting locks. */
+	trx_mutex_enter(trx);
+
 	for (lock = UT_LIST_GET_FIRST(trx->lock.trx_locks);
 	     lock != NULL;
 	     lock = UT_LIST_GET_NEXT(trx_locks, lock)) {
@@ -715,12 +725,14 @@ lock_get_src_table(
 			    || UT_LIST_GET_FIRST(src->locks) != lock) {
 				/* We only support the case when
 				there is only one lock on this table. */
-				return(NULL);
+				src = NULL;
+				goto func_exit;
 			}
 		} else if (src != tab_lock->table) {
 			/* The transaction is locking more than
 			two tables (src and dest): abort */
-			return(NULL);
+			src = NULL;
+			goto func_exit;
 		}
 
 		/* Check that the source table is locked by
@@ -729,7 +741,8 @@ lock_get_src_table(
 		if (lock_mode == LOCK_IX || lock_mode == LOCK_IS) {
 			if (*mode != LOCK_NONE && *mode != lock_mode) {
 				/* There are multiple locks on src. */
-				return(NULL);
+				src = NULL;
+				goto func_exit;
 			}
 			*mode = lock_mode;
 		}
@@ -740,6 +753,8 @@ lock_get_src_table(
 		src = dest;
 	}
 
+func_exit:
+	trx_mutex_exit(trx);
 	return(src);
 }
 

=== modified file 'storage/innobase/trx/trx0purge.c'
--- a/storage/innobase/trx/trx0purge.c	revid:marko.makela@stripped01911-yy9ctmpapof21kl3
+++ b/storage/innobase/trx/trx0purge.c	revid:marko.makela@stripped8fs3610ch
@@ -1118,7 +1118,8 @@ trx_purge_dml_delay(void)
 	/* If we cannot advance the 'purge view' because of an old
 	'consistent read view', then the DML statements cannot be delayed.
 	Also, srv_max_purge_lag <= 0 means 'infinity'. Note: we do a dirty
-	read of the trx_sys_t data structure here. */
+	read of the trx_sys_t data structure here, without holding
+	trx_sys->read_view_mutex. */
 	if (srv_max_purge_lag > 0
 	    && !UT_LIST_GET_LAST(trx_sys->view_list)) {
 		float	ratio = (float) trx_sys->rseg_history_len

=== modified file 'storage/innobase/trx/trx0sys.c'
--- a/storage/innobase/trx/trx0sys.c	revid:marko.makela@stripped
+++ b/storage/innobase/trx/trx0sys.c	revid:marko.makela@strippedom-20110119104456-ngjuyqs8fs3610ch
@@ -1629,7 +1629,7 @@ trx_sys_close(void)
 	/* Check that all read views are closed except read view owned
 	by a purge. */
 
-	rw_lock_s_lock(&trx_sys->lock);
+	mutex_enter(&trx_sys->read_view_mutex);
 
 	if (UT_LIST_GET_LEN(trx_sys->view_list) > 1) {
 		fprintf(stderr,
@@ -1639,7 +1639,7 @@ trx_sys_close(void)
 			UT_LIST_GET_LEN(trx_sys->view_list) - 1);
 	}
 
-	rw_lock_s_unlock(&trx_sys->lock);
+	mutex_exit(&trx_sys->read_view_mutex);
 
 	sess_close(trx_dummy_sess);
 	trx_dummy_sess = NULL;

=== modified file 'storage/innobase/trx/trx0trx.c'
--- a/storage/innobase/trx/trx0trx.c	revid:marko.makela@stripped3
+++ b/storage/innobase/trx/trx0trx.c	revid:[e-mail]-20110119104456-ngjuyqs8fs3610ch
@@ -1614,20 +1614,30 @@ trx_start_if_not_started_xa(
 /*========================*/
 	trx_t*	trx)	/*!< in: transaction */
 {
-	ut_ad(trx->state != TRX_STATE_COMMITTED_IN_MEMORY);
+	switch (trx->state) {
+	case TRX_STATE_NOT_STARTED:
 
-	if (trx->state == TRX_STATE_NOT_STARTED) {
+		/* Update the info whether we should skip XA steps
+		that eat CPU time.
 
-		/* Update the info whether we should skip XA steps that eat
-		CPU time For the duration of the transaction trx->support_xa
-		is not reread from thd so any changes in the value take effect
-		in the next transaction. This is to avoid a scenario where some
-		undo generated by a transaction, has XA stuff, and other undo,
-		generated by the same transaction, doesn't. */
+		For the duration of the transaction trx->support_xa is
+		not reread from thd so any changes in the value take
+		effect in the next transaction. This is to avoid a
+		scenario where some undo generated by a transaction,
+		has XA stuff, and other undo, generated by the same
+		transaction, doesn't. */
 		trx->support_xa = thd_supports_xa(trx->mysql_thd);
 
 		trx_start_low(trx);
+		/* fall through */
+	case TRX_STATE_ACTIVE:
+		return;
+	case TRX_STATE_PREPARED:
+	case TRX_STATE_COMMITTED_IN_MEMORY:
+		break;
 	}
+
+	ut_error;
 }
 
 /*************************************************************//**
@@ -1638,11 +1648,17 @@ trx_start_if_not_started(
 /*=====================*/
 	trx_t*	trx)	/*!< in: transaction */
 {
-	ut_ad(trx->state != TRX_STATE_COMMITTED_IN_MEMORY);
-
-	if (trx->state == TRX_STATE_NOT_STARTED) {
-
+	switch (trx->state) {
+	case TRX_STATE_NOT_STARTED:
 		trx_start_low(trx);
+		/* fall through */
+	case TRX_STATE_ACTIVE:
+		return;
+	case TRX_STATE_PREPARED:
+	case TRX_STATE_COMMITTED_IN_MEMORY:
+		break;
 	}
-}
 
+	ut_error;
+
+}

Attachment: [text/bzr-bundle] bzr/[e-mail]