Clean up trx_t::is_recovered

4 files changed, 18 insertions, 27 deletions

diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h
index 3aa357ab2e4..16f6eb12bea 100644
--- a/storage/innobase/include/trx0trx.h
+++ b/storage/innobase/include/trx0trx.h
@@ -699,10 +699,10 @@ Normally, only the thread that is currently associated with a running
 transaction may access (read and modify) the trx object, and it may do
 so without holding any mutex. The following are exceptions to this:
 
-* trx_rollback_resurrected() may access resurrected (connectionless)
-transactions while the system is already processing new user
-transactions. The trx_sys.mutex and trx->is_recovered prevent
-a race condition between it and trx_commit().
+* trx_rollback_recovered() may access resurrected (connectionless)
+transactions (state == TRX_STATE_ACTIVE && is_recovered)
+while the system is already processing new user transactions
+(state != TRX_STATE_ACTIVE || !is_recovered).
 
 * trx_print_low() may access transactions not associated with the current
 thread. The caller must be holding lock_sys.mutex.
@@ -839,10 +839,6 @@ public:
 
 	Transitions to COMMITTED are protected by trx_t::mutex. */
 	trx_state_t	state;
-	/** whether this is a recovered transaction that should be
-	rolled back by trx_rollback_or_clean_recovered().
-	Protected by trx_t::mutex for transactions that are in trx_sys. */
-	bool		is_recovered;
 
 	ReadView	read_view;	/*!< consistent read view used in the
 					transaction, or NULL if not yet set */
@@ -852,6 +848,15 @@ public:
 					by trx_t::mutex). */
 
 	/* These fields are not protected by any mutex. */
+
+	/** false=normal transaction, true=recovered (must be rolled back)
+	or disconnected transaction in XA PREPARE STATE.
+
+	This field is accessed by the thread that owns the transaction,
+	without holding any mutex.
+	There is only one foreign-thread access in trx_print_low()
+	and a possible race condition with trx_disconnect_prepared(). */
+	bool		is_recovered;
 	const char*	op_info;	/*!< English text describing the
 					current operation, or an empty
 					string */
diff --git a/storage/innobase/trx/trx0purge.cc b/storage/innobase/trx/trx0purge.cc
index b6206c9b3be..475babb6195 100644
--- a/storage/innobase/trx/trx0purge.cc
+++ b/storage/innobase/trx/trx0purge.cc
@@ -252,7 +252,7 @@ trx_purge_add_undo_to_history(const trx_t* trx, trx_undo_t*& undo, mtr_t* mtr)
 	/* After the purge thread has been given permission to exit,
 	we may roll back transactions (trx->undo_no==0)
 	in THD::cleanup() invoked from unlink_thd() in fast shutdown,
-	or in trx_rollback_resurrected() in slow shutdown.
+	or in trx_rollback_recovered() in slow shutdown.
 
 	Before any transaction-generating background threads or the
 	purge have been started, recv_recovery_rollback_active() can
diff --git a/storage/innobase/trx/trx0roll.cc b/storage/innobase/trx/trx0roll.cc
index 709ec98259c..2a2d2c3a315 100644
--- a/storage/innobase/trx/trx0roll.cc
+++ b/storage/innobase/trx/trx0roll.cc
@@ -764,12 +764,8 @@ static my_bool trx_rollback_recovered_callback(rw_trx_hash_element_t *element,
   mutex_enter(&element->mutex);
   if (trx_t *trx= element->trx)
   {
-    /* The trx->is_recovered flag and trx->state are set
-    atomically under the protection of the trx->mutex in
-    trx_t::commit_state(). We do not want to accidentally clean up
-    a non-recovered transaction here. */
     mutex_enter(&trx->mutex);
-    if (trx->is_recovered && trx_state_eq(trx, TRX_STATE_ACTIVE))
+    if (trx_state_eq(trx, TRX_STATE_ACTIVE) && trx->is_recovered)
       trx_list->push_back(trx);
     mutex_exit(&trx->mutex);
   }
@@ -815,7 +811,8 @@ void trx_rollback_recovered(bool all)
 
     ut_ad(trx);
     ut_d(trx_mutex_enter(trx));
-    ut_ad(trx->is_recovered && trx_state_eq(trx, TRX_STATE_ACTIVE));
+    ut_ad(trx->is_recovered);
+    ut_ad(trx_state_eq(trx, TRX_STATE_ACTIVE));
     ut_d(trx_mutex_exit(trx));
 
     if (!srv_is_being_started && !srv_undo_sources && srv_fast_shutdown)
diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc
index 6cbf0c273d9..f1d2c584093 100644
--- a/storage/innobase/trx/trx0trx.cc
+++ b/storage/innobase/trx/trx0trx.cc
@@ -480,18 +480,6 @@ inline void trx_t::commit_state()
   ut_ad(state != TRX_STATE_COMMITTED_IN_MEMORY
 	|| (is_recovered && !UT_LIST_GET_LEN(lock.trx_locks)));
   state= TRX_STATE_COMMITTED_IN_MEMORY;
-
-  /* If the background thread trx_rollback_or_clean_recovered()
-  is still active then there is a chance that the rollback
-  thread may see this trx as COMMITTED_IN_MEMORY and goes ahead
-  to clean it up calling trx_cleanup_at_db_startup(). This can
-  happen in the case we are committing a trx here that is left
-  in PREPARED state during the crash. Note that commit of the
-  rollback of a PREPARED trx happens in the recovery thread
-  while the rollback of other transactions happen in the
-  background thread. To avoid this race we unconditionally unset
-  the is_recovered flag. */
-  is_recovered= false;
   ut_ad(id || !is_referenced());
 }
 
@@ -1397,6 +1385,7 @@ trx_commit_in_memory(
 		} else {
 			trx_update_mod_tables_timestamp(trx);
 			MONITOR_INC(MONITOR_TRX_RW_COMMIT);
+			trx->is_recovered = false;
 		}
 	}