- restored immutability of the wsrep::view() class by moving index/ID

   logic closer to where it belongs: protocol-specific part of the code,
   `wsrep_provider_v26.cpp`
 - added view object sanity checks to `on_connect()` method

Refs #13

include/wsrep/view.hpp

@@ -30,6 +30,7 @@
 #include "seqno.hpp"
 #include "gtid.hpp"
 #include <vector>
+#include <iostream>
 
 namespace wsrep
 {
@@ -111,11 +112,7 @@ namespace wsrep
             return (members_.empty() && own_index_ == -1);
         }
 
-        void fix_own_index(ssize_t i)
-        {
-            assert(i < ssize_t(members_.size()));
-            own_index_ = i;
-        }
+        void print(std::ostream& os) const;
 
     private:
         wsrep::gtid state_id_;
@@ -126,6 +123,12 @@ namespace wsrep
         int protocol_version_;
         std::vector<wsrep::view::member> members_;
     };
+
+    static inline
+    std::ostream& operator<<(std::ostream& os, const wsrep::view& v)
+    {
+        v.print(os); return os;
+    }
 }
 
 #endif // WSREP_VIEW

src/CMakeLists.txt

@@ -11,6 +11,7 @@ add_library(wsrep-lib
   logger.cpp
   provider.cpp
   seqno.cpp
+  view.cpp
   server_state.cpp
   transaction.cpp
   wsrep_provider_v26.cpp)

src/server_state.cpp

@@ -567,7 +567,25 @@ wsrep::server_state::causal_read(int timeout) const
 
 void wsrep::server_state::on_connect(const wsrep::view& view)
 {
-    assert(id_.is_undefined());
+    // Sanity checks
+    if (id_.is_undefined() == false)
+    {
+        wsrep::log_warning() << "Unexpected connection in connected state. "
+                             << "Received view: " << view
+                             << "Previous ID: " << id_;
+        assert(0);
+    }
+
+    if (view.own_index() < 0 ||
+        size_t(view.own_index()) >= view.members().size())
+    {
+        std::ostringstream os;
+        os << "Invalid view on connect: own index out of range: " << view;
+        wsrep::log_error() << os.str();
+        assert(0);
+        throw wsrep::runtime_error(os.str());
+    }
+
     id_ = view.members()[view.own_index()].id();
 
     wsrep::log_info() << "Server "
@@ -585,25 +603,15 @@ void wsrep::server_state::on_connect(const wsrep::view& view)
 void wsrep::server_state::on_view(const wsrep::view& view,
                                   wsrep::high_priority_service* high_priority_service)
 {
-    const std::vector<wsrep::view::member>& members(view.members());
-    int own_idx(-1);
-
-    if (!id_.is_undefined())
-    {
-        for (size_t i = 0; i != members.size(); ++i)
-        {
-            if (members[i].id() == id_) { own_idx = i; break; }
-        }
-    }
-
     wsrep::log_info()
         << "================================================\nView:\n"
         << "  id: " << view.state_id() << "\n"
         << "  status: " << view.status() << "\n"
         << "  prococol_version: " << view.protocol_version() << "\n"
-        << "  own_index: " << own_idx << "\n"
+        << "  own_index: " << view.own_index() << "\n"
         << "  final: " << view.final() << "\n"
         << "  members";
+    const std::vector<wsrep::view::member>& members(view.members());
     for (std::vector<wsrep::view::member>::const_iterator i(members.begin());
          i != members.end(); ++i)
     {
@@ -612,7 +620,6 @@ void wsrep::server_state::on_view(const wsrep::view& view,
     }
     wsrep::log_info() << "=================================================";
     current_view_ = view;
-    current_view_.fix_own_index(own_idx);
     if (view.status() == wsrep::view::primary)
     {
         wsrep::unique_lock<wsrep::mutex> lock(mutex_);

src/view.cpp

@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2018 Codership Oy <[email protected]>
+ *
+ * This file is part of wsrep-lib.
+ *
+ * Wsrep-lib is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Wsrep-lib is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with wsrep-lib.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#include "wsrep/view.hpp"
+
+void wsrep::view::print(std::ostream& os) const
+{
+    os << "  id: " << state_id() << "\n"
+       << "  status: " << status() << "\n"
+       << "  prococol_version: " << protocol_version() << "\n"
+       << "  final: " << final() << "\n"
+       << "  own_index: " << own_index() << "\n"
+       << "  members(" << members().size() << "):\n";
+
+    for (std::vector<wsrep::view::member>::const_iterator i(members().begin());
+         i != members().end(); ++i)
+    {
+        os << "\t" << (i - members().begin()) /* ordinal index */
+           << ") id: " << i->id()
+           << ", name: " << i->name() << "\n";
+    }
+}

src/wsrep_provider_v26.cpp

@@ -287,7 +287,8 @@ namespace
     {
         return capabilities;
     }
-    wsrep::view view_from_native(const wsrep_view_info& view_info)
+    wsrep::view view_from_native(const wsrep_view_info& view_info,
+                                 const wsrep::id& own_id)
     {
         std::vector<wsrep::view::member> members;
         for (int i(0); i < view_info.memb_num; ++i)
@@ -303,6 +304,25 @@ namespace
                         sizeof(view_info.members[i].incoming)));
             members.push_back(wsrep::view::member(id, name, incoming));
         }
+
+        int own_idx(-1);
+        if (own_id.is_undefined())
+        {
+            // If own ID is undefined, obtain it from the view. This is
+            // the case on the initial connect to cluster.
+            own_idx = view_info.my_idx;
+        }
+        else
+        {
+            // If the node has already obtained its ID from cluster,
+            // its position in the view (or lack thereof) must be determined
+            // by the ID.
+            for (size_t i(0); i < members.size(); ++i)
+            {
+                if (own_id == members[i].id()) { own_idx = i; break; }
+            }
+        }
+
         return wsrep::view(
             wsrep::gtid(
                 wsrep::id(view_info.state_id.uuid.data,
@@ -311,7 +331,7 @@ namespace
             wsrep::seqno(view_info.view),
             map_view_status_from_native(view_info.status),
             map_capabilities_from_native(view_info.capabilities),
-            view_info.my_idx,
+            own_idx,
             view_info.proto_ver,
             members);
     }
@@ -325,9 +345,11 @@ namespace
         const wsrep_view_info_t* view_info)
     {
         assert(app_ctx);
-        wsrep::view view(view_from_native(*view_info));
         wsrep::server_state& server_state(
             *reinterpret_cast<wsrep::server_state*>(app_ctx));
+        assert(server_state.id().is_undefined());
+        wsrep::view view(view_from_native(*view_info, server_state.id()));
+        assert(view.own_index() >= 0);
         try
         {
             server_state.on_connect(view);
@@ -354,7 +376,7 @@ namespace
             reinterpret_cast<wsrep::high_priority_service*>(recv_ctx));
         try
         {
-            wsrep::view view(view_from_native(*view_info));
+            wsrep::view view(view_from_native(*view_info, server_state.id()));
             server_state.on_view(view, high_priority_service);
             return WSREP_CB_SUCCESS;
         }