MDEV-15326: Backport trx_t::is_referenced()

Backport the applicable part of Sergey Vojtovich's commit
0ca2ea1a6550bb718efc878ed451be69e8244cd4 from MariaDB Server 10.3.

trx reference counter was updated under mutex and read without any
protection. This is both slow and unsafe. Use atomic operations for
reference counter accesses.

8 files changed, 60 insertions, 56 deletions

diff --git a/storage/innobase/include/row0vers.h b/storage/innobase/include/row0vers.h
index b5279e5851b..327bb546356 100644
--- a/storage/innobase/include/row0vers.h
+++ b/storage/innobase/include/row0vers.h
@@ -44,7 +44,7 @@ index record.
 @param[in]	index	secondary index
 @param[in]	offsets	rec_get_offsets(rec, index)
 @return	the active transaction; state must be rechecked after
-trx_mutex_enter(), and trx_release_reference() must be invoked
+trx_mutex_enter(), and trx->release_reference() must be invoked
 @retval	NULL if the record was committed */
 trx_t*
 row_vers_impl_x_locked(
diff --git a/storage/innobase/include/trx0sys.ic b/storage/innobase/include/trx0sys.ic
index ac84404a397..0a4d583671f 100644
--- a/storage/innobase/include/trx0sys.ic
+++ b/storage/innobase/include/trx0sys.ic
@@ -346,7 +346,6 @@ inline trx_t* trx_rw_is_active(trx_id_t trx_id, bool* corrupt, bool ref_count)
 		mutex_enter(trx_mutex);
 		ut_ad(!trx_state_eq(trx, TRX_STATE_NOT_STARTED));
 		ut_ad(trx->id == trx_id);
-		ut_ad(trx->n_ref >= 0);
 		if (trx_state_eq(trx, TRX_STATE_COMMITTED_IN_MEMORY)) {
 			/* We have an early state check here to avoid
 			committer starvation in a wait loop for
@@ -357,7 +356,12 @@ inline trx_t* trx_rw_is_active(trx_id_t trx_id, bool* corrupt, bool ref_count)
 			rechecked by the caller after reacquiring the mutex. */
 			trx = NULL;
 		} else {
-			++trx->n_ref;
+			/* The reference could be safely incremented after
+			releasing one of trx_mutex or trx_sys->mutex.
+			Holding trx->mutex here may prevent a few false
+			references that could have a negative performance
+			impact on trx_commit_in_memory(). */
+			trx->reference();
 		}
 		mutex_exit(trx_mutex);
 	}
diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h
index 796e6b8bda6..1d71920d6c5 100644
--- a/storage/innobase/include/trx0trx.h
+++ b/storage/innobase/include/trx0trx.h
@@ -501,18 +501,6 @@ trx_set_rw_mode(
 	trx_t*		trx);
 
 /**
-Release the transaction. Decrease the reference count.
-@param trx Transaction that is being released */
-UNIV_INLINE
-void
-trx_release_reference(
-	trx_t*		trx);
-
-/**
-Check if the transaction is being referenced. */
-#define trx_is_referenced(t)	((t)->n_ref > 0)
-
-/**
 Transactions that aren't started by the MySQL server don't set
 the trx_t::mysql_thd field. For such transactions we set the lock
 wait timeout to 0 instead of the user configured value that comes
@@ -571,7 +559,7 @@ Check transaction state */
 	ut_ad(trx_state_eq((t), TRX_STATE_NOT_STARTED));		\
 	ut_ad(!(t)->id);						\
 	ut_ad(!(t)->has_logged());					\
-	ut_ad(!(t)->n_ref);						\
+	ut_ad(!(t)->is_referenced());					\
 	ut_ad(!MVCC::is_view_active((t)->read_view));			\
 	ut_ad((t)->lock.wait_thr == NULL);				\
 	ut_ad(UT_LIST_GET_LEN((t)->lock.trx_locks) == 0);		\
@@ -801,6 +789,19 @@ struct trx_rsegs_t {
 };
 
 struct trx_t {
+private:
+  /**
+    Count of references.
+
+    We can't release the locks nor commit the transaction until this reference
+    is 0. We can change the state to TRX_STATE_COMMITTED_IN_MEMORY to signify
+    that it is no longer "active".
+  */
+
+  int32_t n_ref;
+
+
+public:
 	TrxMutex	mutex;		/*!< Mutex protecting the fields
 					state and lock (except some fields
 					of lock, which are protected by
@@ -1100,14 +1101,6 @@ struct trx_t {
 	const char*	start_file;	/*!< Filename where it was started */
 #endif /* UNIV_DEBUG */
 
-	lint		n_ref;		/*!< Count of references, protected
-					by trx_t::mutex. We can't release the
-					locks nor commit the transaction until
-					this reference is 0.  We can change
-					the state to COMMITTED_IN_MEMORY to
-					signify that it is no longer
-					"active". */
-
 	XID*		xid;		/*!< X/Open XA transaction
 					identification to identify a
 					transaction branch */
@@ -1184,6 +1177,33 @@ public:
   /** Release any explicit locks of a committing transaction. */
   inline void release_locks();
 
+
+  bool is_referenced()
+  {
+    return my_atomic_load32_explicit(&n_ref, MY_MEMORY_ORDER_RELAXED) > 0;
+  }
+
+
+  void reference()
+  {
+#ifdef UNIV_DEBUG
+  int32_t old_n_ref=
+#endif
+    my_atomic_add32_explicit(&n_ref, 1, MY_MEMORY_ORDER_RELAXED);
+    ut_ad(old_n_ref >= 0);
+  }
+
+
+  void release_reference()
+  {
+#ifdef UNIV_DEBUG
+  int32_t old_n_ref=
+#endif
+    my_atomic_add32_explicit(&n_ref, -1, MY_MEMORY_ORDER_RELAXED);
+    ut_ad(old_n_ref > 0);
+  }
+
+
 private:
 	/** Assign a rollback segment for modifying temporary tables.
 	@return the assigned rollback segment */
diff --git a/storage/innobase/include/trx0trx.ic b/storage/innobase/include/trx0trx.ic
index 64681e00a62..4a5b1ba717f 100644
--- a/storage/innobase/include/trx0trx.ic
+++ b/storage/innobase/include/trx0trx.ic
@@ -211,23 +211,6 @@ ok:
 }
 
 /**
-Release the transaction. Decrease the reference count.
-@param trx Transaction that is being released */
-UNIV_INLINE
-void
-trx_release_reference(
-	trx_t*		trx)
-{
-	trx_mutex_enter(trx);
-
-	ut_ad(trx->n_ref > 0);
-	--trx->n_ref;
-
-	trx_mutex_exit(trx);
-}
-
-
-/**
 @param trx		Get the active view for this transaction, if one exists
 @return the transaction's read view or NULL if one not assigned. */
 UNIV_INLINE
diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
index 49c0e1da743..b7e17097546 100644
--- a/storage/innobase/lock/lock0lock.cc
+++ b/storage/innobase/lock/lock0lock.cc
@@ -5772,10 +5772,12 @@ lock_rec_convert_impl_to_expl_for_trx(
 	trx_t*			trx,	/*!< in/out: active transaction */
 	ulint			heap_no)/*!< in: rec heap number to lock */
 {
-	DEBUG_SYNC_C("before_lock_rec_convert_impl_to_expl_for_trx");
+	ut_ad(page_rec_is_leaf(rec));
 
+	DEBUG_SYNC_C("before_lock_rec_convert_impl_to_expl_for_trx");
 	lock_mutex_enter();
 	trx_mutex_enter(trx);
+	ut_ad(trx->is_referenced());
 	ut_ad(!trx_state_eq(trx, TRX_STATE_NOT_STARTED));
 
 	if (!trx_state_eq(trx, TRX_STATE_COMMITTED_IN_MEMORY)
@@ -5786,9 +5788,8 @@ lock_rec_convert_impl_to_expl_for_trx(
 	}
 
 	lock_mutex_exit();
-	ut_ad(trx->n_ref > 0);
-	--trx->n_ref;
 	trx_mutex_exit(trx);
+	trx->release_reference();
 
 	DEBUG_SYNC_C("after_lock_rec_convert_impl_to_expl_for_trx");
 }
@@ -5830,7 +5831,7 @@ lock_rec_convert_impl_to_expl(
 	if (trx != 0) {
 		ulint	heap_no = page_rec_get_heap_no(rec);
 
-		ut_ad(trx_is_referenced(trx));
+		ut_ad(trx->is_referenced());
 
 		/* If the transaction is still active and has no
 		explicit x-lock set on the record, set one for it.
diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc
index 19130576425..dae51354196 100644
--- a/storage/innobase/row/row0sel.cc
+++ b/storage/innobase/row/row0sel.cc
@@ -5087,7 +5087,7 @@ wrong_offs:
 					   rec, index, offsets)) {
 				/* The record belongs to an active
 				transaction. We must acquire a lock. */
-				trx_release_reference(trx);
+				trx->release_reference();
 			} else {
 				/* The secondary index record does not
 				point to a delete-marked clustered index
diff --git a/storage/innobase/row/row0vers.cc b/storage/innobase/row/row0vers.cc
index 533d6df1321..03e586d4bee 100644
--- a/storage/innobase/row/row0vers.cc
+++ b/storage/innobase/row/row0vers.cc
@@ -77,7 +77,7 @@ index record.
 @param[in]	offsets		rec_get_offsets(rec, index)
 @param[in,out]	mtr		mini-transaction
 @return	the active transaction; state must be rechecked after
-trx_mutex_enter(), and trx_release_reference() must be invoked
+trx_mutex_enter(), and trx->release_reference() must be invoked
 @retval	NULL if the record was committed */
 UNIV_INLINE
 trx_t*
@@ -213,7 +213,7 @@ row_vers_impl_x_locked_low(
 				created with a clear delete-mark flag.
 				(We never insert a delete-marked record.) */
 not_locked:
-				trx_release_reference(trx);
+				trx->release_reference();
 				trx = 0;
 			}
 
@@ -362,7 +362,7 @@ index record.
 @param[in]	index	secondary index
 @param[in]	offsets	rec_get_offsets(rec, index)
 @return	the active transaction; state must be rechecked after
-trx_mutex_enter(), and trx_release_reference() must be invoked
+trx_mutex_enter(), and trx->release_reference() must be invoked
 @retval	NULL if the record was committed */
 trx_t*
 row_vers_impl_x_locked(
@@ -408,7 +408,7 @@ row_vers_impl_x_locked(
 		trx = row_vers_impl_x_locked_low(
 			clust_rec, clust_index, rec, index, offsets, &mtr);
 
-		ut_ad(trx == 0 || trx_is_referenced(trx));
+		ut_ad(trx == 0 || trx->is_referenced());
 	}
 
 	mtr_commit(&mtr);
diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc
index 40dcc37b30b..1dcca7c1f72 100644
--- a/storage/innobase/trx/trx0trx.cc
+++ b/storage/innobase/trx/trx0trx.cc
@@ -551,7 +551,7 @@ inline void trx_t::commit_state()
   background thread. To avoid this race we unconditionally unset
   the is_recovered flag. */
   is_recovered= false;
-  ut_ad(id || !trx_is_referenced(this));
+  ut_ad(id || !is_referenced());
 }
 
 /** Release any explicit locks of a committing transaction. */
@@ -1705,13 +1705,9 @@ trx_commit_in_memory(
 			/* Wait for any implicit-to-explicit lock
 			conversions to cease, so that there will be no
 			race condition in lock_release(). */
-			trx_mutex_enter(trx);
-			while (UNIV_UNLIKELY(trx_is_referenced(trx))) {
-				trx_mutex_exit(trx);
+			while (UNIV_UNLIKELY(trx->is_referenced())) {
 				ut_delay(srv_spin_wait_delay);
-				trx_mutex_enter(trx);
 			}
-			trx_mutex_exit(trx);
 
 			trx->release_locks();
 			trx->id = 0;