From 8c2daa7e3d89887034204a5ea60bed9a34e5249f Mon Sep 17 00:00:00 2001
From: Teemu Ollakka <teemu.ollakka@galeracluster.com>
Date: Fri, 15 Feb 2019 11:44:17 +0200
Subject: [PATCH] Handle BF abort during fragment removal

Fragment removal for SR transaction is done in transaction context
to make it atomic. However, this means that if the BF abort arrives
during fragment removal, the transaction may be rolled back inside
client_service::remove_fragments(), and client_state::after_prepare()
is left in aborted state. This case was not handled in the
post condition checks of after_prepare().

Fixed assertion in after_prepare() and implemented unit test.
---
 src/transaction.cpp           |  3 ++-
 test/mock_client_state.hpp    | 16 +++++++++++++++-
 test/transaction_test_2pc.cpp | 20 ++++++++++++++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/transaction.cpp b/src/transaction.cpp
index cb34682..b5fa771 100644
--- a/src/transaction.cpp
+++ b/src/transaction.cpp
@@ -321,7 +321,8 @@ int wsrep::transaction::before_prepare(
     assert(state() == s_preparing ||
            (ret && (state() == s_must_abort ||
                     state() == s_must_replay ||
-                    state() == s_cert_failed)));
+                    state() == s_cert_failed ||
+                    state() == s_aborted)));
     debug_log_state("before_prepare_leave");
     return ret;
 }
diff --git a/test/mock_client_state.hpp b/test/mock_client_state.hpp
index efb8094..af25fd5 100644
--- a/test/mock_client_state.hpp
+++ b/test/mock_client_state.hpp
@@ -65,6 +65,7 @@ namespace wsrep
               // , fail_next_applying_()
               // , fail_next_toi_()
             , bf_abort_during_wait_()
+            , bf_abort_during_fragment_removal_()
             , error_during_prepare_data_()
             , killed_before_certify_()
             , sync_point_enabled_()
@@ -83,7 +84,19 @@ namespace wsrep
 
         void emergency_shutdown() WSREP_OVERRIDE { ++aborts_; }
 
-        int remove_fragments() WSREP_OVERRIDE { return 0; }
+        int remove_fragments() WSREP_OVERRIDE
+        {
+            if (bf_abort_during_fragment_removal_)
+            {
+                client_state_.before_rollback();
+                client_state_.after_rollback();
+                return 1;
+            }
+            else
+            {
+                return 0;
+            }
+        }
         void will_replay() WSREP_OVERRIDE { }
 
         enum wsrep::provider::status
@@ -165,6 +178,7 @@ namespace wsrep
         // bool fail_next_applying_;
         // bool fail_next_toi_;
         bool bf_abort_during_wait_;
+        bool bf_abort_during_fragment_removal_;
         bool error_during_prepare_data_;
         bool killed_before_certify_;
         std::string sync_point_enabled_;
diff --git a/test/transaction_test_2pc.cpp b/test/transaction_test_2pc.cpp
index b6c30c1..63645d3 100644
--- a/test/transaction_test_2pc.cpp
+++ b/test/transaction_test_2pc.cpp
@@ -240,6 +240,26 @@ BOOST_FIXTURE_TEST_CASE(transaction_streaming_2pc_commit_two_statements,
     BOOST_REQUIRE(sc.provider().commit_fragments() == 1);
 }
 
+//
+// Fragments are removed in before_prepare in running transaction context.
+// However, the BF abort may arrive during this removal and the
+// client_service::remove_fragments() may roll back the transaction
+// internally. This will cause the transaction to leave before_prepare()
+// in aborted state.
+//
+BOOST_FIXTURE_TEST_CASE(transaction_streaming_2pc_bf_abort_during_fragment_removal,
+                        streaming_client_fixture_row)
+{
+    BOOST_REQUIRE(cc.start_transaction(wsrep::transaction_id(1)) == 0);
+    BOOST_REQUIRE(cc.after_row() == 0);
+    BOOST_REQUIRE(tc.streaming_context().fragments_certified() == 1);
+    cc.bf_abort_during_fragment_removal_ = true;
+    BOOST_REQUIRE(cc.before_prepare());
+    BOOST_REQUIRE(tc.state() == wsrep::transaction::s_aborted);
+    BOOST_REQUIRE(cc.after_statement());
+    BOOST_REQUIRE(tc.active() == false);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 //                              APPLYING                                     //
 ///////////////////////////////////////////////////////////////////////////////