error parameter to nbo calls and m_undefined for toi_mode

toi_mode is set only when actually inside phase one and two.
In between it goes back to m_undefined.

dbsim/db_high_priority_service.cpp

@@ -70,7 +70,8 @@ int db::high_priority_service::apply_toi(
 
 int db::high_priority_service::apply_nbo_begin(
     const wsrep::ws_meta&,
-    const wsrep::const_buffer&)
+    const wsrep::const_buffer&,
+    wsrep::mutable_buffer&)
 {
     throw wsrep::not_implemented_error();
 }

dbsim/db_high_priority_service.hpp

@@ -50,7 +50,8 @@ namespace db
         int rollback(const wsrep::ws_handle&, const wsrep::ws_meta&) override;
         int apply_toi(const wsrep::ws_meta&, const wsrep::const_buffer&,
                       wsrep::mutable_buffer&) override;
-        int apply_nbo_begin(const wsrep::ws_meta&, const wsrep::const_buffer&)
+        int apply_nbo_begin(const wsrep::ws_meta&, const wsrep::const_buffer&,
+                            wsrep::mutable_buffer&)
             override;
         void adopt_apply_error(wsrep::mutable_buffer&) override;
         virtual void after_apply() override;

include/wsrep/client_state.hpp

@@ -752,8 +752,11 @@ namespace wsrep
         /**
          * End non-blocking operation phase after aquiring required
          * resources for operation.
+         *
+         * @param err definition of the error that happened during the
+         *            execution of phase one (empty for no error)
          */
-        int end_nbo_phase_one();
+        int end_nbo_phase_one(const wsrep::mutable_buffer& err);
 
         /**
          * Enter in NBO mode. This method should be called when the
@@ -771,20 +774,17 @@ namespace wsrep
          * Begin non-blocking operation phase two. The keys argument
          * passed to this call must contain the same keys which were
          * passed to begin_nbo_phase_one().
-         *
-         * @todo Keys should be stored internally in client_state
-         *       so that they would not be required as an argument
-         *       here.
-         *
-         * @param keys Array of keys for non-blocking operation.
          */
         int begin_nbo_phase_two();
 
         /**
          * End non-blocking operation phase two. This call will
          * release TOI critical section and set the mode to m_local.
+         *
+         * @param err definition of the error that happened during the
+         *            execution of phase two (empty for no error)
          */
-        int end_nbo_phase_two();
+        int end_nbo_phase_two(const wsrep::mutable_buffer& err);
 
         /**
          * Get reference to the client mutex.

include/wsrep/high_priority_service.hpp

@@ -173,12 +173,14 @@ namespace wsrep
          *
          * @param ws_meta Write set meta data.
          * @param data Buffer containing the command to execute.
+         * @params err Buffer to store error data
          *
          * @return Zero in case of success, non-zero if the asynchronous
          *         process could not be started.
          */
         virtual int apply_nbo_begin(const wsrep::ws_meta& ws_meta,
-                                    const wsrep::const_buffer& data) = 0;
+                                    const wsrep::const_buffer& data,
+                                    wsrep::mutable_buffer& err) = 0;
 
         /**
          * Actions to take after applying a write set was completed.

src/client_state.cpp

@@ -462,7 +462,7 @@ int wsrep::client_state::begin_nbo_phase_one(const wsrep::key_array& keys,
     wsrep::unique_lock<wsrep::mutex> lock(mutex_);
     assert(state_ == s_exec);
     assert(mode_ == m_local);
-    assert(toi_mode_ == m_local);
+    assert(toi_mode_ == m_undefined);
 
     /**
      * @todo Implement retrying if the call fails due to certification
@@ -487,7 +487,9 @@ int wsrep::client_state::begin_nbo_phase_one(const wsrep::key_array& keys,
         current_error_status_ = status;
         if (!toi_meta_.seqno().is_undefined())
         {
-            provider().leave_toi(id_);
+            // TODO(leandro): do we need to pass sth here? is leave_toi necessary here? enter_toi_local doesn't do it.
+            wsrep::mutable_buffer err;
+            provider().leave_toi(id_, err);
         }
         toi_meta_ = wsrep::ws_meta();
         ret= 1;
@@ -497,14 +499,14 @@ int wsrep::client_state::begin_nbo_phase_one(const wsrep::key_array& keys,
     return ret;
 }
 
-int wsrep::client_state::end_nbo_phase_one()
+int wsrep::client_state::end_nbo_phase_one(const wsrep::mutable_buffer& err)
 {
     debug_log_state("end_nbo_phase_one: enter");
     assert(state_ == s_exec);
     assert(mode_ == m_nbo);
     assert(in_toi());
 
-    enum wsrep::provider::status status(provider().leave_toi(id_));
+    enum wsrep::provider::status status(provider().leave_toi(id_, err));
     wsrep::unique_lock<wsrep::mutex> lock(mutex_);
     int ret;
     switch (status)
@@ -519,6 +521,7 @@ int wsrep::client_state::end_nbo_phase_one()
     }
     nbo_meta_ = toi_meta_;
     toi_meta_ = wsrep::ws_meta();
+    toi_mode_ = m_undefined;
     debug_log_state("end_nbo_phase_one: leave");
     return ret;
 }
@@ -527,9 +530,9 @@ int wsrep::client_state::enter_nbo_mode(const wsrep::ws_meta& ws_meta)
 {
     assert(state_ == s_exec);
     assert(mode_ == m_local);
+    assert(toi_mode_ == m_undefined);
     wsrep::unique_lock<wsrep::mutex> lock(mutex_);
     nbo_meta_ = ws_meta;
-    toi_mode_ = mode_;
     mode(lock, m_nbo);
     return 0;
 }
@@ -539,6 +542,7 @@ int wsrep::client_state::begin_nbo_phase_two()
     debug_log_state("begin_nbo_phase_two: enter");
     assert(state_ == s_exec);
     assert(mode_ == m_nbo);
+    assert(toi_mode_ == m_undefined);
     assert(!in_toi());
 
     wsrep::unique_lock<wsrep::mutex> lock(mutex_);
@@ -556,6 +560,7 @@ int wsrep::client_state::begin_nbo_phase_two()
     case wsrep::provider::success:
         ret= 0;
         toi_meta_ = nbo_meta_;
+        toi_mode_ = m_local;
         break;
     default:
         current_error_status_ = status;
@@ -566,14 +571,15 @@ int wsrep::client_state::begin_nbo_phase_two()
     return ret;
 }
 
-int wsrep::client_state::end_nbo_phase_two()
+int wsrep::client_state::end_nbo_phase_two(const wsrep::mutable_buffer& err)
 {
     debug_log_state("end_nbo_phase_two: enter");
     assert(state_ == s_exec);
     assert(mode_ == m_nbo);
+    assert(toi_mode_ == m_local);
     assert(in_toi());
     enum wsrep::provider::status status(
-        provider().leave_toi(id_));
+        provider().leave_toi(id_, err));
     wsrep::unique_lock<wsrep::mutex> lock(mutex_);
     int ret;
     switch (status)
@@ -587,6 +593,7 @@ int wsrep::client_state::end_nbo_phase_two()
         break;
     }
     toi_meta_ = wsrep::ws_meta();
+    toi_mode_ = m_undefined;
     nbo_meta_ = wsrep::ws_meta();
     mode(lock, m_local);
     debug_log_state("end_nbo_phase_two: leave");

src/server_state.cpp

@@ -464,8 +464,9 @@ static int apply_toi(wsrep::provider& provider,
     else if (wsrep::starts_transaction(ws_meta.flags()))
     {
         provider.commit_order_enter(ws_handle, ws_meta);
-        int ret(high_priority_service.apply_nbo_begin(ws_meta, data));
+        wsrep::mutable_buffer err;
-        provider.commit_order_leave(ws_handle, ws_meta);
+        int ret(high_priority_service.apply_nbo_begin(ws_meta, data, err));
+        provider.commit_order_leave(ws_handle, ws_meta, err);
         return ret;
     }
     else if (wsrep::commits_transaction(ws_meta.flags()))
@@ -473,7 +474,8 @@ static int apply_toi(wsrep::provider& provider,
         // NBO end event is ignored here, both local and applied
         // have NBO end handled via local TOI calls.
         provider.commit_order_enter(ws_handle, ws_meta);
-        provider.commit_order_leave(ws_handle, ws_meta);
+        wsrep::mutable_buffer err;
+        provider.commit_order_leave(ws_handle, ws_meta, err);
         return 0;
     }
     else

test/mock_high_priority_service.cpp

@@ -98,7 +98,8 @@ int wsrep::mock_high_priority_service::apply_toi(const wsrep::ws_meta&,
 
 int wsrep::mock_high_priority_service::apply_nbo_begin(
     const wsrep::ws_meta& ws_meta,
-    const wsrep::const_buffer&)
+    const wsrep::const_buffer&,
+    wsrep::mutable_buffer&)
 {
     const int nbo_begin_flags(wsrep::provider::flag::isolation |
                               wsrep::provider::flag::start_transaction);

test/mock_high_priority_service.hpp

@@ -71,7 +71,8 @@ namespace wsrep
                       const wsrep::const_buffer&,
                       wsrep::mutable_buffer&) WSREP_OVERRIDE;
         int apply_nbo_begin(const wsrep::ws_meta&,
-                            const wsrep::const_buffer&) WSREP_OVERRIDE;
+                            const wsrep::const_buffer&,
+                            wsrep::mutable_buffer&) WSREP_OVERRIDE;
         void adopt_apply_error(wsrep::mutable_buffer& err) WSREP_OVERRIDE;
         void after_apply() WSREP_OVERRIDE;
         void store_globals() WSREP_OVERRIDE { }

test/nbo_test.cpp

@@ -43,10 +43,11 @@ BOOST_FIXTURE_TEST_CASE(test_local_nbo,
     BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
     // After required resoureces have been grabbed, NBO leave should
     // end TOI but leave the client state in m_nbo.
-    BOOST_REQUIRE(cc.end_nbo_phase_one() == 0);
+    const wsrep::mutable_buffer err;
+    BOOST_REQUIRE(cc.end_nbo_phase_one(err) == 0);
     BOOST_REQUIRE(cc.mode() == wsrep::client_state::m_nbo);
     BOOST_REQUIRE(cc.in_toi() == false);
-    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
+    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_undefined);
     // Second phase replicates the NBO end event and grabs TOI
     // again for finalizing the NBO.
     BOOST_REQUIRE(cc.begin_nbo_phase_two() == 0);
@@ -55,10 +56,10 @@ BOOST_FIXTURE_TEST_CASE(test_local_nbo,
     BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
     // End of NBO phase will leave TOI and put the client state back to
     // m_local
-    BOOST_REQUIRE(cc.end_nbo_phase_two() == 0);
+    BOOST_REQUIRE(cc.end_nbo_phase_two(err) == 0);
     BOOST_REQUIRE(cc.mode() == wsrep::client_state::m_local);
     BOOST_REQUIRE(cc.in_toi() == false);
-    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
+    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_undefined);
 
     // There must have been two toi write sets, one with
     // start transaction flag, another with commit flag.
@@ -88,7 +89,7 @@ BOOST_FIXTURE_TEST_CASE(test_local_nbo_cert_failure,
                   wsrep::provider::error_certification_failed);
     BOOST_REQUIRE(cc.mode() == wsrep::client_state::m_local);
     BOOST_REQUIRE(cc.in_toi() == false);
-    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
+    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_undefined);
 }
 
 // This test case operates through server_state object in order to
@@ -115,7 +116,8 @@ BOOST_FIXTURE_TEST_CASE(test_applying_nbo,
                                                   nbo_begin.size())) == 0);
     wsrep::mock_client* nbo_cs(hps.nbo_cs());
     BOOST_REQUIRE(nbo_cs);
-    BOOST_REQUIRE(nbo_cs->toi_mode() == wsrep::client_state::m_local);
+    // TODO(leandro): should applying side really be m_local here?
+    BOOST_REQUIRE(nbo_cs->toi_mode() == wsrep::client_state::m_undefined);
     BOOST_REQUIRE(nbo_cs->mode() == wsrep::client_state::m_nbo);
 
     // After this point the control is on local process and applier
@@ -131,10 +133,11 @@ BOOST_FIXTURE_TEST_CASE(test_applying_nbo,
     BOOST_REQUIRE(nbo_cs->toi_mode() == wsrep::client_state::m_local);
     // Ending phase two should make nbo_cs leave TOI mode and
     // return to m_local mode.
-    BOOST_REQUIRE(nbo_cs->end_nbo_phase_two() == 0);
+    const wsrep::mutable_buffer err;
+    BOOST_REQUIRE(nbo_cs->end_nbo_phase_two(err) == 0);
     BOOST_REQUIRE(nbo_cs->mode() == wsrep::client_state::m_local);
     BOOST_REQUIRE(nbo_cs->in_toi() == false);
-    BOOST_REQUIRE(nbo_cs->toi_mode() == wsrep::client_state::m_local);
+    BOOST_REQUIRE(nbo_cs->toi_mode() == wsrep::client_state::m_undefined);
 
     // There must have been one toi write set with commit flag.
     BOOST_REQUIRE(sc.provider().toi_write_sets() == 1);

test/toi_test.cpp

@@ -27,21 +27,22 @@ BOOST_FIXTURE_TEST_CASE(test_toi_mode,
                         replicating_client_fixture_sync_rm)
 {
     BOOST_REQUIRE(cc.mode() == wsrep::client_state::m_local);
-    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
+    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_undefined);
     wsrep::key key(wsrep::key::exclusive);
     key.append_key_part("k1", 2);
     key.append_key_part("k2", 2);
     wsrep::key_array keys{key};
     wsrep::const_buffer buf("toi", 3);
-    BOOST_REQUIRE(cc.enter_toi(keys, buf,
+    BOOST_REQUIRE(cc.enter_toi_local(keys, buf,
                                      wsrep::provider::flag::start_transaction |
                                      wsrep::provider::flag::commit) == 0);
     BOOST_REQUIRE(cc.mode() == wsrep::client_state::m_toi);
     BOOST_REQUIRE(cc.in_toi());
     BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
-    BOOST_REQUIRE(cc.leave_toi() == 0);
+    wsrep::mutable_buffer err;
+    BOOST_REQUIRE(cc.leave_toi_local(err) == 0);
     BOOST_REQUIRE(cc.mode() == wsrep::client_state::m_local);
-    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
+    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_undefined);
     BOOST_REQUIRE(sc.provider().toi_write_sets() == 1);
     BOOST_REQUIRE(sc.provider().toi_start_transaction() == 1);
     BOOST_REQUIRE(sc.provider().toi_commit() == 1);
@@ -50,7 +51,7 @@ BOOST_FIXTURE_TEST_CASE(test_toi_mode,
 BOOST_FIXTURE_TEST_CASE(test_toi_applying,
                         applying_client_fixture)
 {
-    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
+    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_undefined);
     wsrep::ws_meta ws_meta(wsrep::gtid(wsrep::id("1"), wsrep::seqno(2)),
                            wsrep::stid(sc.id(),
                                        wsrep::transaction_id::undefined(),
@@ -58,10 +59,10 @@ BOOST_FIXTURE_TEST_CASE(test_toi_applying,
                            wsrep::seqno(1),
                            wsrep::provider::flag::start_transaction |
                            wsrep::provider::flag::commit);
-    BOOST_REQUIRE(cc.enter_toi(ws_meta) == 0);
+    cc.enter_toi_mode(ws_meta);
     BOOST_REQUIRE(cc.in_toi());
     BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_high_priority);
-    BOOST_REQUIRE(cc.leave_toi() == 0);
+    cc.leave_toi_mode();
-    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_local);
+    BOOST_REQUIRE(cc.toi_mode() == wsrep::client_state::m_undefined);
     cc.after_applying();
 }