From 477a71dd46cc423fd24deb4601ce16949edf1658 Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Thu, 10 Oct 2019 13:54:05 +0300 Subject: [PATCH] Updated wsrep-API, added -Wconversion to compiler flags, fixed errors. --- CMakeLists.txt | 5 +- dbsim/db_client.cpp | 7 +- dbsim/db_client.hpp | 4 + dbsim/db_simulator.cpp | 2 +- dbsim/db_storage_engine.cpp | 3 +- dbsim/db_storage_engine.hpp | 5 ++ include/wsrep/client_id.hpp | 5 +- include/wsrep/transaction_id.hpp | 3 +- include/wsrep/view.hpp | 2 +- src/gtid.cpp | 2 +- src/id.cpp | 4 +- src/server_state.cpp | 6 +- src/transaction.cpp | 17 ++++- src/view.cpp | 2 +- src/wsrep_provider_v26.cpp | 126 +++++++++++++++---------------- test/server_context_test.cpp | 2 - test/transaction_test.cpp | 4 - wsrep-API/v26 | 2 +- 18 files changed, 110 insertions(+), 91 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cdeee834..3634259a 100644 --- a/CMakeLists.txt +++ b/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() diff --git a/dbsim/db_client.cpp b/dbsim/db_client.cpp index 480556cc..dc3f923f 100644 --- a/dbsim/db_client.cpp +++ b/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 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) << "%"; } } diff --git a/dbsim/db_client.hpp b/dbsim/db_client.hpp index fd012f9f..8a51d2d9 100644 --- a/dbsim/db_client.hpp +++ b/dbsim/db_client.hpp @@ -28,6 +28,8 @@ #include "db_client_service.hpp" #include "db_high_priority_service.hpp" +#include + 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_; }; } diff --git a/dbsim/db_simulator.cpp b/dbsim/db_simulator.cpp index 003616cd..12a3e225 100644 --- a/dbsim/db_simulator.cpp +++ b/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 diff --git a/dbsim/db_storage_engine.cpp b/dbsim/db_storage_engine.cpp index 2be0fe5c..c102783a 100644 --- a/dbsim/db_storage_engine.cpp +++ b/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 uniform_dist(0, alg_freq_); wsrep::unique_lock lock(mutex_); - if (alg_freq_ && (std::rand() % alg_freq_) == 0) + if (alg_freq_ && uniform_dist(random_engine_) == 0) { if (transactions_.empty() == false) { diff --git a/dbsim/db_storage_engine.hpp b/dbsim/db_storage_engine.hpp index ba649258..0721a2ad 100644 --- a/dbsim/db_storage_engine.hpp +++ b/dbsim/db_storage_engine.hpp @@ -27,6 +27,7 @@ #include #include +#include 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 bf_aborts_; wsrep::gtid position_; wsrep::view view_; + std::random_device random_device_; + std::default_random_engine random_engine_; }; } diff --git a/include/wsrep/client_id.hpp b/include/wsrep/client_id.hpp index 2861d8e6..f7597c88 100644 --- a/include/wsrep/client_id.hpp +++ b/include/wsrep/client_id.hpp @@ -21,6 +21,7 @@ #define WSREP_CLIENT_ID_HPP #include +#include namespace wsrep { @@ -29,14 +30,14 @@ namespace wsrep public: typedef unsigned long long type; client_id() - : id_(-1) + : id_(std::numeric_limits::max()) { } template explicit client_id(I id) : id_(static_cast(id)) { } type get() const { return id_; } - static type undefined() { return -1; } + static type undefined() { return std::numeric_limits::max(); } bool operator<(const client_id& other) const { return (id_ < other.id_); diff --git a/include/wsrep/transaction_id.hpp b/include/wsrep/transaction_id.hpp index 133eedc5..f5fb5dbc 100644 --- a/include/wsrep/transaction_id.hpp +++ b/include/wsrep/transaction_id.hpp @@ -21,6 +21,7 @@ #define WSREP_TRANSACTION_ID_HPP #include +#include namespace wsrep { @@ -31,7 +32,7 @@ namespace wsrep transaction_id() - : id_(-1) + : id_(std::numeric_limits::max()) { } template diff --git a/include/wsrep/view.hpp b/include/wsrep/view.hpp index 5eef9a63..7e1348b0 100644 --- a/include/wsrep/view.hpp +++ b/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 diff --git a/src/gtid.cpp b/src/gtid.cpp index 5fa90f6b..cef88d27 100644 --- a/src/gtid.cpp +++ b/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(os.str().size()); } diff --git a/src/id.cpp b/src/id.cpp index 6c355e7f..32a70a3d 100644 --- a/src/id.cpp +++ b/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(id.data())); - ssize_t size(id.size()); - if (std::count_if(ptr, ptr + size, ::isalnum) == size) + size_t size(id.size()); + if (static_cast(std::count_if(ptr, ptr + size, ::isalnum)) == size) { return (os << std::string(ptr, size)); } diff --git a/src/server_state.cpp b/src/server_state.cpp index cde28ea2..9469a76d 100644 --- a/src/server_state.cpp +++ b/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 && - id_ != view.members()[view.own_index()].id()) + const size_t own_index(static_cast(view.own_index())); + 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 " diff --git a/src/transaction.cpp b/src/transaction.cpp index 59fa3691..57dc8c9a 100644 --- a/src/transaction.cpp +++ b/src/transaction.cpp @@ -1052,9 +1052,22 @@ int wsrep::transaction::streaming_step(wsrep::unique_lock& 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()) { diff --git a/src/view.cpp b/src/view.cpp index e0291410..d5bff099 100644 --- a/src/view.cpp +++ b/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(i - members_.begin()); } } return -1; diff --git a/src/wsrep_provider_v26.cpp b/src/wsrep_provider_v26.cpp index 92640e33..6c020773 100644 --- a/src/wsrep_provider_v26.cpp +++ b/src/wsrep_provider_v26.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include @@ -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::max() + // in static_assert + static_assert(WSREP_FLAGS_LAST < INT_MAX, + "WSREP_FLAGS_LAST < INT_MAX"); + return static_cast((flags & static_cast(from)) ? + static_cast(to) : 0); } uint32_t map_flags_to_native(int flags) { - using wsrep::provider; - return (map_one(flags, - provider::flag::start_transaction, - WSREP_FLAG_TRX_START) | - map_one(flags, - provider::flag::commit, - WSREP_FLAG_TRX_END) | - map_one(flags, - provider::flag::rollback, - WSREP_FLAG_ROLLBACK) | - map_one(flags, - provider::flag::isolation, - WSREP_FLAG_ISOLATION) | - map_one(flags, - provider::flag::pa_unsafe, - 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) - { - using wsrep::provider; - return (map_one(flags, - WSREP_FLAG_TRX_START, - provider::flag::start_transaction) | - map_one(flags, - WSREP_FLAG_TRX_END, - provider::flag::commit) | - map_one(flags, - WSREP_FLAG_ROLLBACK, - provider::flag::rollback) | - map_one(flags, - WSREP_FLAG_ISOLATION, - provider::flag::isolation) | - map_one(flags, - WSREP_FLAG_PA_UNSAFE, - provider::flag::pa_unsafe) | - // 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)); + using wsrep::provider; + return static_cast( + map_one(flags, provider::flag::start_transaction, + WSREP_FLAG_TRX_START) | + map_one(flags, provider::flag::commit, WSREP_FLAG_TRX_END) | + map_one(flags, provider::flag::rollback, WSREP_FLAG_ROLLBACK) | + map_one(flags, provider::flag::isolation, WSREP_FLAG_ISOLATION) | + map_one(flags, provider::flag::pa_unsafe, 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_u32) + { + using wsrep::provider; + const int flags(static_cast(flags_u32)); + return static_cast( + map_one(flags, WSREP_FLAG_TRX_START, + provider::flag::start_transaction) | + map_one(flags, WSREP_FLAG_TRX_END, provider::flag::commit) | + map_one(flags, WSREP_FLAG_ROLLBACK, provider::flag::rollback) | + map_one(flags, WSREP_FLAG_ISOLATION, provider::flag::isolation) | + map_one(flags, WSREP_FLAG_PA_UNSAFE, provider::flag::pa_unsafe) | + // 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(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(i); + break; + } } } @@ -351,11 +338,18 @@ namespace wsrep::server_state& server_state( *reinterpret_cast(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() || - // reconnect to primary component - server_state.id() == view.members()[view.own_index()].id()); + server_state.id().is_undefined() || + // reconnect to primary component + server_state.id() == + view.members()[static_cast(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() { diff --git a/test/server_context_test.cpp b/test/server_context_test.cpp index 47dea466..7ced77f4 100644 --- a/test/server_context_test.cpp +++ b/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(0xff)) == "unknown"); } /////////////////////////////////////////////////////////////////////////////// diff --git a/test/transaction_test.cpp b/test/transaction_test.cpp index dd4cf388..01a39ab4 100644 --- a/test/transaction_test.cpp +++ b/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(0xff)) == "unknown"); - } diff --git a/wsrep-API/v26 b/wsrep-API/v26 index 75a5f452..c7cec7d7 160000 --- a/wsrep-API/v26 +++ b/wsrep-API/v26 @@ -1 +1 @@ -Subproject commit 75a5f452f2ba07b0f4a3a9a94825fccc71b27398 +Subproject commit c7cec7d719f8bd6f44a138b1c4e29a959d910fc9