From 97d9f93648a1d7a1be6c4cd68204cf025098d789 Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Tue, 12 Jun 2018 10:20:58 +0300 Subject: [PATCH] Refactored seqno and id classes out of provider.hpp --- include/wsrep/id.hpp | 75 ++++++++++++++++++++++ include/wsrep/provider.hpp | 105 +++---------------------------- include/wsrep/seqno.hpp | 74 ++++++++++++++++++++++ src/CMakeLists.txt | 3 +- src/id.cpp | 54 ++++++++++++++++ src/mock_provider.hpp | 16 +++-- src/mock_server_context.hpp | 2 +- src/provider.cpp | 30 --------- src/server_context.cpp | 11 ++-- src/server_context_test.cpp | 4 +- src/test_utils.cpp | 4 +- src/transaction_context_test.cpp | 6 +- src/wsrep_provider_v26.cpp | 6 +- src/wsrep_provider_v26.hpp | 2 +- 14 files changed, 242 insertions(+), 150 deletions(-) create mode 100644 include/wsrep/id.hpp create mode 100644 include/wsrep/seqno.hpp create mode 100644 src/id.cpp diff --git a/include/wsrep/id.hpp b/include/wsrep/id.hpp new file mode 100644 index 0000000..190c409 --- /dev/null +++ b/include/wsrep/id.hpp @@ -0,0 +1,75 @@ +// +// Copyright (C) 2018 Codership Oy +// + +/*! \file id.hpp + * + * A generic identifier utility class. + */ +#ifndef WSREP_ID_HPP +#define WSREP_ID_HPP + +#include "exception.hpp" + +#include +#include // std::memset() + +namespace wsrep +{ + /*! + * The idientifier class stores identifiers either in UUID + * format or in string format. The storage format is decided + * upon construction. If the given string contains a valid + * UUID, the storage format will be binary. Otherwise the + * string will be copied verbatim. If the string format is used, + * the maximum length of the identifier is limited to 16 bytes. + */ + class id + { + public: + /*! + * Default constructor. Constructs an empty identifier. + */ + id() : data_() { std::memset(data_, 0, sizeof(data_)); } + + /*! + * Construct from string. The input string may contain either + * valid UUID or a string with maximum 16 bytes length. + */ + id(const std::string&); + + /*! + * Construct from void pointer. + */ + id (const void* data, size_t size) : data_() + { + if (size > 16) + { + throw wsrep::runtime_error("Too long identifier"); + } + std::memset(data_, 0, sizeof(data_)); + std::memcpy(data_, data, size); + } + + bool operator<(const id& other) const + { + return (std::memcmp(data_, other.data_, sizeof(data_)) < 0); + } + + bool operator==(const id& other) const + { + return (std::memcmp(data_, other.data_, sizeof(data_)) == 0); + } + + const void* data() const { return data_; } + + size_t size() const { return sizeof(data_); } + + private: + unsigned char data_[16]; + }; + + std::ostream& operator<<(std::ostream&, const wsrep::id& id); +} + +#endif // WSREP_ID_HPP diff --git a/include/wsrep/provider.hpp b/include/wsrep/provider.hpp index 8df045b..4d4916d 100644 --- a/include/wsrep/provider.hpp +++ b/include/wsrep/provider.hpp @@ -5,6 +5,8 @@ #ifndef WSREP_PROVIDER_HPP #define WSREP_PROVIDER_HPP +#include "id.hpp" +#include "seqno.hpp" #include "key.hpp" #include "data.hpp" #include "client_id.hpp" @@ -20,102 +22,7 @@ namespace wsrep { - /*! \class seqno - * - * Sequence number type. - * - * By convention, nil value is zero, negative values are not allowed. - * Relation operators are restricted to < and > on purpose to - * enforce correct use. - */ - class seqno - { - public: - seqno() - : seqno_() - { } - - template - seqno(I seqno) - : seqno_(seqno) - { - assert(seqno_ >= 0); - } - - long long get() const - { - return seqno_; - } - - bool nil() const - { - return (seqno_ == 0); - } - - bool operator<(seqno other) const - { - return (seqno_ < other.seqno_); - } - - bool operator>(seqno other) const - { - return (seqno_ > other.seqno_); - } - - seqno operator+(seqno other) const - { - return (seqno_ + other.seqno_); - } - - static seqno undefined() { return 0; } - private: - long long seqno_; - }; - - static inline std::ostream& operator<<(std::ostream& os, wsrep::seqno seqno) - { - return (os << seqno.get()); - } - - class id - { - public: - enum type - { - none, - string, - uuid - }; - - id() - : type_(none) - , data_() - { } - - id(const std::string&); - - id (const void* data, size_t data_size) - : type_() - , data_() - { - assert(data_size <= 16); - std::memcpy(data_, data, data_size); - } - bool operator<(const id& other) const - { - return (std::memcmp(data_, other.data_, sizeof(data_)) < 0); - } - bool operator==(const id& other) const - { - return (std::memcmp(data_, other.data_, sizeof(data_)) == 0); - } - const unsigned char* data() const { return data_; } - size_t data_size() const { return 16; } - - private: - enum type type_; - unsigned char data_[16]; - }; + class server_context; class gtid { @@ -242,7 +149,6 @@ namespace wsrep int flags_; }; - // Abstract interface for provider implementations class provider { @@ -308,6 +214,9 @@ namespace wsrep static const int snapshot = (1 << 7); }; + provider(wsrep::server_context& server_context) + : server_context_(server_context) + { } virtual ~provider() { } // Provider state management virtual int connect(const std::string& cluster_name, @@ -368,6 +277,8 @@ namespace wsrep // virtual struct wsrep* native() = 0; // Factory method static provider* make_provider(const std::string& provider); + private: + wsrep::server_context& server_context_; }; static inline std::string flags_to_string(int flags) diff --git a/include/wsrep/seqno.hpp b/include/wsrep/seqno.hpp new file mode 100644 index 0000000..cf76637 --- /dev/null +++ b/include/wsrep/seqno.hpp @@ -0,0 +1,74 @@ +// +// Copyright (C) 2018 Codership Oy +// + +#ifndef WSREP_SEQNO_HPP +#define WSREP_SEQNO_HPP + +#include "exception.hpp" + +#include + +namespace wsrep +{ + /*! \class seqno + * + * Sequence number type. + * + * By convention, nil value is zero, negative values are not allowed. + * Relation operators are restricted to < and > on purpose to + * enforce correct use. + */ + class seqno + { + public: + seqno() + : seqno_() + { } + + explicit seqno(long long seqno) + : seqno_(seqno) + { + if (seqno_ < 0) + { + throw wsrep::runtime_error("Negative seqno given"); + } + } + + long long get() const + { + return seqno_; + } + + bool nil() const + { + return (seqno_ == 0); + } + + bool operator<(seqno other) const + { + return (seqno_ < other.seqno_); + } + + bool operator>(seqno other) const + { + return (seqno_ > other.seqno_); + } + + seqno operator+(seqno other) const + { + return (seqno(seqno_ + other.seqno_)); + } + + static seqno undefined() { return seqno(0); } + private: + long long seqno_; + }; + + static inline std::ostream& operator<<(std::ostream& os, wsrep::seqno seqno) + { + return (os << seqno.get()); + } +} + +#endif // WSREP_SEQNO_HPP diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 462ef13..5207ae9 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -5,9 +5,10 @@ add_library(wsrep-lib + client_context.cpp + id.cpp logger.cpp provider.cpp - client_context.cpp server_context.cpp transaction_context.cpp wsrep_provider_v26.cpp) diff --git a/src/id.cpp b/src/id.cpp new file mode 100644 index 0000000..3ff5c7e --- /dev/null +++ b/src/id.cpp @@ -0,0 +1,54 @@ +// +// Copyright (C) 2018 Codership Oy +// + +#include "wsrep/id.hpp" +#include + +#include +#include +#include + +wsrep::id::id(const std::string& str) + : data_() +{ + wsrep_uuid_t wsrep_uuid; + if (wsrep_uuid_scan(str.c_str(), str.size(), &wsrep_uuid) == + WSREP_UUID_STR_LEN) + { + std::memcpy(data_, wsrep_uuid.data, sizeof(data_)); + } + else if (str.size() <= 16) + { + std::memcpy(data_, str.c_str(), str.size()); + } + else + { + std::ostringstream os; + os << "String '" << str + << "' does not contain UUID or is longer thatn 16 bytes"; + throw wsrep::runtime_error(os.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) + { + return (os << std::string(ptr, size)); + } + else + { + char uuid_str[WSREP_UUID_STR_LEN + 1]; + wsrep_uuid_t uuid; + std::memcpy(uuid.data, ptr, sizeof(uuid.data)); + if (wsrep_uuid_print(&uuid, uuid_str, sizeof(uuid_str)) < 0) + { + throw wsrep::runtime_error("Could not print uuid"); + } + uuid_str[WSREP_UUID_STR_LEN] = '\0'; + return (os << uuid_str); + } +} diff --git a/src/mock_provider.hpp b/src/mock_provider.hpp index 6b314de..6782e8a 100644 --- a/src/mock_provider.hpp +++ b/src/mock_provider.hpp @@ -18,8 +18,9 @@ namespace wsrep public: typedef std::map bf_abort_map; - mock_provider() - : group_id_("1") + mock_provider(wsrep::server_context& server_context) + : provider(server_context) + , group_id_("1") , server_id_("1") , group_seqno_(0) , bf_abort_map_() @@ -59,8 +60,9 @@ namespace wsrep if (it == bf_abort_map_.end()) { ++group_seqno_; - wsrep::gtid gtid(group_id_, group_seqno_); - ws_meta = wsrep::ws_meta(gtid, stid, group_seqno_ - 1, + wsrep::gtid gtid(group_id_, wsrep::seqno(group_seqno_)); + ws_meta = wsrep::ws_meta(gtid, stid, + wsrep::seqno(group_seqno_ - 1), flags); return wsrep::provider::success; } @@ -76,8 +78,10 @@ namespace wsrep else { ++group_seqno_; - wsrep::gtid gtid(group_id_, group_seqno_); - ws_meta = wsrep::ws_meta(gtid, stid, group_seqno_ - 1, flags); + wsrep::gtid gtid(group_id_, wsrep::seqno(group_seqno_)); + ws_meta = wsrep::ws_meta(gtid, stid, + wsrep::seqno(group_seqno_ - 1), + flags); ret = wsrep::provider::error_bf_abort; } bf_abort_map_.erase(it); diff --git a/src/mock_server_context.hpp b/src/mock_server_context.hpp index 08e33d1..6dedfa4 100644 --- a/src/mock_server_context.hpp +++ b/src/mock_server_context.hpp @@ -23,7 +23,7 @@ namespace wsrep name, id, "", "./", rollback_mode) , mutex_() , cond_() - , provider_() + , provider_(*this) , last_client_id_(0) { } wsrep::mock_provider& provider() const diff --git a/src/provider.cpp b/src/provider.cpp index a479d5c..11c9be4 100644 --- a/src/provider.cpp +++ b/src/provider.cpp @@ -4,36 +4,6 @@ #include "wsrep/provider.hpp" -#include "provider_impl.hpp" - -#include -#include - -#include - -wsrep::id::id(const std::string& str) - : type_(none) - , data_() -{ - wsrep_uuid_t wsrep_uuid; - if (wsrep_uuid_scan(str.c_str(), str.size(), &wsrep_uuid) == - WSREP_UUID_STR_LEN) - { - type_ = uuid; - std::memcpy(data_, wsrep_uuid.data, sizeof(data_)); - } - else if (str.size() <= 16) - { - type_ = string; - std::memcpy(data_, str.c_str(), str.size()); - } - else - { - std::ostringstream os; - os << "String '" << str << "' does not contain UUID"; - throw wsrep::runtime_error(os.str()); - } -} wsrep::provider* wsrep::provider::make_provider( const std::string&) { diff --git a/src/server_context.cpp b/src/server_context.cpp index 0c7789e..d27fafd 100644 --- a/src/server_context.cpp +++ b/src/server_context.cpp @@ -146,11 +146,11 @@ namespace wsrep::ws_meta ws_meta( wsrep::gtid(wsrep::id(meta->gtid.uuid.data, sizeof(meta->gtid.uuid.data)), - meta->gtid.seqno), + wsrep::seqno(meta->gtid.seqno)), wsrep::stid(wsrep::id(meta->stid.node.data, sizeof(meta->stid.node.data)), meta->stid.trx, - meta->stid.conn), meta->depends_on, + meta->stid.conn), wsrep::seqno(meta->depends_on), map_flags_from_native(flags)); if (ret == WSREP_CB_SUCCESS && client_context->server_context().on_apply( @@ -195,7 +195,7 @@ namespace req_buf->len); wsrep::gtid gtid(wsrep::id(req_gtid->uuid.data, sizeof(req_gtid->uuid.data)), - req_gtid->seqno); + wsrep::seqno(req_gtid->seqno)); server_context.on_sst_request(req, gtid, bypass); return WSREP_CB_SUCCESS; } @@ -212,7 +212,7 @@ int wsrep::server_context::load_provider(const std::string& provider_spec, wsrep::log() << "Loading provider " << provider_spec; if (provider_spec == "mock") { - provider_ = new wsrep::mock_provider; + provider_ = new wsrep::mock_provider(*this); } else { @@ -238,7 +238,8 @@ int wsrep::server_context::load_provider(const std::string& provider_spec, init_args.synced_cb = &synced_cb; std::cerr << init_args.options << "\n"; - provider_ = new wsrep::wsrep_provider_v26(provider_spec.c_str(), + provider_ = new wsrep::wsrep_provider_v26(*this, + provider_spec.c_str(), &init_args); } return 0; diff --git a/src/server_context_test.cpp b/src/server_context_test.cpp index cf6b389..96dffb0 100644 --- a/src/server_context_test.cpp +++ b/src/server_context_test.cpp @@ -18,9 +18,9 @@ namespace wsrep::client_context::m_applier, false) , ws_handle(1, (void*)1) - , ws_meta(wsrep::gtid(wsrep::id("1"), 1), + , ws_meta(wsrep::gtid(wsrep::id("1"), wsrep::seqno(1)), wsrep::stid(wsrep::id("1"), 1, 1), - 0, + wsrep::seqno(0), wsrep::provider::flag::start_transaction | wsrep::provider::flag::commit) { diff --git a/src/test_utils.cpp b/src/test_utils.cpp index c4505f8..9abb0f7 100644 --- a/src/test_utils.cpp +++ b/src/test_utils.cpp @@ -12,14 +12,14 @@ void wsrep_test::bf_abort_unordered(wsrep::client_context& cc) { wsrep::unique_lock lock(cc.mutex()); assert(cc.transaction().ordered() == false); - cc.bf_abort(lock, 1); + cc.bf_abort(lock, wsrep::seqno(1)); } void wsrep_test::bf_abort_ordered(wsrep::client_context& cc) { wsrep::unique_lock lock(cc.mutex()); assert(cc.transaction().ordered()); - cc.bf_abort(lock, 0); + cc.bf_abort(lock, wsrep::seqno(0)); } // BF abort method to abort transactions via provider void wsrep_test::bf_abort_provider(wsrep::mock_server_context& sc, diff --git a/src/transaction_context_test.cpp b/src/transaction_context_test.cpp index 36330da..f6083e2 100644 --- a/src/transaction_context_test.cpp +++ b/src/transaction_context_test.cpp @@ -85,9 +85,9 @@ namespace BOOST_REQUIRE(cc.before_command() == 0); BOOST_REQUIRE(cc.before_statement() == 0); wsrep::ws_handle ws_handle(1, (void*)1); - wsrep::ws_meta ws_meta(wsrep::gtid(wsrep::id("1"), 1), + wsrep::ws_meta ws_meta(wsrep::gtid(wsrep::id("1"), wsrep::seqno(1)), wsrep::stid(sc.id(), 1, cc.id()), - 0, + wsrep::seqno(0), wsrep::provider::flag::start_transaction | wsrep::provider::flag::commit); BOOST_REQUIRE(cc.start_transaction(ws_handle, ws_meta) == 0); @@ -528,7 +528,7 @@ BOOST_FIXTURE_TEST_CASE_TEMPLATE( BOOST_REQUIRE(tc.id() == wsrep::transaction_id(1)); BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_executing); - wsrep_test::bf_abort_provider(sc, tc, 1); + wsrep_test::bf_abort_provider(sc, tc, wsrep::seqno(1)); // Run before commit BOOST_REQUIRE(cc.before_commit()); diff --git a/src/wsrep_provider_v26.cpp b/src/wsrep_provider_v26.cpp index 368e568..2bbb4c6 100644 --- a/src/wsrep_provider_v26.cpp +++ b/src/wsrep_provider_v26.cpp @@ -66,7 +66,7 @@ namespace static inline wsrep::seqno seqno_from_native(wsrep_seqno_t seqno) { - return (seqno == WSREP_SEQNO_UNDEFINED ? 0 : seqno); + return wsrep::seqno(seqno == WSREP_SEQNO_UNDEFINED ? 0 : seqno); } inline uint32_t map_one(const int flags, const int from, const uint32_t to) @@ -202,9 +202,11 @@ namespace } wsrep::wsrep_provider_v26::wsrep_provider_v26( + wsrep::server_context& server_context, const char* path, wsrep_init_args* args) - : wsrep_() + : provider(server_context) + , wsrep_() { if (wsrep_load(path, &wsrep_, 0)) { diff --git a/src/wsrep_provider_v26.hpp b/src/wsrep_provider_v26.hpp index f80facf..151f19b 100644 --- a/src/wsrep_provider_v26.hpp +++ b/src/wsrep_provider_v26.hpp @@ -15,7 +15,7 @@ namespace wsrep { public: - wsrep_provider_v26(const char*, struct wsrep_init_args*); + wsrep_provider_v26(wsrep::server_context&, const char*, struct wsrep_init_args*); ~wsrep_provider_v26(); int connect(const std::string&, const std::string&, const std::string&, bool);