MXS-2220 Add server version and type information struct

The old fields are still used.

include/maxscale/server.hh

@@ -51,36 +51,13 @@ struct SERVER_STATS
     uint64_t packets = 0;       /**< Number of packets routed to this server */
 };
 
-/* Server version */
+// TODO: Add clustrix etc types
-struct SERVER_VERSION
-{
-    uint32_t major;
-    uint32_t minor;
-    uint32_t patch;
-};
-
 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()
     {

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))
     {

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 */
 };
 

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;
+}

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::Version info = srv->get_version();
-        server_decode_version(server_get_version(srv), &decoded);
+        auto major = info.major;
-        auto major = decoded.major;
+        auto minor = info.minor;
-        auto minor = decoded.minor;
+        auto patch = info.patch;
-        auto patch = decoded.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)
         {