Updated wsrep-API, added -Wconversion to compiler flags, fixed errors.

CMakeLists.txt

@@ -67,13 +67,16 @@ if (NOT CMAKE_CXX_STANDARD OR CMAKE_CXX_STANDARD LESS 11)
   endif()
 endif()
 
+# C flags
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Wconversion")
 # CXX flags
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Woverloaded-virtual -g")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Woverloaded-virtual -Wconversion -g")
 
 if (WSREP_LIB_STRICT_BUILD_FLAGS)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Weffc++")
 endif()
 if (WSREP_LIB_MAINTAINER_MODE)
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Werror")
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
 endif()
 

dbsim/db_client.cpp

@@ -34,6 +34,8 @@ db::client::client(db::server& server,
     , client_state_(mutex_, cond_, server_state_, client_service_, client_id, mode)
     , client_service_(*this)
     , se_trx_(server.storage_engine())
+    , random_device_()
+    , random_engine_(random_device_())
     , stats_()
 { }
 
@@ -109,7 +111,8 @@ void db::client::run_one_transaction()
             // wsrep::log_debug() << "Generate write set";
             assert(transaction.active());
             assert(err == 0);
-            int data(std::rand() % params_.n_rows);
+            std::uniform_int_distribution<size_t> uniform_dist(0, params_.n_rows);
+            const size_t data(uniform_dist(random_engine_));
             std::ostringstream os;
             os << data;
             wsrep::key key(wsrep::key::exclusive);
@@ -172,6 +175,6 @@ void db::client::report_progress(size_t i) const
     {
         wsrep::log_info() << "client: " << client_state_.id().get()
                           << " transactions: " << i
-                          << " " << 100*double(i)/params_.n_transactions << "%";
+                          << " " << 100*double(i)/double(params_.n_transactions) << "%";
     }
 }

dbsim/db_client.hpp

@@ -28,6 +28,8 @@
 #include "db_client_service.hpp"
 #include "db_high_priority_service.hpp"
 
+#include <random>
+
 namespace db
 {
     class server;
@@ -77,6 +79,8 @@ namespace db
         db::client_state client_state_;
         db::client_service client_service_;
         db::storage_engine::transaction se_trx_;
+        std::random_device random_device_;
+        std::default_random_engine random_engine_;
         struct stats stats_;
     };
 }

dbsim/db_simulator.cpp

@@ -89,7 +89,7 @@ std::string db::simulator::stats() const
        << "\n"
        << "Seconds: " << duration
        << " \n"
-       << "Transactions per second: " << transactions/duration
+       << "Transactions per second: " << double(transactions)/double(duration)
        << "\n"
        << "BF aborts: "
        << bf_aborts

dbsim/db_storage_engine.cpp

@@ -61,8 +61,9 @@ void db::storage_engine::transaction::rollback()
 
 void db::storage_engine::bf_abort_some(const wsrep::transaction& txc)
 {
+    std::uniform_int_distribution<size_t> uniform_dist(0, alg_freq_);
     wsrep::unique_lock<wsrep::mutex> lock(mutex_);
-    if (alg_freq_ && (std::rand() % alg_freq_) == 0)
+    if (alg_freq_ && uniform_dist(random_engine_) == 0)
     {
         if (transactions_.empty() == false)
         {

dbsim/db_storage_engine.hpp

@@ -27,6 +27,7 @@
 
 #include <atomic>
 #include <unordered_set>
+#include <random>
 
 namespace db
 {
@@ -41,6 +42,8 @@ namespace db
             , bf_aborts_()
             , position_()
             , view_()
+            , random_device_()
+            , random_engine_(random_device_())
         { }
 
         class transaction
@@ -80,6 +83,8 @@ namespace db
         std::atomic<long long> bf_aborts_;
         wsrep::gtid position_;
         wsrep::view view_;
+        std::random_device random_device_;
+        std::default_random_engine random_engine_;
     };
 }
 

include/wsrep/client_id.hpp

@@ -21,6 +21,7 @@
 #define WSREP_CLIENT_ID_HPP
 
 #include <ostream>
+#include <limits>
 
 namespace wsrep
 {
@@ -29,14 +30,14 @@ namespace wsrep
     public:
         typedef unsigned long long type;
         client_id()
-            : id_(-1)
+            : id_(std::numeric_limits<type>::max())
         { }
         template <typename I>
         explicit client_id(I id)
             : id_(static_cast<type>(id))
         { }
         type get() const { return id_; }
-        static type undefined() { return -1; }
+        static type undefined() { return std::numeric_limits<type>::max(); }
         bool operator<(const client_id& other) const
         {
             return (id_ < other.id_);

include/wsrep/transaction_id.hpp

@@ -21,6 +21,7 @@
 #define WSREP_TRANSACTION_ID_HPP
 
 #include <iostream>
+#include <limits>
 
 namespace wsrep
 {
@@ -31,7 +32,7 @@ namespace wsrep
 
 
         transaction_id()
-            : id_(-1)
+            : id_(std::numeric_limits<type>::max())
         { }
 
         template <typename I>

include/wsrep/view.hpp

@@ -97,7 +97,7 @@ namespace wsrep
         wsrep::view::status status() const
         { return status_; }
 
-        ssize_t capabilities() const
+        int capabilities() const
         { return capabilities_; }
 
         ssize_t own_index() const

src/gtid.cpp

@@ -50,5 +50,5 @@ ssize_t wsrep::gtid_print_to_c_str(
         return -ENOBUFS;
     }
     std::strncpy(buf, os.str().c_str(), os.str().size());
-    return os.str().size();
+    return static_cast<ssize_t>(os.str().size());
 }

src/id.cpp

@@ -51,8 +51,8 @@ wsrep::id::id(const std::string& str)
 std::ostream& wsrep::operator<<(std::ostream& os, const wsrep::id& id)
 {
     const char* ptr(static_cast<const char*>(id.data()));
-    ssize_t size(id.size());
+    size_t size(id.size());
-    if (std::count_if(ptr, ptr + size, ::isalnum) == size)
+    if (static_cast<size_t>(std::count_if(ptr, ptr + size, ::isalnum)) == size)
     {
         return (os << std::string(ptr, size));
     }

src/server_state.cpp

@@ -824,8 +824,8 @@ void wsrep::server_state::on_connect(const wsrep::view& view)
         throw wsrep::runtime_error(os.str());
     }
 
-    if (id_.is_undefined() == false &&
+    const size_t own_index(static_cast<size_t>(view.own_index()));
-        id_ != view.members()[view.own_index()].id())
+    if (id_.is_undefined() == false && id_ != view.members()[own_index].id())
     {
         std::ostringstream os;
         os << "Connection in connected state.\n"
@@ -840,7 +840,7 @@ void wsrep::server_state::on_connect(const wsrep::view& view)
     }
     else
     {
-        id_ = view.members()[view.own_index()].id();
+        id_ = view.members()[own_index].id();
     }
 
     wsrep::log_info() << "Server "

src/transaction.cpp

@@ -1052,9 +1052,22 @@ int wsrep::transaction::streaming_step(wsrep::unique_lock<wsrep::mutex>& lock)
     assert(lock.owns_lock());
     assert(streaming_context_.fragment_size());
 
+    if (client_service_.bytes_generated() <
+        streaming_context_.bytes_certified())
+    {
+        /* Something went wrong on DBMS side in keeping track of
+           generated bytes. Return an error to abort the transaction. */
+        wsrep::log_warning() << "Bytes generated "
+                             << client_service_.bytes_generated()
+                             << " less than bytes certified "
+                             << streaming_context_.bytes_certified()
+                             << ", aborting streaming transaction";
+        return 1;
+    }
     int ret(0);
-    const ssize_t bytes_to_replicate(client_service_.bytes_generated() -
+
-                                     streaming_context_.bytes_certified());
+    const size_t bytes_to_replicate(client_service_.bytes_generated() -
+                                    streaming_context_.bytes_certified());
 
     switch (streaming_context_.fragment_unit())
     {

src/view.cpp

@@ -27,7 +27,7 @@ int wsrep::view::member_index(const wsrep::id& member_id) const
     {
         if (i->id() == member_id)
         {
-            return (i - members_.begin());
+            return static_cast<int>(i - members_.begin());
         }
     }
     return -1;

src/wsrep_provider_v26.cpp

@@ -29,6 +29,7 @@
 #include <wsrep_api.h>
 
 #include <cassert>
+#include <climits>
 
 #include <iostream>
 #include <sstream>
@@ -96,69 +97,51 @@ namespace
     inline uint32_t map_one(const int flags, const F from,
                             const T to)
     {
-        return ((flags & from) ? to : 0);
+        // INT_MAX because GCC 4.4 does not eat numeric_limits<int>::max()
+        // in static_assert
+        static_assert(WSREP_FLAGS_LAST < INT_MAX,
+                      "WSREP_FLAGS_LAST < INT_MAX");
+        return static_cast<uint32_t>((flags & static_cast<int>(from)) ?
+                                     static_cast<int>(to) : 0);
     }
 
     uint32_t map_flags_to_native(int flags)
     {
-        using wsrep::provider;
+      using wsrep::provider;
-        return (map_one(flags,
+      return static_cast<uint32_t>(
-                        provider::flag::start_transaction,
+          map_one(flags, provider::flag::start_transaction,
-                        WSREP_FLAG_TRX_START) |
+                  WSREP_FLAG_TRX_START) |
-                map_one(flags,
+          map_one(flags, provider::flag::commit, WSREP_FLAG_TRX_END) |
-                        provider::flag::commit,
+          map_one(flags, provider::flag::rollback, WSREP_FLAG_ROLLBACK) |
-                        WSREP_FLAG_TRX_END) |
+          map_one(flags, provider::flag::isolation, WSREP_FLAG_ISOLATION) |
-                map_one(flags,
+          map_one(flags, provider::flag::pa_unsafe, WSREP_FLAG_PA_UNSAFE) |
-                        provider::flag::rollback,
+          // map_one(flags, provider::flag::commutative, WSREP_FLAG_COMMUTATIVE)
-                        WSREP_FLAG_ROLLBACK) |
+          // |
-                map_one(flags,
+          // map_one(flags, provider::flag::native, WSREP_FLAG_NATIVE) |
-                        provider::flag::isolation,
+          map_one(flags, provider::flag::prepare, WSREP_FLAG_TRX_PREPARE) |
-                        WSREP_FLAG_ISOLATION) |
+          map_one(flags, provider::flag::snapshot, WSREP_FLAG_SNAPSHOT) |
-                map_one(flags,
+          map_one(flags, provider::flag::implicit_deps,
-                        provider::flag::pa_unsafe,
+                  WSREP_FLAG_IMPLICIT_DEPS));
-                        WSREP_FLAG_PA_UNSAFE) |
-                // map_one(flags, provider::flag::commutative, WSREP_FLAG_COMMUTATIVE) |
-                // map_one(flags, provider::flag::native, WSREP_FLAG_NATIVE) |
-                map_one(flags,
-                        provider::flag::prepare,
-                        WSREP_FLAG_TRX_PREPARE) |
-                map_one(flags,
-                        provider::flag::snapshot,
-                        WSREP_FLAG_SNAPSHOT) |
-                map_one(flags,
-                        provider::flag::implicit_deps,
-                        WSREP_FLAG_IMPLICIT_DEPS));
     }
 
-    int map_flags_from_native(uint32_t flags)
+    int map_flags_from_native(uint32_t flags_u32)
     {
-        using wsrep::provider;
+      using wsrep::provider;
-        return (map_one(flags,
+      const int flags(static_cast<int>(flags_u32));
-                        WSREP_FLAG_TRX_START,
+      return static_cast<int>(
-                        provider::flag::start_transaction) |
+          map_one(flags, WSREP_FLAG_TRX_START,
-                map_one(flags,
+                  provider::flag::start_transaction) |
-                        WSREP_FLAG_TRX_END,
+          map_one(flags, WSREP_FLAG_TRX_END, provider::flag::commit) |
-                        provider::flag::commit) |
+          map_one(flags, WSREP_FLAG_ROLLBACK, provider::flag::rollback) |
-                map_one(flags,
+          map_one(flags, WSREP_FLAG_ISOLATION, provider::flag::isolation) |
-                        WSREP_FLAG_ROLLBACK,
+          map_one(flags, WSREP_FLAG_PA_UNSAFE, provider::flag::pa_unsafe) |
-                        provider::flag::rollback) |
+          // map_one(flags, provider::flag::commutative, WSREP_FLAG_COMMUTATIVE)
-                map_one(flags,
+          // |
-                        WSREP_FLAG_ISOLATION,
+          // map_one(flags, provider::flag::native, WSREP_FLAG_NATIVE) |
-                        provider::flag::isolation) |
+          map_one(flags, WSREP_FLAG_TRX_PREPARE, provider::flag::prepare) |
-                map_one(flags,
+          map_one(flags, WSREP_FLAG_SNAPSHOT, provider::flag::snapshot) |
-                        WSREP_FLAG_PA_UNSAFE,
+          map_one(flags, WSREP_FLAG_IMPLICIT_DEPS,
-                        provider::flag::pa_unsafe) |
+                  provider::flag::implicit_deps));
-                // map_one(flags, provider::flag::commutative, WSREP_FLAG_COMMUTATIVE) |
-                // map_one(flags, provider::flag::native, WSREP_FLAG_NATIVE) |
-                map_one(flags,
-                        WSREP_FLAG_TRX_PREPARE,
-                        provider::flag::prepare) |
-                map_one(flags,
-                        WSREP_FLAG_SNAPSHOT,
-                        provider::flag::snapshot) |
-                map_one(flags,
-                        WSREP_FLAG_IMPLICIT_DEPS,
-                        provider::flag::implicit_deps));
     }
 
     class mutable_ws_handle
@@ -288,7 +271,7 @@ namespace
      * be made explicit. */
     int map_capabilities_from_native(wsrep_cap_t capabilities)
     {
-        return capabilities;
+        return static_cast<int>(capabilities);
     }
     wsrep::view view_from_native(const wsrep_view_info& view_info,
                                  const wsrep::id& own_id)
@@ -322,7 +305,11 @@ namespace
             // by the ID.
             for (size_t i(0); i < members.size(); ++i)
             {
-                if (own_id == members[i].id()) { own_idx = i; break; }
+                if (own_id == members[i].id())
+                {
+                    own_idx = static_cast<int>(i);
+                    break;
+                }
             }
         }
 
@@ -351,11 +338,18 @@ namespace
         wsrep::server_state& server_state(
             *reinterpret_cast<wsrep::server_state*>(app_ctx));
         wsrep::view view(view_from_native(*view_info, server_state.id()));
-        assert(view.own_index() >= 0);
+        const ssize_t own_index(view.own_index());
+        assert(own_index >= 0);
+        if (own_index < 0)
+        {
+            wsrep::log_error() << "Connected without being in reported view";
+            return WSREP_CB_FAILURE;
+        }
         assert(// first connect
-               server_state.id().is_undefined() ||
+            server_state.id().is_undefined() ||
-               // reconnect to primary component
+            // reconnect to primary component
-               server_state.id() == view.members()[view.own_index()].id());
+            server_state.id() ==
+            view.members()[static_cast<size_t>(own_index)].id());
         try
         {
             server_state.on_connect(view);
@@ -661,7 +655,7 @@ int wsrep::wsrep_provider_v26::disconnect()
 
 int wsrep::wsrep_provider_v26::capabilities() const
 {
-    return wsrep_->capabilities(wsrep_);
+    return map_capabilities_from_native(wsrep_->capabilities(wsrep_));
 }
 int wsrep::wsrep_provider_v26::desync()
 {

test/server_context_test.cpp

@@ -318,8 +318,6 @@ BOOST_AUTO_TEST_CASE(server_state_state_strings)
                       wsrep::server_state::s_synced) == "synced");
     BOOST_REQUIRE(wsrep::to_string(
                       wsrep::server_state::s_disconnecting) == "disconnecting");
-    BOOST_REQUIRE(wsrep::to_string(
-                      static_cast<enum wsrep::server_state::state>(0xff)) == "unknown");
 }
 
 ///////////////////////////////////////////////////////////////////////////////

test/transaction_test.cpp

@@ -1410,8 +1410,4 @@ BOOST_AUTO_TEST_CASE(transaction_state_strings)
     BOOST_REQUIRE(
         wsrep::to_string(
             wsrep::transaction::s_replaying) == "replaying");
-    BOOST_REQUIRE(
-        wsrep::to_string(
-            static_cast<enum wsrep::transaction::state>(0xff)) == "unknown");
-
 }