Merge branch '2.3' into 2.4

server/modules/authenticator/MariaDBAuth/dbusers.cc

@@ -21,6 +21,9 @@
 #include <netdb.h>
 #include <stdio.h>
 
+#include <algorithm>
+#include <vector>
+
 #include <maxbase/alloc.h>
 #include <maxscale/dcb.hh>
 #include <maxscale/maxscale.h>
@@ -143,7 +146,7 @@ enum server_category_t
     SERVER_CLUSTRIX
 };
 
-static int    get_users(Listener* listener, bool skip_local);
+static int    get_users(Listener* listener, bool skip_local, SERVER** srv);
 static MYSQL* gw_mysql_init(void);
 static int    gw_mysql_set_timeouts(MYSQL* handle);
 static char*  mysql_format_user_entry(void* data);
@@ -255,9 +258,9 @@ static char* get_users_query(const SERVER::Version& version, bool include_root,
     return rval;
 }
 
-int replace_mysql_users(Listener* listener, bool skip_local)
+int replace_mysql_users(Listener* listener, bool skip_local, SERVER** srv)
 {
-    int i = get_users(listener, skip_local);
+    int i = get_users(listener, skip_local, srv);
     return i;
 }
 
@@ -1139,9 +1142,8 @@ bool query_and_process_users(const char* query, MYSQL* con, SERVICE* service, in
     return rval;
 }
 
-int get_users_from_server(MYSQL* con, SERVER_REF* server_ref, SERVICE* service, Listener* listener)
+int get_users_from_server(MYSQL* con, SERVER* server, SERVICE* service, Listener* listener)
 {
-    SERVER* server = server_ref->server;
     auto server_version = server->version();
     if (server_version.total == 0)      // No monitor or the monitor hasn't ran yet.
     {
@@ -1234,6 +1236,28 @@ int get_users_from_server(MYSQL* con, SERVER_REF* server_ref, SERVICE* service,
     return users;
 }
 
+// Sorts candidates servers so that masters are before slaves which are before only running servers
+static std::vector<SERVER*> get_candidates(SERVICE* service, bool skip_local)
+{
+    std::vector<SERVER*> candidates;
+
+    for (auto server = service->dbref; server; server = server->next)
+    {
+        if (server_ref_is_active(server) && server->server->is_running()
+            && (!skip_local || !server->server->is_mxs_service()))
+        {
+            candidates.push_back(server->server);
+        }
+    }
+
+    std::sort(candidates.begin(), candidates.end(), [](SERVER* a, SERVER* b) {
+                  return (a->is_master() && !b->is_master())
+                  || (a->is_slave() && !b->is_slave() && !b->is_master());
+              });
+
+    return candidates;
+}
+
 /**
  * Load the user/passwd form mysql.user table into the service users' hashtable
  * environment.
@@ -1242,7 +1266,7 @@ int get_users_from_server(MYSQL* con, SERVER_REF* server_ref, SERVICE* service,
  * @param users     The users table into which to load the users
  * @return          -1 on any error or the number of users inserted
  */
-static int get_users(Listener* listener, bool skip_local)
+static int get_users(Listener* listener, bool skip_local, SERVER** srv)
 {
     const char* service_user = NULL;
     const char* service_passwd = NULL;
@@ -1262,33 +1286,18 @@ static int get_users(Listener* listener, bool skip_local)
     sqlite3* handle = get_handle(instance);
     delete_mysql_users(handle);
 
-    SERVER_REF* server = service->dbref;
     int total_users = -1;
-    bool no_active_servers = true;
+    auto candidates = get_candidates(service, skip_local);
 
-    for (server = service->dbref; !maxscale_is_shutting_down() && server; server = server->next)
+    for (auto server : candidates)
     {
-        if (!server_ref_is_active(server) || !server->server->server_is_active()
+        if (MYSQL* con = gw_mysql_init())
-            || (skip_local && server->server->is_mxs_service())
-            || !server->server->is_running())
         {
-            continue;
+            if (mxs_mysql_real_connect(con, server, service_user, dpwd) == NULL)
-        }
-
-        no_active_servers = false;
-
-        MYSQL* con = gw_mysql_init();
-        if (con)
             {
-            if (mxs_mysql_real_connect(con, server->server, service_user, dpwd) == NULL)
+                MXS_ERROR("Failure loading users data from backend [%s:%i] for service [%s]. "
-            {
+                          "MySQL error %i, %s", server->address, server->port, service->name(),
-                MXS_ERROR("Failure loading users data from backend "
+                          mysql_errno(con), mysql_error(con));
-                          "[%s:%i] for service [%s]. MySQL error %i, %s",
-                          server->server->address,
-                          server->server->port,
-                          service->name(),
-                          mysql_errno(con),
-                          mysql_error(con));
                 mysql_close(con);
             }
             else
@@ -1298,6 +1307,7 @@ static int get_users(Listener* listener, bool skip_local)
 
                 if (users > total_users)
                 {
+                    *srv = server;
                     total_users = users;
                 }
 
@@ -1313,12 +1323,12 @@ static int get_users(Listener* listener, bool skip_local)
 
     MXS_FREE(dpwd);
 
-    if (no_active_servers)
+    if (candidates.empty())
     {
         // This service has no servers or all servers are local MaxScale services
         total_users = 0;
     }
-    else if (server == NULL && total_users == -1)
+    else if (*srv == nullptr && total_users == -1)
     {
         MXS_ERROR("Unable to get user data from backend database for service [%s]."
                   " Failed to connect to any of the backend databases.",

server/modules/authenticator/MariaDBAuth/mysql_auth.cc

@@ -774,7 +774,8 @@ static int mysql_auth_load_users(Listener* port)
         first_load = true;
     }
 
-    int loaded = replace_mysql_users(port, first_load);
+    SERVER* srv = nullptr;
+    int loaded = replace_mysql_users(port, first_load, &srv);
     bool injected = false;
 
     if (loaded <= 0)
@@ -821,7 +822,9 @@ static int mysql_auth_load_users(Listener* port)
     }
     else if (loaded > 0 && first_load)
     {
-        MXS_NOTICE("[%s] Loaded %d MySQL users for listener %s.", service->name(), loaded, port->name());
+        mxb_assert(srv);
+        MXS_NOTICE("[%s] Loaded %d MySQL users for listener %s from server %s.",
+                   service->name(), loaded, port->name(), srv->name());
     }
 
     return rc;

server/modules/authenticator/MariaDBAuth/mysql_auth.hh

@@ -188,10 +188,11 @@ bool dbusers_save(sqlite3* src, const char* filename);
  *
  * @param service    The current service
  * @param skip_local Skip loading of users on local MaxScale services
+ * @param srv        Server where the users were loaded from (output)
  *
  * @return -1 on any error or the number of users inserted (0 means no users at all)
  */
-int replace_mysql_users(Listener* listener, bool skip_local);
+int replace_mysql_users(Listener* listener, bool skip_local, SERVER** srv);
 
 /**
  * @brief Verify the user has access to the database

server/modules/routing/readwritesplit/readwritesplit.cc

@@ -415,8 +415,6 @@ json_t* RWSplit::diagnostics_json() const
 
     for (const auto& a : all_server_stats())
     {
-        mxb_assert(a.second.total == a.second.read + a.second.write);
-
         ServerStats::CurrentStats stats = a.second.current_stats();
 
         json_t* obj = json_object();

server/modules/routing/readwritesplit/rwsplitsession.cc

@@ -1027,7 +1027,6 @@ void RWSplitSession::handleError(GWBUF* errmsgbuf,
                 {
                     // We were expecting a response but we aren't going to get one
                     mxb_assert(m_expected_responses > 0);
-                    m_expected_responses--;
                     errmsg += " Lost connection to master server while waiting for a result.";
 
                     if (can_retry_query())
@@ -1042,6 +1041,14 @@ void RWSplitSession::handleError(GWBUF* errmsgbuf,
                         can_continue = true;
                         send_readonly_error(m_client);
                     }
+
+                    // Decrement the expected response count only if we know we can continue the sesssion.
+                    // This keeps the internal logic sound even if another query is routed before the session
+                    // is closed.
+                    if (can_continue)
+                    {
+                        m_expected_responses--;
+                    }
                 }
 
                 if (session_trx_is_active(session) && m_otrx_state == OTRX_INACTIVE)