Remove MXS_MONITORED_SERVER mapping from MariaDBMon

The mapping was rarely used.

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;
+        MariaDBServer* rejoin_cand = get_server(rejoin_cand_srv);
-        MXS_MONITORED_SERVER* mon_slave_cand = mon_get_monitored_server(m_monitor, rejoin_server);
+        if (rejoin_cand)
-        if (mon_slave_cand)
         {
-            MariaDBServer* slave_cand = get_server_info(mon_slave_cand);
+            if (server_is_rejoin_suspect(rejoin_cand, output))
-            if (server_is_rejoin_suspect(slave_cand, output))
             {
                 string gtid_update_error;
                 if (m_master->update_gtids(&gtid_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 safe_rejoin = rejoin_cand->can_replicate_from(m_master, &no_rejoin_reason);
-                    bool empty_gtid = slave_cand->m_gtid_current_pos.empty();
+                    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.",
+            PRINT_MXS_JSON_ERROR(output, "%s is not monitored by %s, cannot rejoin.",
-                                 rejoin_serv_name);
+                                 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);
     }

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

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<MXS_MONITORED_SERVER*, MariaDBServer*> ServerInfoMap;
 // Map of server id:s to MariaDBServer. Useful when constructing the replication graph.
 typedef std::unordered_map<int64_t, MariaDBServer*> 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();

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()
+    mxb_assert(m_monitor->m_servers.empty() && m_monitor->m_servers_by_id.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)
     {