diff --git a/server/modules/monitor/mariadbmon/cluster_manipulation.cc b/server/modules/monitor/mariadbmon/cluster_manipulation.cc index c18220705..a57e53d46 100644 --- a/server/modules/monitor/mariadbmon/cluster_manipulation.cc +++ b/server/modules/monitor/mariadbmon/cluster_manipulation.cc @@ -105,17 +105,15 @@ bool MariaDBMonitor::manual_failover(json_t** output) return failover_done; } -bool MariaDBMonitor::manual_rejoin(SERVER* rejoin_server, json_t** output) +bool MariaDBMonitor::manual_rejoin(SERVER* rejoin_cand_srv, json_t** output) { bool rval = false; if (cluster_can_be_joined()) { - const char* rejoin_serv_name = rejoin_server->name; - MXS_MONITORED_SERVER* mon_slave_cand = mon_get_monitored_server(m_monitor, rejoin_server); - if (mon_slave_cand) + MariaDBServer* rejoin_cand = get_server(rejoin_cand_srv); + if (rejoin_cand) { - MariaDBServer* slave_cand = get_server_info(mon_slave_cand); - if (server_is_rejoin_suspect(slave_cand, output)) + if (server_is_rejoin_suspect(rejoin_cand, output)) { string gtid_update_error; if (m_master->update_gtids(>id_update_error)) @@ -125,8 +123,8 @@ bool MariaDBMonitor::manual_rejoin(SERVER* rejoin_server, json_t** output) // can be rejoined manually. // TODO: Add the warning to JSON output. string no_rejoin_reason; - bool safe_rejoin = slave_cand->can_replicate_from(m_master, &no_rejoin_reason); - bool empty_gtid = slave_cand->m_gtid_current_pos.empty(); + bool safe_rejoin = rejoin_cand->can_replicate_from(m_master, &no_rejoin_reason); + bool empty_gtid = rejoin_cand->m_gtid_current_pos.empty(); bool rejoin_allowed = false; if (safe_rejoin) { @@ -138,19 +136,19 @@ bool MariaDBMonitor::manual_rejoin(SERVER* rejoin_server, json_t** output) { rejoin_allowed = true; MXB_WARNING("gtid_curren_pos of %s is empty. Manual rejoin is unsafe " - "but allowed.", rejoin_serv_name); + "but allowed.", rejoin_cand->name()); } else { PRINT_MXS_JSON_ERROR(output, "%s cannot replicate from master server %s: %s", - rejoin_serv_name, m_master->name(), + rejoin_cand->name(), m_master->name(), no_rejoin_reason.c_str()); } } if (rejoin_allowed) { - ServerArray joinable_server = {slave_cand}; + ServerArray joinable_server = {rejoin_cand}; if (do_rejoin(joinable_server, output) == 1) { rval = true; @@ -172,13 +170,13 @@ bool MariaDBMonitor::manual_rejoin(SERVER* rejoin_server, json_t** output) } else { - PRINT_MXS_JSON_ERROR(output, "The given server '%s' is not monitored by this monitor.", - rejoin_serv_name); + PRINT_MXS_JSON_ERROR(output, "%s is not monitored by %s, cannot rejoin.", + rejoin_cand_srv->name, m_monitor->name); } } else { - const char BAD_CLUSTER[] = "The server cluster of monitor '%s' is not in a state valid for joining. " + const char BAD_CLUSTER[] = "The server cluster of monitor %s is not in a valid state for joining. " "Either it has no master or its gtid domain is unknown."; PRINT_MXS_JSON_ERROR(output, BAD_CLUSTER, m_monitor->name); } diff --git a/server/modules/monitor/mariadbmon/mariadbmon.cc b/server/modules/monitor/mariadbmon/mariadbmon.cc index 20369c3b2..ecad99d1c 100644 --- a/server/modules/monitor/mariadbmon/mariadbmon.cc +++ b/server/modules/monitor/mariadbmon/mariadbmon.cc @@ -81,13 +81,6 @@ void MariaDBMonitor::reset_server_info() { m_servers.push_back(new MariaDBServer(mon_server, m_servers.size(), m_assume_unique_hostnames)); } - for (auto iter = m_servers.begin(); iter != m_servers.end(); iter++) - { - auto mon_server = (*iter)->m_server_base; - mxb_assert(m_server_info.count(mon_server) == 0); - ServerInfoMap::value_type new_val(mon_server, *iter); - m_server_info.insert(new_val); - } } void MariaDBMonitor::clear_server_info() @@ -98,7 +91,6 @@ void MariaDBMonitor::clear_server_info() } // All MariaDBServer*:s are now invalid, as well as any dependant data. m_servers.clear(); - m_server_info.clear(); m_servers_by_id.clear(); m_excluded_servers.clear(); assign_new_master(NULL); @@ -116,42 +108,6 @@ void MariaDBMonitor::reset_node_index_info() } } -/** - * Get monitor-specific server info for the monitored server. - * - * @param handle - * @param db Server to get info for. Must be a valid server or function crashes. - * @return The server info. - */ -MariaDBServer* MariaDBMonitor::get_server_info(MXS_MONITORED_SERVER* db) -{ - mxb_assert(m_server_info.count(db) == 1); // Should always exist in the map - return m_server_info[db]; -} - -MariaDBServer* MariaDBMonitor::get_server(int64_t id) -{ - auto found = m_servers_by_id.find(id); - return (found != m_servers_by_id.end()) ? (*found).second : NULL; -} - -/** - * Get the equivalent MariaDBServer. - * - * @param server Which server to search for - * @return MariaDBServer if found, NULL otherwise - */ -MariaDBServer* MariaDBMonitor::get_server(SERVER* server) -{ - MariaDBServer* found = NULL; - auto mon_server = mon_get_monitored_server(m_monitor, server); - if (mon_server) - { - found = get_server_info(mon_server); - } - return found; -} - MariaDBServer* MariaDBMonitor::get_server(const std::string& host, int port) { // TODO: Do this with a map lookup @@ -168,6 +124,29 @@ MariaDBServer* MariaDBMonitor::get_server(const std::string& host, int port) return found; } +MariaDBServer* MariaDBMonitor::get_server(int64_t id) +{ + auto found = m_servers_by_id.find(id); + return (found != m_servers_by_id.end()) ? (*found).second : NULL; +} + +MariaDBServer* MariaDBMonitor::get_server(MXS_MONITORED_SERVER* mon_server) +{ + return get_server(mon_server->server); +} + +MariaDBServer* MariaDBMonitor::get_server(SERVER* server) +{ + for (auto iter : m_servers) + { + if (iter->m_server_base->server == server) + { + return iter; + } + } + return NULL; +} + bool MariaDBMonitor::set_replication_credentials(const MXS_CONFIG_PARAMETER* params) { bool rval = false; @@ -234,7 +213,7 @@ bool MariaDBMonitor::configure(const MXS_CONFIG_PARAMETER* params) int n_excluded = mon_config_get_servers(params, CN_NO_PROMOTE_SERVERS, m_monitor, &excluded_array); for (int i = 0; i < n_excluded; i++) { - m_excluded_servers.push_back(get_server_info(excluded_array[i])); + m_excluded_servers.push_back(get_server(excluded_array[i])); } MXS_FREE(excluded_array); @@ -437,7 +416,7 @@ void MariaDBMonitor::pre_loop() // This is somewhat questionable, as the journal only contains status bits but no actual topology // info. In a fringe case the actual queried topology may not match the journal data, freezing the // master to a suboptimal choice. - assign_new_master(get_server_info(journal_master)); + assign_new_master(get_server(journal_master)); } /* This loop can be removed if/once the replication check code is inside tick. It's required so that diff --git a/server/modules/monitor/mariadbmon/mariadbmon.hh b/server/modules/monitor/mariadbmon/mariadbmon.hh index d8e4c9e7b..8a40356ce 100644 --- a/server/modules/monitor/mariadbmon/mariadbmon.hh +++ b/server/modules/monitor/mariadbmon/mariadbmon.hh @@ -29,8 +29,6 @@ extern const char* const CN_SWITCHOVER_ON_LOW_DISK_SPACE; extern const char* const CN_PROMOTION_SQL_FILE; extern const char* const CN_DEMOTION_SQL_FILE; -// Map of base struct to MariaDBServer. Does not own the server objects. -typedef std::unordered_map ServerInfoMap; // Map of server id:s to MariaDBServer. Useful when constructing the replication graph. typedef std::unordered_map IdToServerMap; // Map of cycle number to cycle members. The elements should be ordered for predictability when iterating. @@ -168,7 +166,6 @@ private: // Server containers, mostly constant. ServerArray m_servers; /* Servers of the monitor */ - ServerInfoMap m_server_info; /* Map from server base struct to MariaDBServer */ IdToServerMap m_servers_by_id; /* Map from server id:s to MariaDBServer */ // Topology related fields @@ -251,10 +248,10 @@ private: std::string diagnostics_to_string() const; json_t* to_json() const; - MariaDBServer* get_server_info(MXS_MONITORED_SERVER* db); - MariaDBServer* get_server(int64_t id); - MariaDBServer* get_server(SERVER* server); MariaDBServer* get_server(const std::string& host, int port); + MariaDBServer* get_server(int64_t id); + MariaDBServer* get_server(MXS_MONITORED_SERVER* mon_server); + MariaDBServer* get_server(SERVER* server); // Cluster discovery and status assignment methods, top levels void update_server(MariaDBServer* server); @@ -283,7 +280,7 @@ private: // Cluster operation launchers bool manual_switchover(SERVER* new_master, SERVER* current_master, json_t** error_out); bool manual_failover(json_t** output); - bool manual_rejoin(SERVER* rejoin_server, json_t** output); + bool manual_rejoin(SERVER* rejoin_cand_srv, json_t** output); void handle_low_disk_space_master(); void handle_auto_failover(); void handle_auto_rejoin(); diff --git a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc index 2a5a72829..c1f03ef82 100644 --- a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc +++ b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc @@ -140,8 +140,7 @@ int MariaDBMonitor::Test::run_tests() void MariaDBMonitor::Test::init_servers(int count) { clear_servers(); - mxb_assert(m_monitor->m_server_info.empty() && m_monitor->m_servers.empty() - && m_monitor->m_servers_by_id.empty()); + mxb_assert(m_monitor->m_servers.empty() && m_monitor->m_servers_by_id.empty()); for (int i = 1; i < count + 1; i++) { @@ -154,7 +153,6 @@ void MariaDBMonitor::Test::init_servers(int count) MariaDBServer* new_server = new MariaDBServer(mon_server, i - 1); new_server->m_server_id = i; m_monitor->m_servers.push_back(new_server); - m_monitor->m_server_info[mon_server] = new_server; m_monitor->m_servers_by_id[i] = new_server; } m_current_test++; @@ -165,7 +163,6 @@ void MariaDBMonitor::Test::init_servers(int count) */ void MariaDBMonitor::Test::clear_servers() { - m_monitor->m_server_info.clear(); m_monitor->m_servers_by_id.clear(); for (MariaDBServer* server : m_monitor->m_servers) {