Cleanup: Reduce trx_t::mutex hold timebb-10.5-MDEV-24167-2

5 files changed, 49 insertions, 85 deletions

diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
index 853c17a0f41..529f36b3a29 100644
--- a/storage/innobase/lock/lock0lock.cc
+++ b/storage/innobase/lock/lock0lock.cc
@@ -2034,8 +2034,6 @@ lock_rec_cancel(
 /*============*/
 	lock_t*	lock)	/*!< in: waiting record lock request */
 {
-	que_thr_t*	thr;
-
 	ut_ad(lock_mutex_own());
 	ut_ad(lock_get_type_low(lock) == LOCK_REC);
 
@@ -2046,17 +2044,13 @@ lock_rec_cancel(
 
 	lock_reset_lock_and_trx_wait(lock);
 
-	/* The following function releases the trx from lock wait */
-
-	trx_mutex_enter(lock->trx);
-
-	thr = que_thr_end_lock_wait(lock->trx);
-
-	if (thr != NULL) {
+	/* The following releases the trx from lock wait */
+	trx_t *trx = lock->trx;
+	trx_mutex_enter(trx);
+	if (que_thr_t* thr = que_thr_end_lock_wait(trx)) {
 		lock_wait_release_thread_if_suspended(thr);
 	}
-
-	trx_mutex_exit(lock->trx);
+	trx_mutex_exit(trx);
 }
 
 static void lock_grant_and_move_on_page(ulint rec_fold, const page_id_t id)
@@ -4024,7 +4018,6 @@ lock_rec_unlock(
 	heap_no = page_rec_get_heap_no(rec);
 
 	lock_mutex_enter();
-	trx_mutex_enter(trx);
 
 	first_lock = lock_rec_get_first(&lock_sys.rec_hash, block, heap_no);
 
@@ -4039,7 +4032,6 @@ lock_rec_unlock(
 	}
 
 	lock_mutex_exit();
-	trx_mutex_exit(trx);
 
 	{
 		ib::error	err;
@@ -4085,7 +4077,6 @@ released:
 	}
 
 	lock_mutex_exit();
-	trx_mutex_exit(trx);
 }
 
 #ifdef UNIV_DEBUG
@@ -4201,7 +4192,8 @@ lock_trx_table_locks_remove(
 	ut_ad(lock_mutex_own());
 
 	/* It is safe to read this because we are holding the lock mutex */
-	if (!trx->lock.cancel) {
+	const bool have_mutex = trx->lock.cancel;
+	if (!have_mutex) {
 		trx_mutex_enter(trx);
 	} else {
 		ut_ad(trx_mutex_own(trx));
@@ -4218,7 +4210,7 @@ lock_trx_table_locks_remove(
 		if (lock == lock_to_remove) {
 			*it = NULL;
 
-			if (!trx->lock.cancel) {
+			if (!have_mutex) {
 				trx_mutex_exit(trx);
 			}
 
@@ -4226,10 +4218,6 @@ lock_trx_table_locks_remove(
 		}
 	}
 
-	if (!trx->lock.cancel) {
-		trx_mutex_exit(trx);
-	}
-
 	/* Lock must exist in the vector. */
 	ut_error;
 }
diff --git a/storage/innobase/lock/lock0prdt.cc b/storage/innobase/lock/lock0prdt.cc
index 1eb96a0dcf0..9c6fd424005 100644
--- a/storage/innobase/lock/lock0prdt.cc
+++ b/storage/innobase/lock/lock0prdt.cc
@@ -920,20 +920,14 @@ lock_place_prdt_page_lock(
 	trx_t*		trx = thr_get_trx(thr);
 
 	if (lock != NULL) {
-
-		trx_mutex_enter(trx);
-
 		/* Find a matching record lock owned by this transaction. */
 
 		while (lock != NULL && lock->trx != trx) {
-
 			lock = lock_rec_get_next_on_page_const(lock);
 		}
 
 		ut_ad(lock == NULL || lock->type_mode == (mode | LOCK_REC));
 		ut_ad(lock == NULL || lock_rec_get_n_bits(lock) != 0);
-
-		trx_mutex_exit(trx);
 	}
 
 	if (lock == NULL) {
diff --git a/storage/innobase/lock/lock0wait.cc b/storage/innobase/lock/lock0wait.cc
index b606228b1fe..14c93ad9014 100644
--- a/storage/innobase/lock/lock0wait.cc
+++ b/storage/innobase/lock/lock0wait.cc
@@ -459,24 +459,21 @@ lock_wait_check_and_cancel(
 
 		lock_mutex_enter();
 
-		trx_mutex_enter(trx);
-
 		if (trx->lock.wait_lock != NULL) {
-
 			ut_a(trx->lock.que_state == TRX_QUE_LOCK_WAIT);
 
 #ifdef WITH_WSREP
                         if (!wsrep_is_BF_lock_timeout(trx)) {
 #endif /* WITH_WSREP */
+				mutex_enter(&trx->mutex);
 				lock_cancel_waiting_and_release(trx->lock.wait_lock);
+				mutex_exit(&trx->mutex);
 #ifdef WITH_WSREP
                         }
 #endif /* WITH_WSREP */
 		}
 
 		lock_mutex_exit();
-
-		trx_mutex_exit(trx);
 	}
 }
 
diff --git a/storage/innobase/que/que0que.cc b/storage/innobase/que/que0que.cc
index 18fe7626d57..ce6e494cdd1 100644
--- a/storage/innobase/que/que0que.cc
+++ b/storage/innobase/que/que0que.cc
@@ -590,22 +590,18 @@ que_thr_node_step(
 		return(thr);
 	}
 
-	trx_mutex_enter(thr_get_trx(thr));
+	auto mutex = &thr->graph->trx->mutex;
 
-	if (que_thr_peek_stop(thr)) {
+	mutex_enter(mutex);
 
-		trx_mutex_exit(thr_get_trx(thr));
-
-		return(thr);
+	if (!que_thr_peek_stop(thr)) {
+		/* Thread execution completed */
+		thr->state = QUE_THR_COMPLETED;
+		thr = NULL;
 	}
 
-	/* Thread execution completed */
-
-	thr->state = QUE_THR_COMPLETED;
-
-	trx_mutex_exit(thr_get_trx(thr));
-
-	return(NULL);
+	mutex_exit(mutex);
+	return(thr);
 }
 
 /**********************************************************************//**
@@ -728,27 +724,24 @@ que_thr_stop_for_mysql(
 	trx_mutex_enter(trx);
 
 	if (thr->state == QUE_THR_RUNNING) {
-
-		if (trx->error_state != DB_SUCCESS
-		    && trx->error_state != DB_LOCK_WAIT) {
-
-			/* Error handling built for the MySQL interface */
+		switch (trx->error_state) {
+		default:
+			/* Error handling built for the MariaDB interface */
 			thr->state = QUE_THR_COMPLETED;
-		} else {
+			break;
+		case DB_SUCCESS:
+		case DB_LOCK_WAIT:
 			/* It must have been a lock wait but the lock was
 			already released, or this transaction was chosen
 			as a victim in selective deadlock resolution */
-
-			trx_mutex_exit(trx);
-
-			return;
+			goto func_exit;
 		}
 	}
 
 	ut_ad(thr->is_active);
 	ut_d(thr->set_active(false));
 	thr->is_active= false;
-
+func_exit:
 	trx_mutex_exit(trx);
 }
 
@@ -1009,28 +1002,23 @@ que_run_threads_low(
 		next_thr = que_thr_step(thr);
 		/*-------------------------*/
 
-		trx_mutex_enter(trx);
-
-		ut_a(next_thr == NULL || trx->error_state == DB_SUCCESS);
-
-		if (next_thr != thr) {
-			ut_a(next_thr == NULL);
-
+		if (next_thr) {
+			ut_a(trx->error_state == DB_SUCCESS);
+			ut_a(next_thr == thr);
+		} else {
 			/* This can change next_thr to a non-NULL value
 			if there was a lock wait that already completed. */
 
+			mutex_enter(&trx->mutex);
 			que_thr_dec_refer_count(thr, &next_thr);
+			mutex_exit(&trx->mutex);
 
 			if (next_thr != NULL) {
-
 				thr = next_thr;
 			}
 		}
 
 		ut_ad(trx == thr_get_trx(thr));
-
-		trx_mutex_exit(trx);
-
 	} while (next_thr != NULL);
 }
 
@@ -1049,6 +1037,12 @@ loop:
 	que_run_threads_low(thr);
 
 	switch (thr->state) {
+	default:
+		ut_error;
+	case QUE_THR_COMPLETED:
+	case QUE_THR_COMMAND_WAIT:
+		/* Do nothing */
+		break;
 
 	case QUE_THR_RUNNING:
 		/* There probably was a lock wait, but it already ended
@@ -1058,30 +1052,21 @@ loop:
 
 	case QUE_THR_LOCK_WAIT:
 		lock_wait_suspend_thread(thr);
+		trx_t* trx = thr->graph->trx;
 
-		trx_mutex_enter(thr_get_trx(thr));
-
-		ut_a(thr_get_trx(thr)->id != 0);
-
-		if (thr_get_trx(thr)->error_state != DB_SUCCESS) {
+		trx_mutex_enter(trx);
+		ut_ad(trx->id);
+		const dberr_t err = trx->error_state;
+		if (err != DB_SUCCESS) {
 			/* thr was chosen as a deadlock victim or there was
 			a lock wait timeout */
-
 			que_thr_dec_refer_count(thr, NULL);
-			trx_mutex_exit(thr_get_trx(thr));
-			break;
 		}
+		trx_mutex_exit(trx);
 
-		trx_mutex_exit(thr_get_trx(thr));
-		goto loop;
-
-	case QUE_THR_COMPLETED:
-	case QUE_THR_COMMAND_WAIT:
-		/* Do nothing */
-		break;
-
-	default:
-		ut_error;
+		if (err == DB_SUCCESS) {
+			goto loop;
+		}
 	}
 }
 
diff --git a/storage/innobase/trx/trx0roll.cc b/storage/innobase/trx/trx0roll.cc
index dc3dd51bb89..0c6f6ef55f3 100644
--- a/storage/innobase/trx/trx0roll.cc
+++ b/storage/innobase/trx/trx0roll.cc
@@ -959,14 +959,14 @@ trx_rollback_step(
 
 		trx = thr_get_trx(thr);
 
-		trx_mutex_enter(trx);
-
 		node->state = ROLL_NODE_WAIT;
 
 		ut_a(node->undo_thr == NULL);
 
 		roll_limit = node->savept ? node->savept->least_undo_no : 0;
 
+		trx_mutex_enter(trx);
+
 		trx_commit_or_rollback_prepare(trx);
 
 		node->undo_thr = trx_rollback_start(trx, roll_limit);