MDEV-15104 - Remove trx_sys_t::serialisation_list

serialisation_list was supposed to instantly give minimum registered
transaction serialisation number. However maintaining and accessing
this list requires global mutex protection.

Since we already take MVCC snapshot by iterating trx_sys_t::rw_trx_hash,
it is cheap to integrate minimum registered transaction lookup into this
iteration.

storage/innobase/include/trx0sys.h

@@ -393,6 +393,7 @@ struct rw_trx_hash_element_t
 
 
   trx_id_t id; /* lf_hash_init() relies on this to be first in the struct */
+  trx_id_t no;
   trx_t *trx;
   ib_mutex_t mutex;
 };
@@ -483,6 +484,7 @@ class rw_trx_hash_t
     ut_ad(element->trx == 0);
     element->trx= trx;
     element->id= trx->id;
+    element->no= TRX_ID_MAX;
     trx->rw_trx_hash_element= element;
   }
 
@@ -819,10 +821,6 @@ struct trx_sys_t {
 	MY_ALIGNED(CACHE_LINE_SIZE)
 	MVCC		mvcc;		/*!< Multi version concurrency control
 					manager */
-	trx_ut_list_t	serialisation_list;
-					/*!< Ordered on trx_t::no of all the
-					currenrtly active RW transactions */
-
 	MY_ALIGNED(CACHE_LINE_SIZE)
 	trx_ut_list_t	mysql_trx_list;	/*!< List of transactions created
 					for MySQL. All user transactions are
@@ -937,22 +935,27 @@ struct trx_sys_t {
     @param[in,out] caller_trx used to get access to rw_trx_hash_pins
     @param[out]    ids        array to store registered transaction identifiers
     @param[out]    max_trx_id variable to store m_max_trx_id value
+    @param[out]    mix_trx_no variable to store min(trx->no) value
   */
 
-  void snapshot_ids(trx_t *caller_trx, trx_ids_t *ids, trx_id_t *max_trx_id)
+  void snapshot_ids(trx_t *caller_trx, trx_ids_t *ids, trx_id_t *max_trx_id,
+                    trx_id_t *min_trx_no)
   {
     snapshot_ids_arg arg(ids);
 
     while ((arg.m_id= get_rw_trx_hash_version()) != get_max_trx_id())
       ut_delay(1);
+    arg.m_no= arg.m_id;
 
     ids->clear();
     ids->reserve(rw_trx_hash.size() + 32);
-    *max_trx_id= arg.m_id;
     rw_trx_hash.iterate(caller_trx,
                         reinterpret_cast<my_hash_walk_action>(copy_one_id),
                         &arg);
     std::sort(ids->begin(), ids->end());
+
+    *max_trx_id= arg.m_id;
+    *min_trx_no= arg.m_no;
   }
 
 
@@ -1050,6 +1053,7 @@ struct trx_sys_t {
     snapshot_ids_arg(trx_ids_t *ids): m_ids(ids) {}
     trx_ids_t *m_ids;
     trx_id_t m_id;
+    trx_id_t m_no;
   };
 
 
@@ -1057,7 +1061,13 @@ struct trx_sys_t {
                              snapshot_ids_arg *arg)
   {
     if (element->id < arg->m_id)
+    {
+      trx_id_t no= static_cast<trx_id_t>(my_atomic_load64_explicit(
+        reinterpret_cast<int64*>(&element->no), MY_MEMORY_ORDER_RELAXED));
       arg->m_ids->push_back(element->id);
+      if (no < arg->m_no)
+        arg->m_no= no;
+    }
     return 0;
   }
 

storage/innobase/include/trx0trx.h

@@ -976,12 +976,6 @@ struct trx_t {
 
 	ReadView	read_view;	/*!< consistent read view used in the
 					transaction, or NULL if not yet set */
-
-	UT_LIST_NODE_T(trx_t)
-			no_list;	/*!< Required during view creation
-					to check for the view limit for
-					transactions that are committing */
-
 	trx_lock_t	lock;		/*!< Information about the transaction
 					locks and state. Protected by
 					trx->mutex or lock_sys->mutex

storage/innobase/read/read0read.cc

@@ -218,14 +218,9 @@ MVCC::validate() const
 void ReadView::open(trx_t *trx)
 {
   ut_ad(mutex_own(&trx_sys.mutex));
-  trx_sys.snapshot_ids(trx, &m_ids, &m_low_limit_id);
-  m_low_limit_no= m_low_limit_id;
+  trx_sys.snapshot_ids(trx, &m_ids, &m_low_limit_id, &m_low_limit_no);
   m_up_limit_id= m_ids.empty() ? m_low_limit_id : m_ids.front();
   ut_ad(m_up_limit_id <= m_low_limit_id);
-
-  if (const trx_t *trx= UT_LIST_GET_FIRST(trx_sys.serialisation_list))
-    if (trx->no < m_low_limit_no)
-      m_low_limit_no= trx->no;
 }
 
 

storage/innobase/trx/trx0sys.cc

@@ -401,10 +401,7 @@ trx_sys_t::create()
 	ut_ad(!is_initialised());
 	m_initialised = true;
 	mutex_create(LATCH_ID_TRX_SYS, &mutex);
-
-	UT_LIST_INIT(serialisation_list, &trx_t::no_list);
 	UT_LIST_INIT(mysql_trx_list, &trx_t::mysql_trx_list);
-
 	rw_trx_hash.init();
 }
 
@@ -535,7 +532,6 @@ trx_sys_t::close()
 	}
 
 	ut_a(UT_LIST_GET_LEN(mysql_trx_list) == 0);
-	ut_a(UT_LIST_GET_LEN(serialisation_list) == 0);
 
 	/* We used placement new to create this mutex. Call the destructor. */
 	mutex_free(&mutex);

storage/innobase/trx/trx0trx.cc

@@ -1210,52 +1210,42 @@ trx_start_low(
 }
 
 /** Set the serialisation number for a persistent committed transaction.
-@param[in,out]	trx	committed transaction with persistent changes
-@param[in,out]	rseg	rollback segment for undo, or NULL */
+@param[in,out]	trx	committed transaction with persistent changes */
 static
 void
-trx_serialise(trx_t* trx, trx_rseg_t* rseg)
+trx_serialise(trx_t* trx)
 {
-	ut_ad(!rseg || rseg == trx->rsegs.m_redo.rseg);
+	trx_rseg_t *rseg = trx->rsegs.m_redo.rseg;
+	ut_ad(rseg);
+	ut_ad(mutex_own(&rseg->mutex));
 
-	mutex_enter(&trx_sys.mutex);
+	if (rseg->last_page_no == FIL_NULL) {
+		mutex_enter(&purge_sys->pq_mutex);
+	}
 
 	trx->no = trx_sys.get_new_trx_id();
-
-	/* Track the minimum serialisation number. */
-	UT_LIST_ADD_LAST(trx_sys.serialisation_list, trx);
+	my_atomic_store64_explicit(reinterpret_cast<int64*>
+				   (&trx->rw_trx_hash_element->no),
+				   trx->no, MY_MEMORY_ORDER_RELAXED);
 
 	/* If the rollack segment is not empty then the
 	new trx_t::no can't be less than any trx_t::no
 	already in the rollback segment. User threads only
 	produce events when a rollback segment is empty. */
-	if (rseg && rseg->last_page_no == FIL_NULL) {
+	if (rseg->last_page_no == FIL_NULL) {
 		TrxUndoRsegs	elem(trx->no);
 		elem.push_back(rseg);
 
-		mutex_enter(&purge_sys->pq_mutex);
-
-		/* This is to reduce the pressure on the trx_sys_t::mutex
-		though in reality it should make very little (read no)
-		difference because this code path is only taken when the
-		rbs is empty. */
-
-		mutex_exit(&trx_sys.mutex);
-
 		purge_sys->purge_queue.push(elem);
-
 		mutex_exit(&purge_sys->pq_mutex);
-	} else {
-		mutex_exit(&trx_sys.mutex);
 	}
 }
 
 /****************************************************************//**
 Assign the transaction its history serialisation number and write the
-update UNDO log record to the assigned rollback segment.
-@return true if a serialisation log was written */
+update UNDO log record to the assigned rollback segment. */
 static
-bool
+void
 trx_write_serialisation_history(
 /*============================*/
 	trx_t*		trx,	/*!< in/out: transaction */
@@ -1290,26 +1280,24 @@ trx_write_serialisation_history(
 	if (!rseg) {
 		ut_ad(!trx->rsegs.m_redo.undo);
 		ut_ad(!trx->rsegs.m_redo.old_insert);
-		return false;
+		return;
 	}
 
 	trx_undo_t*& undo = trx->rsegs.m_redo.undo;
 	trx_undo_t*& old_insert = trx->rsegs.m_redo.old_insert;
 
 	if (!undo && !old_insert) {
-		return false;
+		return;
 	}
 
 	ut_ad(!trx->read_only);
-	trx_rseg_t*	undo_rseg
-		= undo ? undo->rseg : old_insert ? old_insert->rseg : NULL;
 	ut_ad(!undo || undo->rseg == rseg);
 	ut_ad(!old_insert || old_insert->rseg == rseg);
 	mutex_enter(&rseg->mutex);
 
 	/* Assign the transaction serialisation number and add any
 	undo log to the purge queue. */
-	trx_serialise(trx, undo_rseg);
+	trx_serialise(trx);
 
 	/* It is not necessary to acquire trx->undo_mutex here because
 	only a single OS thread is allowed to commit this transaction.
@@ -1336,7 +1324,7 @@ trx_write_serialisation_history(
 #ifdef WITH_WSREP
 	    && !update_wsrep
 #endif
-	    ) return true;
+	    ) return;
 
 	buf_block_t* block = trx_sysf_get(mtr);
 #ifdef WITH_WSREP
@@ -1357,8 +1345,6 @@ trx_write_serialisation_history(
 
 		trx->mysql_log_file_name = NULL;
 	}
-
-	return(true);
 }
 
 /********************************************************************
@@ -1514,29 +1500,6 @@ trx_update_mod_tables_timestamp(
 	trx->mod_tables.clear();
 }
 
-/**
-Erase the transaction from running transaction lists and serialization
-list.
-@param[in] trx		Transaction to erase, must have an ID > 0
-@param[in] serialised	true if serialisation log was written */
-static
-void
-trx_erase_lists(
-	trx_t*	trx,
-	bool	serialised)
-{
-	ut_ad(trx->id > 0);
-
-	if (serialised) {
-		mutex_enter(&trx_sys.mutex);
-		UT_LIST_REMOVE(trx_sys.serialisation_list, trx);
-	} else {
-		mutex_enter(&trx_sys.mutex);
-	}
-	mutex_exit(&trx_sys.mutex);
-	trx_sys.deregister_rw(trx);
-}
-
 /****************************************************************//**
 Commits a transaction in memory. */
 static
@@ -1544,17 +1507,13 @@ void
 trx_commit_in_memory(
 /*=================*/
 	trx_t*		trx,	/*!< in/out: transaction */
-	const mtr_t*	mtr,	/*!< in: mini-transaction of
+	const mtr_t*	mtr)	/*!< in: mini-transaction of
 				trx_write_serialisation_history(), or NULL if
 				the transaction did not modify anything */
-	bool		serialised)
-				/*!< in: true if serialisation log was
-				written */
 {
 	trx->must_flush_log_later = false;
 	trx->read_view.close();
 
-
 	if (trx_is_autocommit_non_locking(trx)) {
 		ut_ad(trx->id == 0);
 		ut_ad(trx->read_only);
@@ -1590,9 +1549,9 @@ trx_commit_in_memory(
 	} else {
 		if (trx->id > 0) {
 			/* For consistent snapshot, we need to remove current
-			transaction from running transaction id list for mvcc
-			before doing commit and releasing locks. */
-			trx_erase_lists(trx, serialised);
+			transaction from rw_trx_hash before doing commit and
+			releasing locks. */
+			trx_sys.deregister_rw(trx);
 		}
 
 		lock_trx_release_locks(trx);
@@ -1769,13 +1728,11 @@ trx_commit_low(
 		}
 	}
 
-	bool	serialised;
-
 	if (mtr != NULL) {
 
 		mtr->set_sync();
 
-		serialised = trx_write_serialisation_history(trx, mtr);
+		trx_write_serialisation_history(trx, mtr);
 
 		/* The following call commits the mini-transaction, making the
 		whole transaction committed in the file-based world, at this
@@ -1803,9 +1760,6 @@ trx_commit_low(
 					DBUG_SUICIDE();
 				});
 		/*--------------*/
-
-	} else {
-		serialised = false;
 	}
 #ifndef DBUG_OFF
 	/* In case of this function is called from a stack executing
@@ -1821,7 +1775,7 @@ trx_commit_low(
 	}
 #endif
 
-	trx_commit_in_memory(trx, mtr, serialised);
+	trx_commit_in_memory(trx, mtr);
 }
 
 /****************************************************************//**