Cleanup server version updating

include/maxscale/mysql_utils.h

@@ -134,12 +134,13 @@ mxs_mysql_name_kind_t mxs_mysql_name_to_pcre(char* pcre,
                                              mxs_pcre_quote_approach_t approach);
 
 /**
- * Set the server information
+ * Get server information from connector, store it to server object. This does not query
+ * the server as the data has been read while connecting.
  *
- * @param mysql   A MySQL handle to the server.
+ * @param mysql   MySQL handle from which information is read.
- * @param server  The server whose version information should be updated.
+ * @param server  Server object to write.
  */
-void mxs_mysql_set_server_version(MYSQL* mysql, SERVER* server);
+void mxs_mysql_update_server_version(MYSQL* mysql, SERVER* server);
 
 /**
  * Enable/disable the logging of all SQL statements MaxScale sends to

server/core/mysql_utils.cc

@@ -407,25 +407,13 @@ mxs_mysql_name_kind_t mxs_mysql_name_to_pcre(char* pcre,
     return rv;
 }
 
-void mxs_mysql_set_server_version(MYSQL* mysql, SERVER* server)
+void mxs_mysql_update_server_version(MYSQL* mysql, SERVER* server)
 {
+    // This function should only be called for a live connection.
     const char* version_string = mysql_get_server_info(mysql);
-
+    unsigned long version_num = mysql_get_server_version(mysql);
-    if (version_string)
+    mxb_assert(version_string != NULL && version_num != 0);
-    {
+    server_set_version(server, version_string, version_num);
-        unsigned long version = mysql_get_server_version(mysql);
-
-        server_set_version(server, version_string, version);
-
-        if (strcasestr(version_string, "mariadb") != NULL)
-        {
-            server->server_type = SERVER_TYPE_MARIADB;
-        }
-        else
-        {
-            server->server_type = SERVER_TYPE_MYSQL;
-        }
-    }
 }
 
 void mxs_mysql_set_log_statements(bool enable)

server/core/server.cc

@@ -1092,13 +1092,11 @@ uint64_t server_map_status(const char* str)
  */
 void server_set_version_string(SERVER* server, const char* version_string)
 {
-    // There is a race here. The string may be accessed, while we are
+    // Possible data race. The string may be accessed while we are updating it.
-    // updating it. Thus we take some precautions to ensure that the
+    // Take some precautions to ensure that the string cannot be completely garbled at any point.
-    // 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(server->version_string);
     size_t new_len = strlen(version_string);
-
     if (new_len >= MAX_SERVER_VERSION_LEN)
     {
         new_len = MAX_SERVER_VERSION_LEN - 1;
@@ -1111,10 +1109,9 @@ void server_set_version_string(SERVER* server, const char* version_string)
         memset(server->version_string + 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(server->version_string, version_string, 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.
 }
 
 /**
@@ -1128,8 +1125,9 @@ void server_set_version_string(SERVER* server, const char* version_string)
 void server_set_version(SERVER* server, const char* version_string, uint64_t version)
 {
     server_set_version_string(server, version_string);
-
     atomic_store_uint64(&server->version, version);
+    bool is_mariadb = (strcasestr(version_string, "mariadb") != NULL);
+    server->server_type = is_mariadb ? SERVER_TYPE_MARIADB : SERVER_TYPE_MYSQL;
 }
 
 uint64_t server_get_version(const SERVER* server)

server/modules/authenticator/MySQLAuth/dbusers.cc

@@ -687,7 +687,7 @@ static bool check_server_permissions(SERVICE* service,
 
     if (server->version_string[0] == 0)
     {
-        mxs_mysql_set_server_version(mysql, server);
+        mxs_mysql_update_server_version(mysql, server);
     }
 
     const char* format = "SELECT user, host, %s, Select_priv FROM mysql.user limit 1";
@@ -1010,7 +1010,7 @@ int get_users_from_server(MYSQL* con, SERVER_REF* server_ref, SERVICE* service,
 {
     if (server_ref->server->version_string[0] == 0)
     {
-        mxs_mysql_set_server_version(con, server_ref->server);
+        mxs_mysql_update_server_version(con, server_ref->server);
     }
 
     char* query = get_users_query(server_ref->server->version_string,

server/modules/monitor/galeramon/galeramon.cc

@@ -132,7 +132,7 @@ void GaleraMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server)
     char* server_string;
 
     /* get server version string */
-    mxs_mysql_set_server_version(monitored_server->con, monitored_server->server);
+    mxs_mysql_update_server_version(monitored_server->con, monitored_server->server);
     server_string = monitored_server->server->version_string;
 
     /* Check if the the Galera FSM shows this node is joined to the cluster */

server/modules/monitor/mariadbmon/mariadbserver.cc

@@ -881,7 +881,7 @@ void MariaDBServer::update_server_version()
 
     /* Get server version string, also get/set numeric representation. This function does not query the
      * server, since the data was obtained when connecting. */
-    mxs_mysql_set_server_version(conn, srv);
+    mxs_mysql_update_server_version(conn, srv);
 
     // Check whether this server is a MaxScale Binlog Server.
     MYSQL_RES* result;

server/modules/monitor/mmmon/mmmon.cc

@@ -90,7 +90,7 @@ void MMMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server)
     server_version = mysql_get_server_version(monitored_server->con);
 
     /* get server version string */
-    mxs_mysql_set_server_version(monitored_server->con, monitored_server->server);
+    mxs_mysql_update_server_version(monitored_server->con, monitored_server->server);
     server_string = monitored_server->server->version_string;
 
     /* get server_id form current node */

server/modules/monitor/ndbclustermon/ndbclustermon.cc

@@ -51,7 +51,7 @@ void NDBCMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server)
     char* server_string;
 
     /* get server version string */
-    mxs_mysql_set_server_version(monitored_server->con, monitored_server->server);
+    mxs_mysql_update_server_version(monitored_server->con, monitored_server->server);
     server_string = monitored_server->server->version_string;
 
     /* Check if the the SQL node is able to contact one or more data nodes */