From ae746fb28957140fb996a4aaf994baea58bd5287 Mon Sep 17 00:00:00 2001
From: Leandro Pacheco <leandro.pacheco@galeracluster.com>
Date: Tue, 5 Mar 2019 10:53:21 +0100
Subject: [PATCH] fixing reviewer comments

- style fixes
- small improvement to avoid unnecessary search on close_orphaned_sr
---
 include/wsrep/server_state.hpp |  1 +
 src/server_state.cpp           | 30 +++++++--------------
 src/view.cpp                   |  5 ++--
 test/view_test.cpp             | 48 +++++++++++++++++-----------------
 4 files changed, 37 insertions(+), 47 deletions(-)

diff --git a/include/wsrep/server_state.hpp b/include/wsrep/server_state.hpp
index 4ddcce6..fd204fb 100644
--- a/include/wsrep/server_state.hpp
+++ b/include/wsrep/server_state.hpp
@@ -355,6 +355,7 @@ namespace wsrep
          *  Return current view
          */
         const wsrep::view& current_view() const { return current_view_; }
+
         /**
          * Set last committed GTID.
          */
diff --git a/src/server_state.cpp b/src/server_state.cpp
index 07b4118..b53c444 100644
--- a/src/server_state.cpp
+++ b/src/server_state.cpp
@@ -887,21 +887,8 @@ void wsrep::server_state::on_view(const wsrep::view& view,
 {
     wsrep::log_info()
         << "================================================\nView:\n"
-        << "  id: " << view.state_id() << "\n"
-        << "  seqno: " << view.view_seqno() << "\n"
-        << "  status: " << view.status() << "\n"
-        << "  protocol_version: " << view.protocol_version() << "\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)
-    {
-        wsrep::log_info() << "    id: " << i->id() << " "
-                          << "name: " << i->name();
-    }
-    wsrep::log_info() << "=================================================";
+        << view
+        << "=================================================";
     if (current_view_.status() == wsrep::view::primary)
     {
         previous_primary_view_ = current_view_;
@@ -1309,12 +1296,13 @@ void wsrep::server_state::close_orphaned_sr_transactions(
     streaming_appliers_map::iterator i(streaming_appliers_.begin());
     while (i != streaming_appliers_.end())
     {
-        bool origin_not_in_view = std::find_if(current_view_.members().begin(),
-                                               current_view_.members().end(),
-                                               server_id_cmp(i->first.first)) ==
-            current_view_.members().end();
-
-        if (origin_not_in_view || equal_consecutive_views)
+        // rollback SR on equal consecutive primary views or if its
+        // originator is not in the current view
+        if (equal_consecutive_views ||
+            (std::find_if(current_view_.members().begin(),
+                          current_view_.members().end(),
+                          server_id_cmp(i->first.first)) ==
+             current_view_.members().end()))
         {
             WSREP_LOG_DEBUG(wsrep::log::debug_log_level(),
                             wsrep::log::debug_level_server_state,
diff --git a/src/view.cpp b/src/view.cpp
index 598b055..e029141 100644
--- a/src/view.cpp
+++ b/src/view.cpp
@@ -43,7 +43,8 @@ bool wsrep::view::equal_membership(const wsrep::view& other) const
     for (std::vector<member>::const_iterator i(members_.begin());
          i != members_.end(); ++i)
     {
-        if (other.member_index(i->id()) == -1) {
+        if (other.member_index(i->id()) == -1)
+        {
             return false;
         }
     }
@@ -54,7 +55,7 @@ void wsrep::view::print(std::ostream& os) const
 {
     os << "  id: " << state_id() << "\n"
        << "  status: " << to_c_string(status()) << "\n"
-       << "  prococol_version: " << protocol_version() << "\n"
+       << "  protocol_version: " << protocol_version() << "\n"
        << "  capabilities: " << provider::capability::str(capabilities())<<"\n"
        << "  final: " << (final() ? "yes" : "no") << "\n"
        << "  own_index: " << own_index() << "\n"
diff --git a/test/view_test.cpp b/test/view_test.cpp
index 8ffe852..430b592 100644
--- a/test/view_test.cpp
+++ b/test/view_test.cpp
@@ -28,12 +28,12 @@ BOOST_AUTO_TEST_CASE(view_test_member_index)
     members.push_back(wsrep::view::member(wsrep::id("3"), "", ""));
 
     wsrep::view view(wsrep::gtid(wsrep::id("cluster"), wsrep::seqno(1)),
-		     wsrep::seqno(1),
-		     wsrep::view::primary,
-		     0,
-		     1,
-		     0,
-		     members);
+                     wsrep::seqno(1),
+                     wsrep::view::primary,
+                     0,
+                     1,
+                     0,
+                     members);
     BOOST_REQUIRE(view.member_index(wsrep::id("1")) == 0);
     BOOST_REQUIRE(view.member_index(wsrep::id("2")) == 1);
     BOOST_REQUIRE(view.member_index(wsrep::id("3")) == 2);
@@ -59,28 +59,28 @@ BOOST_AUTO_TEST_CASE(view_test_equal_membership)
     m3.push_back(wsrep::view::member(wsrep::id("4"), "", ""));
 
     wsrep::view v1(wsrep::gtid(wsrep::id("cluster"), wsrep::seqno(1)),
-		     wsrep::seqno(1),
-		     wsrep::view::primary,
-		     0,
-		     1,
-		     0,
-             m1);
+                   wsrep::seqno(1),
+                   wsrep::view::primary,
+                   0,
+                   1,
+                   0,
+                   m1);
 
     wsrep::view v2(wsrep::gtid(wsrep::id("cluster"), wsrep::seqno(1)),
-		     wsrep::seqno(1),
-		     wsrep::view::primary,
-		     0,
-		     1,
-		     0,
-		     m2);
+                   wsrep::seqno(1),
+                   wsrep::view::primary,
+                   0,
+                   1,
+                   0,
+                   m2);
 
     wsrep::view v3(wsrep::gtid(wsrep::id("cluster"), wsrep::seqno(1)),
-		     wsrep::seqno(1),
-		     wsrep::view::primary,
-		     0,
-		     1,
-		     0,
-		     m3);
+                   wsrep::seqno(1),
+                   wsrep::view::primary,
+                   0,
+                   1,
+                   0,
+                   m3);
 
     BOOST_REQUIRE(v1.equal_membership(v2));
     BOOST_REQUIRE(v2.equal_membership(v1));