From 8c7a003761be87c9495d78efa27dc4440f5c782f Mon Sep 17 00:00:00 2001
From: sjaakola <seppo.jaakola@iki.fi>
Date: Fri, 1 Oct 2021 12:11:29 +0300
Subject: [PATCH] added new transaction state: s_waiting_for_replayers to make
 it posisble to diagnose from outside, if potential BF victim transaction is
 blocked in replayer wait loop (before entering commit phase). Also, added new
 forced state change to s_must_abort, if before_rollback() is used for a
 transaction in s_committing state. Such situation can happen, if BF abort
 happens for a victim, which has passed certification and moved to commit
 phase before the BF aborting happens.

---
 include/wsrep/transaction.hpp |  2 ++
 src/transaction.cpp           | 48 ++++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/include/wsrep/transaction.hpp b/include/wsrep/transaction.hpp
index 76835fd..d339626 100644
--- a/include/wsrep/transaction.hpp
+++ b/include/wsrep/transaction.hpp
@@ -58,6 +58,7 @@ namespace wsrep
             s_aborting,
             s_aborted,
             s_must_replay,
+            s_waiting_for_replayers,
             s_replaying
         };
         static const int n_states = s_replaying + 1;
@@ -299,6 +300,7 @@ namespace wsrep
         case wsrep::transaction::s_aborting: return "aborting";
         case wsrep::transaction::s_aborted: return "aborted";
         case wsrep::transaction::s_must_replay: return "must_replay";
+        case wsrep::transaction::s_waiting_for_replayers: return "waiting_for_replayers";
         case wsrep::transaction::s_replaying: return "replaying";
         }
         return "unknown";
diff --git a/src/transaction.cpp b/src/transaction.cpp
index 69380a4..ed2a107 100644
--- a/src/transaction.cpp
+++ b/src/transaction.cpp
@@ -656,10 +656,11 @@ int wsrep::transaction::before_rollback()
 {
     wsrep::unique_lock<wsrep::mutex> lock(client_state_.mutex());
     debug_log_state("before_rollback_enter");
-    assert(state() == s_executing ||
-           state() == s_preparing ||
-           state() == s_prepared ||
+    assert(state() == s_executing  ||
+           state() == s_preparing  ||
+           state() == s_prepared   ||
            state() == s_must_abort ||
+           state() == s_committing ||
            // Background rollbacker or rollback initiated from SE
            state() == s_aborting ||
            state() == s_cert_failed ||
@@ -672,6 +673,15 @@ int wsrep::transaction::before_rollback()
         {
             client_service_.debug_sync("wsrep_before_SR_rollback");
         }
+        /* this state change was added to support BF aborting
+           at later state (before prepare), where victim has certified
+           successfully, and state has already
+           propagated to committing phase
+        */
+        if (state() == s_committing) {
+	  state(lock, s_must_abort);
+	}
+
         switch (state())
         {
         case s_preparing:
@@ -984,6 +994,7 @@ bool wsrep::transaction::bf_abort(
         case s_preparing:
         case s_prepared:
         case s_certifying:
+        case s_waiting_for_replayers:
         case s_committing:
         {
             wsrep::seqno victim_seqno;
@@ -1306,22 +1317,22 @@ void wsrep::transaction::state(
            next_state == s_aborting);
 
     static const char allowed[n_states][n_states] =
-        { /*  ex pg pd ce co oc ct cf ma ab ad mr re */
-            { 0, 1, 0, 1, 0, 0, 0, 0, 1, 1, 0, 0, 0}, /* ex */
-            { 0, 0, 1, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0}, /* pg */
-            { 0, 0, 0, 1, 1, 0, 0, 0, 1, 1, 0, 0, 0}, /* pd */
-            { 1, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 0, 0}, /* ce */
-            { 0, 0, 0, 0, 0, 1, 1, 0, 1, 0, 0, 0, 0}, /* co */
-            { 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0}, /* oc */
-            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* ct */
-            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0}, /* cf */
-            { 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 1, 0}, /* ma */
-            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0}, /* ab */
-            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* ad */
-            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, /* mr */
-            { 0, 1, 0, 0, 1, 0, 1, 0, 0, 0, 1, 0, 0}  /* re */
+        { /*  ex pg pd ce co oc ct cf ma ab ad mr wr re */
+	    { 0, 1, 0, 1, 0, 0, 0, 0, 1, 1, 0, 0, 1, 0}, /* ex */
+            { 0, 0, 1, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0}, /* pg */
+            { 0, 0, 0, 1, 1, 0, 0, 0, 1, 1, 0, 0, 1, 0}, /* pd */
+            { 1, 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0}, /* ce */
+            { 0, 0, 0, 0, 0, 1, 1, 0, 1, 0, 0, 0, 0, 0}, /* co */
+            { 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0}, /* oc */
+            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* ct */
+            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0}, /* cf */
+            { 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 1, 0, 0}, /* ma */
+            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0}, /* ab */
+            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* ad */
+            { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, /* mr */
+            { 0, 0, 0, 1, 1, 0, 0, 0, 1, 1, 0, 0, 0, 1}, /* wr */
+            { 0, 1, 0, 0, 1, 0, 1, 0, 0, 0, 1, 0, 0, 0}  /* re */
         };
-
     if (!allowed[state_][next_state])
     {
         wsrep::log_debug() << "unallowed state transition for transaction "
@@ -1681,6 +1692,7 @@ int wsrep::transaction::certify_commit(
 {
     assert(lock.owns_lock());
     assert(active());
+    state(lock, s_waiting_for_replayers);
     client_service_.wait_for_replayers(lock);
 
     assert(lock.owns_lock());