From eacf88f6a5f9474e62ddfe00fa4276a47e5d6010 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Fri, 14 Dec 2018 14:52:16 +0200 Subject: [PATCH] MXS-2220 Add server version and type information struct The old fields are still used. --- include/maxscale/server.hh | 47 ++++++++------- query_classifier/test/version_sensitivity.cc | 10 +++- server/core/internal/server.hh | 37 +++++++++++- server/core/server.cc | 57 +++++++++++++++++++ .../monitor/mariadbmon/mariadbserver.cc | 9 ++- 5 files changed, 127 insertions(+), 33 deletions(-) diff --git a/include/maxscale/server.hh b/include/maxscale/server.hh index 5e287fc19..5e12aba17 100644 --- a/include/maxscale/server.hh +++ b/include/maxscale/server.hh @@ -51,36 +51,13 @@ struct SERVER_STATS uint64_t packets = 0; /**< Number of packets routed to this server */ }; -/* Server version */ -struct SERVER_VERSION -{ - uint32_t major; - uint32_t minor; - uint32_t patch; -}; - +// TODO: Add clustrix etc types enum server_type_t { SERVER_TYPE_MARIADB, SERVER_TYPE_MYSQL }; -static inline void server_decode_version(uint64_t version, SERVER_VERSION* server_version) -{ - uint32_t major = version / 10000; - uint32_t minor = (version - major * 10000) / 100; - uint32_t patch = version - major * 10000 - minor * 100; - - server_version->major = major; - server_version->minor = minor; - server_version->patch = patch; -} - -static uint64_t server_encode_version(const SERVER_VERSION* server_version) -{ - return server_version->major * 10000 + server_version->minor * 100 + server_version->patch; -} - /** * The SERVER structure defines a backend server. Each server has a name * or IP address for the server, a port that the server listens on and @@ -96,6 +73,13 @@ public: static const int MAX_VERSION_LEN = 256; static const int RLAG_UNDEFINED = -1; // Default replication lag value + struct Version + { + uint32_t major = 0; + uint32_t minor = 0; + uint32_t patch = 0; + }; + // Base settings char* name = nullptr; /**< Server config name */ char* protocol = nullptr; /**< Backend protocol module name */ @@ -180,6 +164,21 @@ public: */ virtual std::string get_custom_parameter(const std::string& name) const = 0; + /** + * Update server version. + * + * @param version_num New numeric version + * @param version_str New version string + */ + virtual void set_version(uint64_t version_num, const std::string& version_str) = 0; + + /** + * Get numeric version information. TODO: Rename to "version" once cleanup is done. + * + * @return Major, minor and patch numbers + */ + virtual Version get_version() const = 0; + protected: SERVER() { diff --git a/query_classifier/test/version_sensitivity.cc b/query_classifier/test/version_sensitivity.cc index 5754079b5..33642eacd 100644 --- a/query_classifier/test/version_sensitivity.cc +++ b/query_classifier/test/version_sensitivity.cc @@ -70,8 +70,12 @@ int test() string valid_json("SELECT Json_Array(56, 3.1416, 'My name is \"Foo\"', NULL)"); string invalid_json("SELECT Json_Foo(56, 3.1416, 'My name is \"Foo\"', NULL)"); - SERVER_VERSION sv; + auto encode_version = [](const SERVER::Version& sv) -> uint64_t + { + return sv.major * 10000 + sv.minor * 100 + sv.patch; + }; + SERVER::Version sv; // pre-Json sv.major = 10; sv.minor = 0; @@ -79,7 +83,7 @@ int test() cout << "Testing pre-Json server." << endl; - qc_set_server_version(server_encode_version(&sv)); + qc_set_server_version(encode_version(sv)); if (!test(valid_json, QUERY_TYPE_READ | QUERY_TYPE_WRITE)) { @@ -98,7 +102,7 @@ int test() sv.minor = 2; sv.patch = 3; - qc_set_server_version(server_encode_version(&sv)); + qc_set_server_version(encode_version(sv)); if (!test(valid_json, QUERY_TYPE_READ)) { diff --git a/server/core/internal/server.hh b/server/core/internal/server.hh index c78ba6e26..607b09dd9 100644 --- a/server/core/internal/server.hh +++ b/server/core/internal/server.hh @@ -98,6 +98,13 @@ public: return m_settings.persistpoolmax > 0; } + void set_version(uint64_t version_num, const std::string& version_str) override; + + Version get_version() const override + { + return info.get(); + } + /** * Get a DCB from the persistent connection pool, if possible * @@ -230,8 +237,36 @@ private: long persistpoolmax = 0; /**< Maximum size of persistent connections pool */ long persistmaxtime = 0; /**< Maximum number of seconds connection can live */ }; - Settings m_settings; /**< Server settings */ + /** + * Stores server version info. Encodes/decodes to/from the version number received from the server. + * Also stores the version string and parses information from it. */ + class VersionInfo + { + public: + /** + * Read in and decode a numeric version from the server. Deduce server type from string. + * + * @param version_num Version number from server + * @param version_string Version string from server + */ + void set(uint64_t version_num, const std::string& version_string); + + /** + * Get version and type info. + * + * @return Version information + */ + Version get() const; + + private: + mutable std::mutex m_lock; /**< Protects against concurrent writing */ + Version m_version; + server_type_t m_type = SERVER_TYPE_MARIADB; + }; + + Settings m_settings; /**< Server settings */ + VersionInfo info; /**< Server version and type information */ maxbase::EMAverage m_response_time; /**< Response time calculations for this server */ }; diff --git a/server/core/server.cc b/server/core/server.cc index f130a8a9c..c7386bf65 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -124,6 +124,35 @@ private: MXS_CONFIG_PARAMETER* m_params = nullptr; }; +/** + * Write to char array by first zeroing any extra space. This reduces effects of concurrent reading. + * + * @param dest Destination buffer. The buffer is assumed to contains at least \0 at the end. + * @param dest_size Maximum size of destination buffer, including terminating \0. + * @param source Source string. A maximum of @c dest_size - 1 characters are copied. + */ +void careful_strcpy(char* dest, size_t dest_size, const std::string& source) +{ + // The string may be accessed while we are updating it. + // Take some precautions to ensure that the string cannot be completely garbled at any point. + // Strictly speaking, this is not fool-proof as writes may not appear in order to the reader. + size_t old_len = strlen(dest); + size_t new_len = source.length(); + if (new_len >= dest_size) + { + new_len = dest_size - 1; // Need space for the \0. + } + + if (new_len < old_len) + { + // If the new string is shorter, zero out the excess data. + memset(dest + new_len, 0, old_len - new_len); + } + + // No null-byte needs to be set. The array starts out as all zeros and the above memset adds + // the necessary null, should the new string be shorter than the old. + strncpy(dest, source.c_str(), new_len); +} } Server* Server::server_alloc(const char* name, MXS_CONFIG_PARAMETER* params) @@ -999,6 +1028,11 @@ void server_set_version_string(SERVER* server, const char* version_string) strncpy(server->version_string, version_string, new_len); } +void Server::set_version(uint64_t version_num, const std::string& version_str) +{ + info.set(version_num, version_str); +} + /** * Set the version of the server. * @@ -1454,3 +1488,26 @@ bool Server::is_custom_parameter(const string& name) const } return true; } + +void Server::VersionInfo::set(uint64_t version, const std::string& version_str) +{ + /* This only protects against concurrent writing which could result in garbled values. Reads are not + * synchronized. Since writing is rare, this is an unlikely issue. Readers should be prepared to + * sometimes get inconsistent values. */ + Guard lock(m_lock); + + uint32_t major, minor, patch; + major = version / 10000; + minor = (version - major * 10000) / 100; + patch = version - major * 10000 - minor * 100; + m_version.major = major; + m_version.minor = minor; + m_version.patch = patch; + bool is_mariadb = (strcasestr(version_str.c_str(), "mariadb") != NULL); + m_type = is_mariadb ? SERVER_TYPE_MARIADB : SERVER_TYPE_MYSQL; +} + +Server::Version Server::VersionInfo::get() const +{ + return m_version; +} diff --git a/server/modules/monitor/mariadbmon/mariadbserver.cc b/server/modules/monitor/mariadbmon/mariadbserver.cc index c18c78bea..ab363c5f9 100644 --- a/server/modules/monitor/mariadbmon/mariadbserver.cc +++ b/server/modules/monitor/mariadbmon/mariadbserver.cc @@ -912,11 +912,10 @@ void MariaDBServer::update_server_version() /* Not a binlog server, check version number and supported features. */ m_srv_type = server_type::NORMAL; m_capabilities = Capabilities(); - SERVER_VERSION decoded = {0, 0, 0}; - server_decode_version(server_get_version(srv), &decoded); - auto major = decoded.major; - auto minor = decoded.minor; - auto patch = decoded.patch; + SERVER::Version info = srv->get_version(); + auto major = info.major; + auto minor = info.minor; + auto patch = info.patch; // MariaDB/MySQL 5.5 is the oldest supported version. MySQL 6 and later are treated as 5.5. if ((major == 5 && minor >= 5) || major > 5) {