From 91968c28afc62d91df365587a122ca1789892354 Mon Sep 17 00:00:00 2001
From: Alexey Yurchenko <alexey.yurchenko@galeracluster.com>
Date: Sun, 28 Nov 2021 23:06:39 +0200
Subject: [PATCH] 1. Never transition from `s_donor` directly to `s_synced`,
 always wait    for SYNCED event as expected. 2. Fix transition to `s_joined`
 only after we have a complete state.    Conditions for `s_joined` are either:
    - SST seqno exceeds connected seqno    - committed seqno exceeds connected
 seqno    - current state is `s_donor`

Refs codership/wsrep-lib#175
---
 include/wsrep/seqno.hpp | 11 ++++++++++
 src/server_state.cpp    | 47 ++++++++++++++++++++++++++++++++---------
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/include/wsrep/seqno.hpp b/include/wsrep/seqno.hpp
index 27563375..9d8cedbc 100644
--- a/include/wsrep/seqno.hpp
+++ b/include/wsrep/seqno.hpp
@@ -60,6 +60,17 @@ namespace wsrep
         {
             return (seqno_ > other.seqno_);
         }
+
+        bool operator<=(seqno other) const
+        {
+            return !(seqno_ > other.seqno_);
+        }
+
+        bool operator>=(seqno other) const
+        {
+            return !(seqno_ < other.seqno_);
+        }
+
         bool operator==(seqno other) const
         {
             return (seqno_ == other.seqno_);
diff --git a/src/server_state.cpp b/src/server_state.cpp
index 63b7396b..6ba5d0ac 100644
--- a/src/server_state.cpp
+++ b/src/server_state.cpp
@@ -668,8 +668,10 @@ int wsrep::server_state::start_sst(const std::string& sst_request,
     if (server_service_.start_sst(sst_request, gtid, bypass))
     {
         lock.lock();
-        wsrep::log_warning() << "SST start failed";
-        state(lock, s_synced);
+        wsrep::log_warning() << "SST preparation failed";
+        // v26 API does not have JOINED event, so in anticipation of SYNCED
+        // we must do it here.
+        state(lock, s_joined);
         ret = 1;
     }
     return ret;
@@ -683,6 +685,8 @@ void wsrep::server_state::sst_sent(const wsrep::gtid& gtid, int error)
         wsrep::log_info() << "SST sending failed: " << 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.
     state(lock, s_joined);
     lock.unlock();
     enum provider::status const retval(provider().sst_sent(gtid, error));
@@ -719,7 +723,6 @@ void wsrep::server_state::sst_received(wsrep::client_service& cs,
                 assert(init_initialized_);
             }
         }
-        state(lock, s_joined);
         lock.unlock();
 
         if (id_.is_undefined())
@@ -731,6 +734,14 @@ void wsrep::server_state::sst_received(wsrep::client_service& cs,
 
         gtid = server_service_.get_position(cs);
         wsrep::log_info() << "Recovered position from storage: " << gtid;
+        if (gtid.seqno() >= connected_gtid().seqno())
+        {
+            /* Now the node has all the data the cluster has: part in
+             * storage, part in replication event queue. */
+            lock.lock();
+            state(lock, s_joined);
+            lock.unlock();
+        }
         wsrep::view const v(server_service_.get_view(cs, id_));
         wsrep::log_info() << "Recovered view from SST:\n" << v;
 
@@ -811,6 +822,21 @@ void wsrep::server_state::last_committed_gtid(const wsrep::gtid& gtid)
            last_committed_gtid_.seqno() + 1 == gtid.seqno());
     last_committed_gtid_ = gtid;
     cond_.notify_all();
+
+    if (state(lock) < s_joined)
+    {
+        // wsrep API does not have a dedicated JOINED event, but we know
+        // that the node is joined when it commits GTID at which it connected
+        assert(state(lock) == s_initialized || state(lock) == s_joiner);
+        if (gtid.seqno() >= connected_gtid().seqno())
+        {
+            state(lock, s_joined);
+        }
+        else
+        {
+            assert(gtid.seqno() < connected_gtid().seqno());
+        }
+    }
 }
 
 wsrep::gtid wsrep::server_state::last_committed_gtid() const
@@ -1084,13 +1110,14 @@ void wsrep::server_state::on_sync()
         {
         case s_synced:
             break;
-        case s_connected:
-            state(lock, s_joiner);
-            // fall through
-        case s_joiner:
-            state(lock, s_initializing);
+        case s_connected:                 // Seed node path: provider becomes
+            state(lock, s_joiner);        // synced with itself before anything
+            // fall through               // else. Then goes DB initialization.
+        case s_joiner:                    // |
+            state(lock, s_initializing);  // V
             break;
         case s_donor:
+            assert(false); // this should never happen
             state(lock, s_joined);
             state(lock, s_synced);
             break;
@@ -1357,14 +1384,14 @@ void wsrep::server_state::state(
     assert(lock.owns_lock());
     static const char allowed[n_states_][n_states_] =
         {
-            /* dis, ing, ized, cted, jer, jed, dor, sed, ding */
+            /* dis, ing, ized, cted, jer, jed, dor, sed, ding to/from */
             {  0,   1,   0,    1,    0,   0,   0,   0,   0}, /* dis */
             {  1,   0,   1,    0,    0,   0,   0,   0,   1}, /* ing */
             {  1,   0,   0,    1,    0,   1,   0,   0,   1}, /* ized */
             {  1,   0,   0,    1,    1,   0,   0,   1,   1}, /* cted */
             {  1,   1,   0,    0,    0,   1,   0,   0,   1}, /* jer */
             {  1,   0,   0,    1,    0,   0,   1,   1,   1}, /* jed */
-            {  1,   0,   0,    1,    0,   1,   0,   1,   1}, /* dor */
+            {  1,   0,   0,    1,    0,   1,   0,   0,   1}, /* dor */
             {  1,   0,   0,    1,    0,   1,   1,   0,   1}, /* sed */
             {  1,   0,   0,    0,    0,   0,   0,   0,   0}  /* ding */
         };