Ensure that client_service::will_replay() is called.

Modified tests to verify that client_service::will_replay() is
called whenever it is determined that the transaction must replay.

Added a test to verify behavior when provider::commit_order_enter()
returns BF abort error.

Moved call to client_service::will_replay() into transaction::state()
to ensure that it is always called when shift to s_must_replay
happens.

src/transaction.cpp

@@ -1142,6 +1142,11 @@ void wsrep::transaction::state(
         state_hist_.erase(state_hist_.begin());
     }
     state_ = next_state;
+
+    if (state_ == s_must_replay)
+    {
+        client_service_.will_replay();
+    }
 }
 
 bool wsrep::transaction::abort_or_interrupt(
@@ -1531,7 +1536,6 @@ int wsrep::transaction::certify_commit(
             // We got BF aborted after succesful certification
             // and before acquiring client state lock. The trasaction
             // must be replayed.
-            client_service_.will_replay();
             state(lock, s_must_replay);
             break;
         default:
@@ -1557,7 +1561,6 @@ int wsrep::transaction::certify_commit(
         // yet known. Therefore the transaction must roll back
         // and go through replay either to replay and commit the whole
         // transaction or to determine failed certification status.
-        client_service_.will_replay();
         if (state() != s_must_abort)
         {
             state(lock, s_must_abort);

test/mock_client_state.hpp

@@ -72,6 +72,7 @@ namespace wsrep
             , sync_point_action_()
             , bytes_generated_()
             , client_state_(client_state)
+            , will_replay_called_()
             , replays_()
             , aborts_()
         { }
@@ -98,10 +99,11 @@ namespace wsrep
                 return 0;
             }
         }
-        void will_replay() WSREP_OVERRIDE { }
 
-        enum wsrep::provider::status
+        void will_replay() WSREP_OVERRIDE { will_replay_called_ = true; }
-        replay() WSREP_OVERRIDE;
+
+        enum wsrep::provider::status replay() WSREP_OVERRIDE;
+
         void wait_for_replayers(
             wsrep::unique_lock<wsrep::mutex>& lock)
             WSREP_OVERRIDE
@@ -195,10 +197,12 @@ namespace wsrep
         //
         // Verifying the state
         //
+        bool will_replay_called() const { return will_replay_called_; }
         size_t replays() const { return replays_; }
         size_t aborts() const { return aborts_; }
     private:
         wsrep::mock_client_state& client_state_;
+        bool will_replay_called_;
         size_t replays_;
         size_t aborts_;
     };

test/transaction_test.cpp

@@ -249,6 +249,46 @@ BOOST_FIXTURE_TEST_CASE_TEMPLATE(
     BOOST_REQUIRE(cc.current_error());
 }
 
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
+    transaction_1pc_bf_during_commit_order_enter, T,
+    replicating_fixtures, T)
+{
+    wsrep::mock_client& cc(T::cc);
+    const wsrep::transaction& tc(T::tc);
+    auto& sc(T::sc);
+
+    // Start a new transaction with ID 1
+    cc.start_transaction(wsrep::transaction_id(1));
+    BOOST_REQUIRE(tc.active());
+    BOOST_REQUIRE(tc.id() == wsrep::transaction_id(1));
+    BOOST_REQUIRE(tc.state() == wsrep::transaction::s_executing);
+
+    sc.provider().commit_order_enter_result_ = wsrep::provider::error_bf_abort;
+
+    // Run before commit
+    BOOST_REQUIRE(cc.before_commit());
+    BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
+    BOOST_REQUIRE(tc.certified() == true);
+    BOOST_REQUIRE(tc.ordered() == true);
+
+    sc.provider().commit_order_enter_result_ = wsrep::provider::success;
+
+    // Rollback sequence
+    BOOST_REQUIRE(cc.before_rollback() == 0);
+    BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.after_rollback() == 0);
+    BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+
+    // Replay from after_statement()
+    cc.after_statement();
+    BOOST_REQUIRE(cc.replays() > 0);
+    BOOST_REQUIRE(tc.active() == false);
+    BOOST_REQUIRE(tc.ordered() == false);
+    BOOST_REQUIRE(tc.certified() == false);
+    BOOST_REQUIRE(not cc.current_error());
+}
+
 //
 // Test a 1PC transaction for which prepare data fails
 //
@@ -341,6 +381,7 @@ BOOST_FIXTURE_TEST_CASE(
 
     BOOST_REQUIRE(cc.before_commit());
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
     BOOST_REQUIRE(cc.after_statement() == 0);
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_committed);
     BOOST_REQUIRE(cc.current_error() == wsrep::e_success);
@@ -361,6 +402,7 @@ BOOST_FIXTURE_TEST_CASE(
 
     BOOST_REQUIRE(cc.before_commit());
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
     BOOST_REQUIRE(cc.after_statement() );
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_aborted);
     BOOST_REQUIRE(cc.current_error() == wsrep::e_deadlock_error);
@@ -391,6 +433,7 @@ BOOST_FIXTURE_TEST_CASE_TEMPLATE(
     // Run before commit
     BOOST_REQUIRE(cc.before_commit());
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
     BOOST_REQUIRE(tc.certified() == false);
     BOOST_REQUIRE(tc.ordered() == true);
 
@@ -755,6 +798,7 @@ BOOST_FIXTURE_TEST_CASE_TEMPLATE(
     // Run before commit
     BOOST_REQUIRE(cc.before_commit());
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
     BOOST_REQUIRE(tc.certified() == true);
     BOOST_REQUIRE(tc.ordered() == true);
 
@@ -1268,6 +1312,7 @@ BOOST_FIXTURE_TEST_CASE(transaction_row_streaming_bf_abort_committing,
     BOOST_REQUIRE(cc.before_rollback() == 0);
     BOOST_REQUIRE(cc.after_rollback() == 0);
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
     BOOST_REQUIRE(cc.after_statement() == 0);
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_committed);
     BOOST_REQUIRE(sc.provider().fragments() == 2);

test/transaction_test_2pc.cpp

@@ -94,6 +94,7 @@ BOOST_FIXTURE_TEST_CASE(
     wsrep_test::bf_abort_ordered(cc);
     BOOST_REQUIRE(cc.after_prepare());
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
     BOOST_REQUIRE(cc.before_rollback() == 0);
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
     BOOST_REQUIRE(cc.after_rollback() == 0);
@@ -124,6 +125,7 @@ BOOST_FIXTURE_TEST_CASE(
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_abort);
     BOOST_REQUIRE(cc.before_rollback() == 0);
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
     BOOST_REQUIRE(cc.after_rollback() == 0);
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
     BOOST_REQUIRE(cc.after_statement() == 0);
@@ -152,6 +154,7 @@ BOOST_FIXTURE_TEST_CASE(
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_abort);
     BOOST_REQUIRE(cc.before_commit());
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
     BOOST_REQUIRE(tc.certified() == true);
     BOOST_REQUIRE(tc.ordered() == true);
     BOOST_REQUIRE(cc.before_rollback() == 0);
@@ -183,6 +186,7 @@ BOOST_FIXTURE_TEST_CASE(
     sc.provider().commit_order_enter_result_ = wsrep::provider::error_bf_abort;
     BOOST_REQUIRE(cc.before_commit());
     BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay);
+    BOOST_REQUIRE(cc.will_replay_called() == true);
     BOOST_REQUIRE(tc.certified() == true);
     BOOST_REQUIRE(tc.ordered() == true);
     sc.provider().commit_order_enter_result_ = wsrep::provider::success;