Fix assertion unallowed state transition: connected -> joined

When the donor lost its donor state during SST due to cluster
partitioning, the state was erranously changed to `s_joined`
in `start_sst()` and `sst_sent()`, which caused assertion failures
in state checking.

Fixed by changing state to `s_joined` only if donor is still in
`s_donor` state.

src/server_state.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018-2019 Codership Oy <info@codership.com>
+ * Copyright (C) 2018-2023 Codership Oy <info@codership.com>
  *
  * This file is part of wsrep-lib.
  *
@@ -675,8 +675,12 @@ int wsrep::server_state::start_sst(const std::string& sst_request,
         lock.lock();
         wsrep::log_warning() << "SST preparation failed";
         // v26 API does not have JOINED event, so in anticipation of SYNCED
-        // we must do it here.
+        // we must do it here. Do not modify the state if donor lost the
+        // donor state e.g. due to cluster partitioning.
+        if (state(lock) == s_donor)
+        {
             state(lock, s_joined);
+        }
         ret = 1;
     }
     return ret;
@@ -691,8 +695,13 @@ void wsrep::server_state::sst_sent(const wsrep::gtid& gtid, int error)
 
     wsrep::unique_lock<wsrep::mutex> lock(mutex_);
     // v26 API does not have JOINED event, so in anticipation of SYNCED
-    // we must do it here.
+    // we must do it here. Do not modify the state if donor lost the
+    // donor state e.g. due to cluster partitioning.
+    if (state(lock) == s_donor)
+    {
         state(lock, s_joined);
+    }
+
     lock.unlock();
     enum provider::status const retval(provider().sst_sent(gtid, error));
     if (retval != provider::success)

test/mock_server_state.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018-2019 Codership Oy <info@codership.com>
+ * Copyright (C) 2018-2013 Codership Oy <info@codership.com>
  *
  * This file is part of wsrep-lib.
  *
@@ -172,11 +172,20 @@ namespace wsrep
         bool sst_before_init() const WSREP_OVERRIDE
         { return sst_before_init_; }
         std::string sst_request() WSREP_OVERRIDE { return ""; }
-        int start_sst(const std::string&,
+
-                      const wsrep::gtid&,
+        std::function<int()> start_sst_action{};
-                      bool) WSREP_OVERRIDE { return 0; }
+        int start_sst(const std::string&, const wsrep::gtid&,
-        void background_rollback(wsrep::client_state& client_state)
+                      bool) WSREP_OVERRIDE
-            WSREP_OVERRIDE
+        {
+            if (start_sst_action)
+            {
+                return start_sst_action();
+            }
+            return 0;
+        }
+
+        void
+        background_rollback(wsrep::client_state& client_state) WSREP_OVERRIDE
         {
             client_state.before_rollback();
             client_state.after_rollback();

test/server_context_test.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018-2019 Codership Oy <info@codership.com>
+ * Copyright (C) 2018-2023 Codership Oy <info@codership.com>
  *
  * This file is part of wsrep-lib.
  *
@@ -105,6 +105,24 @@ namespace
             BOOST_REQUIRE(ss.state() == wsrep::server_state::s_joiner);
         }
 
+        void non_prim()
+        {
+            BOOST_REQUIRE(ss.state() != wsrep::server_state::s_disconnected);
+            std::vector<wsrep::view::member> members;
+            members.push_back(wsrep::view::member(
+                                  ss.id(), "s1", ""));
+
+            wsrep::view view(wsrep::gtid(),                     // state_id
+                             wsrep::seqno::undefined(),         // view seqno
+                             wsrep::view::non_primary,          // status
+                             0,                                 // capabilities
+                             0,                                 // own_index
+                             0,                                 // protocol ver
+                             members // members
+                );
+            ss.on_view(view, &hps);
+        }
+
         void final_view()
         {
             BOOST_REQUIRE(ss.state() != wsrep::server_state::s_disconnected);
@@ -599,6 +617,35 @@ BOOST_FIXTURE_TEST_CASE(
     BOOST_REQUIRE(ss.state() == wsrep::server_state::s_synced);
 }
 
+BOOST_FIXTURE_TEST_CASE(
+    server_state_sst_first_donor_start_sst_error_in_non_prim,
+    sst_first_server_fixture)
+{
+    bootstrap();
+    BOOST_REQUIRE(ss.state() == wsrep::server_state::s_synced);
+    server_service.start_sst_action = [&]() {
+        non_prim();
+        return 1;
+    };
+    ss.start_sst("", wsrep::gtid(cluster_id, wsrep::seqno(2)), false);
+    BOOST_REQUIRE(ss.state() == wsrep::server_state::s_connected);
+}
+
+BOOST_FIXTURE_TEST_CASE(
+    server_state_sst_first_donor_sst_sent_in_non_prim,
+    sst_first_server_fixture)
+{
+    bootstrap();
+    BOOST_REQUIRE(ss.state() == wsrep::server_state::s_synced);
+    ss.start_sst("", wsrep::gtid(cluster_id, wsrep::seqno(2)), false);
+    BOOST_REQUIRE(ss.state() == wsrep::server_state::s_donor);
+    non_prim();
+    BOOST_REQUIRE(ss.state() == wsrep::server_state::s_connected);
+    ss.sst_sent(wsrep::gtid(cluster_id, wsrep::seqno(2)), 0);
+    // Must stay in connected state
+    BOOST_REQUIRE(ss.state() == wsrep::server_state::s_connected);
+}
+
 /////////////////////////////////////////////////////////////////////////////
 //                    Pause/Resume and Desync/Resync                       //
 /////////////////////////////////////////////////////////////////////////////