diff --git a/server/modules/authenticator/MariaDBAuth/dbusers.cc b/server/modules/authenticator/MariaDBAuth/dbusers.cc index 7ae687e95..4c8c3c31c 100644 --- a/server/modules/authenticator/MariaDBAuth/dbusers.cc +++ b/server/modules/authenticator/MariaDBAuth/dbusers.cc @@ -21,6 +21,9 @@ #include #include +#include +#include + #include #include #include @@ -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 get_candidates(SERVICE* service, bool skip_local) +{ + std::vector 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() - || (skip_local && server->server->is_mxs_service()) - || !server->server->is_running()) + if (MYSQL* con = gw_mysql_init()) { - continue; - } - - no_active_servers = false; - - MYSQL* con = gw_mysql_init(); - if (con) - { - if (mxs_mysql_real_connect(con, server->server, service_user, dpwd) == NULL) + if (mxs_mysql_real_connect(con, server, service_user, dpwd) == NULL) { - MXS_ERROR("Failure loading users data from backend " - "[%s:%i] for service [%s]. MySQL error %i, %s", - server->server->address, - server->server->port, - service->name(), - mysql_errno(con), - mysql_error(con)); + MXS_ERROR("Failure loading users data from backend [%s:%i] for service [%s]. " + "MySQL error %i, %s", server->address, 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.", diff --git a/server/modules/authenticator/MariaDBAuth/mysql_auth.cc b/server/modules/authenticator/MariaDBAuth/mysql_auth.cc index 27ea8613b..a5b504309 100644 --- a/server/modules/authenticator/MariaDBAuth/mysql_auth.cc +++ b/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; diff --git a/server/modules/authenticator/MariaDBAuth/mysql_auth.hh b/server/modules/authenticator/MariaDBAuth/mysql_auth.hh index c52c1a2f2..ecce208fd 100644 --- a/server/modules/authenticator/MariaDBAuth/mysql_auth.hh +++ b/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 diff --git a/server/modules/routing/readwritesplit/readwritesplit.cc b/server/modules/routing/readwritesplit/readwritesplit.cc index b05d3ef6a..1c1bd981b 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.cc +++ b/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(); diff --git a/server/modules/routing/readwritesplit/rwsplitsession.cc b/server/modules/routing/readwritesplit/rwsplitsession.cc index 66c9c62cd..378b9aac8 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.cc +++ b/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)