MDEV-24923 : Port selected Galera conflict resolution changes from 10.6bb-10.3-MDEV-23923

Add condition on trx->state == TRX_STATE_COMMITTED_IN_MEMORY in order to
avoid unnecessary work. If a transaction has already been committed or
rolled back, it will release its locks in lock_release() and let
the waiting thread(s) continue execution.

Let BF wait on lock_rec_has_to_wait and if necessary other BF
is replayed.

bg_wsrep_kill_trx
  Make sure victim_trx is found and check also its state. If
  state is TRX_STATE_COMMITTED_IN_MEMORY we can return.

wsrep_assert_no_bf_bf_wait
  Check both bf trx and conflicting trx state and if one of them
  is TRX_STATE_COMMITTED_IN_MEMORY we can let BF wait for locks
  to be released.

lock_rec_has_to_wait
  We very well can let bf to wait normally as other BF will be
  replayed in case of conflict. For debug builds we will do
  additional sanity checks to catch unsupported bf wait if any.

wsrep_kill_victim
  Check is victim already in TRX_STATE_COMMITTED_IN_MEMORY state and
  if it is we can return.

lock_rec_dequeue_from_page
lock_rec_unlock
  Remove unnecessary wsrep_assert_no_bf_bf_wait function call.
  We can very well let BF wait here.

4 files changed, 84 insertions, 64 deletions

diff --git a/sql/wsrep_thd.cc b/sql/wsrep_thd.cc
index 640401cb1bf..69b0d2e0bcb 100644
--- a/sql/wsrep_thd.cc
+++ b/sql/wsrep_thd.cc
@@ -896,7 +896,7 @@ void wsrep_report_bf_lock_wait(THD *thd,
 {
   if (thd)
   {
-    WSREP_ERROR("Thread %s trx_id: %llu thread: %ld "
+    WSREP_INFO("Thread %s trx_id: %llu thread: %ld "
                 "seqno: %lld query_state: %s conf_state: %s exec_mode: %s "
                 "applier: %d query: %s",
                 wsrep_thd_is_BF(thd, false) ? "BF" : "normal",
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 5074855af64..a6b5d90cdbf 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -18801,14 +18801,25 @@ static void bg_wsrep_kill_trx(
 	if (thd) {
 		wsrep_thd_LOCK(thd);
 		victim_trx = thd_to_trx(thd);
-		lock_mutex_enter();
-		trx_mutex_enter(victim_trx);
-		wsrep_thd_UNLOCK(thd);
-		if (victim_trx->id != arg->trx_id)
-		{
-			trx_mutex_exit(victim_trx);
-			lock_mutex_exit();
-			victim_trx = NULL;
+		/* Victim trx might not exists e.g. on MDL-conflict. */
+		if (victim_trx) {
+			lock_mutex_enter();
+			trx_mutex_enter(victim_trx);
+			wsrep_thd_UNLOCK(thd);
+			if (victim_trx->id != arg->trx_id ||
+			    victim_trx->state == TRX_STATE_COMMITTED_IN_MEMORY)
+			{
+				/* Victim was meanwhile rolled back or
+				committed */
+				trx_mutex_exit(victim_trx);
+				lock_mutex_exit();
+				victim_trx = NULL;
+				wsrep_thd_kill_UNLOCK(thd);
+			}
+		} else {
+			/* find_thread_by_id locked
+			THD::LOCK_thd_kill */
+			wsrep_thd_UNLOCK(thd);
 			wsrep_thd_kill_UNLOCK(thd);
 		}
 	}
diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h
index c1572a0d07f..69db3a99722 100644
--- a/storage/innobase/include/trx0trx.h
+++ b/storage/innobase/include/trx0trx.h
@@ -1122,7 +1122,20 @@ public:
   /** Free the memory to trx_pools */
   void free();
 
-
+#if defined(WITH_WSREP) && defined(UNIV_DEBUG)
+  inline const char* trx_state_str()
+  {
+    switch(state)
+    {
+      case TRX_STATE_NOT_STARTED : return "TRX_STATE_NOT_STARTED"; break;
+      case TRX_STATE_ACTIVE : return "TRX_STATE_ACTIVE"; break;
+      case TRX_STATE_PREPARED : return "TRX_STATE_PREPARED"; break;
+      case TRX_STATE_PREPARED_RECOVERED : return "TRX_STATE_PREPARED"; break;
+      case TRX_STATE_COMMITTED_IN_MEMORY : return "TRX_STATE_COMMITTED_IN_MEMORY"; break;
+      default: ut_error; return " "; break; // for compiler
+    }
+  }
+#endif /* WITH_WSREP && UNIV_DEBUG */
 private:
 	/** Assign a rollback segment for modifying temporary tables.
 	@return the assigned rollback segment */
diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
index ee57a493119..1d9d5c5eae7 100644
--- a/storage/innobase/lock/lock0lock.cc
+++ b/storage/innobase/lock/lock0lock.cc
@@ -637,6 +637,7 @@ lock_rec_get_insert_intention(
 	return(lock->type_mode & LOCK_INSERT_INTENTION);
 }
 
+#ifdef UNIV_DEBUG
 #ifdef WITH_WSREP
 /** Check if both conflicting lock and other record lock are brute force
 (BF). This case is a bug so report lock information and wsrep state.
@@ -647,7 +648,7 @@ lock_rec_get_insert_intention(
 static void wsrep_assert_no_bf_bf_wait(
 	const lock_t* lock_rec1,
 	const lock_t* lock_rec2,
-	const trx_t* trx1)
+	trx_t* trx1)
 {
 	ut_ad(!lock_rec1 || lock_get_type_low(lock_rec1) == LOCK_REC);
 	ut_ad(lock_get_type_low(lock_rec2) == LOCK_REC);
@@ -664,6 +665,22 @@ static void wsrep_assert_no_bf_bf_wait(
 	if (UNIV_LIKELY(!wsrep_thd_is_BF(lock_rec2->trx->mysql_thd, FALSE)))
 		return;
 
+	ut_ad(!trx_mutex_own(trx1));
+	ut_ad(!trx_mutex_own(lock_rec2->trx));
+	trx_mutex_enter(trx1);
+	trx_mutex_enter(lock_rec2->trx);
+
+
+	/* If transaction is already committed in memory we can wait */
+	if (trx_state_eq(trx1, TRX_STATE_COMMITTED_IN_MEMORY) ||
+	    trx_state_eq(lock_rec2->trx, TRX_STATE_COMMITTED_IN_MEMORY)) {
+		trx_mutex_exit(trx1);
+		trx_mutex_exit(lock_rec2->trx);
+		return;
+	}
+	trx_mutex_exit(trx1);
+	trx_mutex_exit(lock_rec2->trx);
+
 	/* if BF - BF order is honored, we can keep trx1 waiting for the lock */
 	if (wsrep_trx_order_before(trx1->mysql_thd, lock_rec2->trx->mysql_thd))
 		return;
@@ -674,6 +691,10 @@ static void wsrep_assert_no_bf_bf_wait(
 	if (wsrep_trx_is_aborting(lock_rec2->trx->mysql_thd))
 		return;
 
+	WSREP_INFO("Trx state trx_id: " TRX_ID_FMT " state: %s"
+		   " trx_id2: " TRX_ID_FMT " state: %s",
+		   trx1->id, trx1->trx_state_str(),
+		   lock_rec2->trx->id, lock_rec2->trx->trx_state_str());
 	mtr_t mtr;
 
 	if (lock_rec1) {
@@ -702,6 +723,7 @@ static void wsrep_assert_no_bf_bf_wait(
 	ut_error;
 }
 #endif /* WITH_WSREP */
+#endif /* UNIV_DEBUG */
 
 /*********************************************************************//**
 Checks if a lock request for a new lock has to wait for request lock2.
@@ -824,9 +846,13 @@ lock_rec_has_to_wait(
 			return false;
 		}
 
-		/* There should not be two conflicting locks that are
-		brute force. If there is it is a bug. */
-		wsrep_assert_no_bf_bf_wait(NULL, lock2, trx);
+#ifdef UNIV_DEBUG
+		/* We very well can let bf to wait normally as other
+		BF will be replayed in case of conflict. For debug
+		builds we will do additional sanity checks to catch
+		unsupported bf wait if any. */
+		wsrep_assert_no_bf_bf_wait(NULL, lock2, const_cast<trx_t*>(trx));
+#endif
 #endif /* WITH_WSREP */
 
 	return true;
@@ -1104,57 +1130,35 @@ wsrep_kill_victim(
 {
 	ut_ad(lock_mutex_own());
 	ut_ad(trx_mutex_own(lock->trx));
+	ut_ad(trx->is_wsrep());
+	ut_ad(lock->trx != trx);
 
-	/* quit for native mysql */
-	if (!trx->is_wsrep()) return;
+	if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
+		return;
+	}
 
-	my_bool bf_this  = wsrep_thd_is_BF(trx->mysql_thd, FALSE);
-	my_bool bf_other = wsrep_thd_is_BF(lock->trx->mysql_thd, FALSE);
-	mtr_t mtr;
+	trx_t* lock_trx = lock->trx;
+
+	if (lock_trx->state == TRX_STATE_COMMITTED_IN_MEMORY
+	    || lock_trx->lock.was_chosen_as_deadlock_victim) {
+              return;
+	}
+
+	my_bool bf_other = wsrep_thd_is_BF(lock_trx->mysql_thd, FALSE);
 
-	if ((bf_this && !bf_other) ||
-		(bf_this && bf_other && wsrep_trx_order_before(
-			trx->mysql_thd, lock->trx->mysql_thd))) {
+	if ((!bf_other) ||
+	    (wsrep_trx_order_before(
+		    trx->mysql_thd, lock_trx->mysql_thd))) {
 
 		if (lock->trx->lock.que_state == TRX_QUE_LOCK_WAIT) {
 			if (UNIV_UNLIKELY(wsrep_debug)) {
-				ib::info() << "WSREP: BF victim waiting\n";
+				WSREP_INFO("BF victim waiting");
 			}
 			/* cannot release lock, until our lock
 			is in the queue*/
-		} else if (lock->trx != trx) {
-			if (wsrep_log_conflicts) {
-				if (bf_this) {
-					ib::info() << "*** Priority TRANSACTION:";
-				} else {
-					ib::info() << "*** Victim TRANSACTION:";
-				}
-
-				trx_print_latched(stderr, trx, 3000);
-
-				if (bf_other) {
-					ib::info() << "*** Priority TRANSACTION:";
-				} else {
-					ib::info() << "*** Victim TRANSACTION:";
-				}
-                                trx_print_latched(stderr, lock->trx, 3000);
-
-				ib::info() << "*** WAITING FOR THIS LOCK TO BE GRANTED:";
-
-				if (lock_get_type(lock) == LOCK_REC) {
-					lock_rec_print(stderr, lock, mtr);
-				} else {
-					lock_table_print(stderr, lock);
-				}
-
-				ib::info() << " SQL1: "
-					   << wsrep_thd_query(trx->mysql_thd);
-				ib::info() << " SQL2: "
-					   << wsrep_thd_query(lock->trx->mysql_thd);
-			}
-
-			wsrep_innobase_kill_one_trx(trx->mysql_thd,
-						    trx, lock->trx, TRUE);
+		} else {
+			wsrep_innobase_kill_one_trx(trx->mysql_thd, trx,
+						    lock_trx, true);
 		}
 	}
 }
@@ -2248,10 +2252,6 @@ static void lock_rec_dequeue_from_page(lock_t* in_lock)
 				/* Grant the lock */
 				ut_ad(lock->trx != in_lock->trx);
 				lock_grant(lock);
-#ifdef WITH_WSREP
-			} else {
-				wsrep_assert_no_bf_bf_wait(c, lock, c->trx);
-#endif /* WITH_WSREP */
 			}
 		}
 	} else {
@@ -4204,10 +4204,6 @@ released:
 				/* Grant the lock */
 				ut_ad(trx != lock->trx);
 				lock_grant(lock);
-#ifdef WITH_WSREP
-			} else {
-				wsrep_assert_no_bf_bf_wait(c, lock, c->trx);
-#endif /* WITH_WSREP */
 			}
 		}
 	} else {