From 7cd0656990c4b97b6870c31b322d0a157d8c6e54 Mon Sep 17 00:00:00 2001
From: Teemu Ollakka <teemu.ollakka@galeracluster.com>
Date: Wed, 19 Dec 2018 14:30:19 +0200
Subject: [PATCH] Allow server_state joiner - disconnecting transition.

Transition joiner - disconnecting may happen when the joiner failed
to receive SST succesfully. Because the system is at undefined state
at this point, skip most of the processing in sst_received()
and return control to caller after notifying the provider about
failure.
---
 src/server_state.cpp         | 57 +++++++++++++++++++++---------------
 test/server_context_test.cpp |  5 +++-
 2 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/src/server_state.cpp b/src/server_state.cpp
index 061f0ae..62b0a7c 100644
--- a/src/server_state.cpp
+++ b/src/server_state.cpp
@@ -520,12 +520,26 @@ void wsrep::server_state::sst_sent(const wsrep::gtid& gtid, int error)
 }
 
 void wsrep::server_state::sst_received(wsrep::client_service& cs,
-                                       const wsrep::gtid& gtid, int error)
+                                       const wsrep::gtid& gtid,
+                                       int const error)
 {
     wsrep::log_info() << "SST received: " << gtid;
     wsrep::unique_lock<wsrep::mutex> lock(mutex_);
     assert(state_ == s_joiner || state_ == s_initialized);
 
+    // Deal with error case first. If the SST failed, the system
+    // may be in unrecoverable state. There is no point of doing
+    // anything else then notifying the provider about failure
+    // and returning control back to caller.
+    if (error)
+    {
+        if (provider().sst_received(gtid, error))
+        {
+            throw wsrep::runtime_error("wsrep::sst_received() failed");
+        }
+        return;
+    }
+
     if (server_service_.sst_before_init())
     {
         if (init_initialized_ == false)
@@ -548,30 +562,27 @@ void wsrep::server_state::sst_received(wsrep::client_service& cs,
             "wsrep::sst_received() called before connection to cluster");
     }
 
-    if (0 == error) /* SST was a success, recover view */
+    wsrep::view const v(server_service_.get_view(cs, id_));
+    wsrep::log_info() << "Recovered view from SST:\n" << v;
+
+    if (v.state_id().id() != gtid.id() ||
+        v.state_id().seqno() > gtid.seqno())
     {
-        wsrep::view const v(server_service_.get_view(cs, id_));
-        wsrep::log_info() << "Recovered view from SST:\n" << v;
-
-        if (v.state_id().id() != gtid.id() ||
-            v.state_id().seqno() > gtid.seqno())
-        {
-            /* Since IN GENERAL we may not be able to recover SST GTID from
-             * the state data, we have to rely on SST script passing the GTID
-             * value explicitly.
-             * Here we check if the passed GTID makes any sense: it should
-             * have the same UUID and greater or equal seqno than the last
-             * logged view. */
-            std::ostringstream msg;
-            msg << "SST script passed bogus GTID: " << gtid
-                << ". Preceeding view GTID: " << v.state_id();
-            throw wsrep::runtime_error(msg.str());
-        }
-
-        current_view_ = v;
-        server_service_.log_view(NULL /* this view is stored already */, v);
+        /* Since IN GENERAL we may not be able to recover SST GTID from
+         * the state data, we have to rely on SST script passing the GTID
+         * value explicitly.
+         * Here we check if the passed GTID makes any sense: it should
+         * have the same UUID and greater or equal seqno than the last
+         * logged view. */
+        std::ostringstream msg;
+        msg << "SST script passed bogus GTID: " << gtid
+            << ". Preceeding view GTID: " << v.state_id();
+        throw wsrep::runtime_error(msg.str());
     }
 
+    current_view_ = v;
+    server_service_.log_view(NULL /* this view is stored already */, v);
+
     if (provider().sst_received(gtid, error))
     {
         throw wsrep::runtime_error("wsrep::sst_received() failed");
@@ -1076,7 +1087,7 @@ void wsrep::server_state::state(
             {  1,   0,   1,    0,    0,   0,   0,   0,   0}, /* ing */
             {  1,   0,   0,    1,    0,   1,   0,   0,   0}, /* ized */
             {  1,   0,   0,    1,    1,   0,   0,   1,   1}, /* cted */
-            {  1,   1,   0,    0,    0,   1,   0,   0,   0}, /* jer */
+            {  1,   1,   0,    0,    0,   1,   0,   0,   1}, /* jer */
             {  1,   0,   0,    1,    0,   0,   0,   1,   1}, /* jed */
             {  1,   0,   0,    0,    0,   1,   0,   0,   1}, /* dor */
             {  1,   0,   0,    1,    0,   1,   1,   0,   1}, /* sed */
diff --git a/test/server_context_test.cpp b/test/server_context_test.cpp
index 273c4a2..57f22b1 100644
--- a/test/server_context_test.cpp
+++ b/test/server_context_test.cpp
@@ -371,7 +371,7 @@ BOOST_FIXTURE_TEST_CASE(
     disconnect();
 }
 
-// Error during SST.q
+// Error during SST.
 BOOST_FIXTURE_TEST_CASE(
     server_state_sst_first_error_on_joiner,
     sst_first_server_fixture)
@@ -379,6 +379,9 @@ BOOST_FIXTURE_TEST_CASE(
     connect_in_view(second_view);
     ss.prepare_for_sst();
     BOOST_REQUIRE(ss.state() == wsrep::server_state::s_joiner);
+    ss.sst_received(cc, wsrep::gtid(wsrep::id::undefined(),
+                                    wsrep::seqno::undefined()), 1);
+    disconnect();
 }
 
 ///////////////////////////////////////////////////////////////////////////////