Monitor types and fields cleanup

Moves some enums to class enums. Moves some free functions to class methods.

include/maxscale/monitor.hh

@@ -84,19 +84,6 @@ extern const char CN_SCRIPT_TIMEOUT[];
  */
 #define MXS_MONITOR_VERSION {5, 0, 0}
 
-/**
- * Specifies capabilities specific for monitor.
- *
- * @see enum routing_capability
- *
- * @note The values of the capabilities here *must* be between 0x0001 0000 0000 0000
- *       and 0x0080 0000 0000 0000, that is, bits 48 to 55.
- */
-enum monitor_capability_t
-{
-    MCAP_TYPE_NONE = 0x0    // TODO: remove once monitor capabilities are defined
-};
-
 // Monitor state enum
 enum monitor_state_t
 {
@@ -104,16 +91,6 @@ enum monitor_state_t
     MONITOR_STATE_RUNNING,
 };
 
-/* Return type of mon_ping_or_connect_to_db(). */
-enum mxs_connect_result_t
-{
-    MONITOR_CONN_EXISTING_OK,   /* Existing connection was ok and server replied to ping. */
-    MONITOR_CONN_NEWCONN_OK,    /* No existing connection or no ping reply. New connection created
-                                 * successfully. */
-    MONITOR_CONN_REFUSED,       /* No existing connection or no ping reply. Server refused new connection. */
-    MONITOR_CONN_TIMEOUT        /* No existing connection or no ping reply. Timeout on new connection. */
-};
-
 /** Monitor events */
 enum mxs_monitor_event_t
 {
@@ -138,12 +115,6 @@ enum mxs_monitor_event_t
     NEW_DONOR_EVENT   = (1 << 17),  /**< new_donor */
 };
 
-enum credentials_approach_t
-{
-    CREDENTIALS_INCLUDE,
-    CREDENTIALS_EXCLUDE,
-};
-
 namespace maxscale
 {
 
@@ -168,6 +139,16 @@ public:
         int connect_attempts {1};       /**< How many times a connection is attempted */
     };
 
+    /* Return type of mon_ping_or_connect_to_db(). */
+    enum class ConnectResult
+    {
+        OLDCONN_OK,    /* Existing connection was ok and server replied to ping. */
+        NEWCONN_OK,    /* No existing connection or no ping reply. New connection created
+                        * successfully. */
+        REFUSED,       /* No existing connection or no ping reply. Server refused new connection. */
+        TIMEOUT        /* No existing connection or no ping reply. Timeout on new connection. */
+    };
+
     /**
      * Maintenance mode request constants.
      */
@@ -203,7 +184,7 @@ public:
 
     bool status_changed();
     bool should_print_fail_status();
-    void log_connect_error(mxs_connect_result_t rval);
+    void log_connect_error(ConnectResult rval);
 
     /**
      * Report query error to log.
@@ -218,7 +199,7 @@ public:
      * @param settings Connection settings
      * @return Connection status.
      */
-    mxs_connect_result_t ping_or_connect(const ConnectionSettings& settings);
+    ConnectResult ping_or_connect(const ConnectionSettings& settings);
 
     const char* get_event_name();
 
@@ -289,10 +270,10 @@ public:
      *                    valid or NULL.
      * @return Connection status.
      */
-    static mxs_connect_result_t ping_or_connect_to_db(const MonitorServer::ConnectionSettings& sett,
+    static MonitorServer::ConnectResult
-                                                      SERVER& server, MYSQL** ppConn);
+    ping_or_connect_to_db(const MonitorServer::ConnectionSettings& sett, SERVER& server, MYSQL** ppConn);
 
-    static bool connection_is_ok(mxs_connect_result_t connect_result);
+    static bool connection_is_ok(MonitorServer::ConnectResult connect_result);
 
     static std::string get_server_monitor(const SERVER* server);
 
@@ -405,23 +386,11 @@ public:
      */
     bool clear_server_status(SERVER* srv, int bit, std::string* errmsg_out);
 
-    /**
-     * Create a list of running servers
-     *
-     * @param dest Destination where the string is appended, must be null terminated
-     * @param len Length of @c dest
-     * @param approach Whether credentials should be included or not.
-     */
-    void append_node_names(char* dest, int len, int status,
-                           credentials_approach_t approach = CREDENTIALS_EXCLUDE);
-
     void show(DCB* dcb);
 
     const std::string m_name;           /**< Monitor instance name. */
     const std::string m_module;         /**< Name of the monitor module */
 
-    mutable std::mutex m_lock;
-
     std::vector<MonitorServer*> m_servers;       /**< Monitored servers */
 
 protected:
@@ -583,6 +552,22 @@ private:
      */
     int launch_command(MonitorServer* ptr, EXTERNCMD* cmd);
 
+    enum class CredentialsApproach
+    {
+        INCLUDE,
+        EXCLUDE,
+    };
+
+    /**
+     * Create a list of running servers
+     *
+     * @param dest Destination where the string is appended, must be null terminated
+     * @param len Length of @c dest
+     * @param approach Whether credentials should be included or not.
+     */
+    void append_node_names(char* dest, int len, int status,
+                           CredentialsApproach approach = CredentialsApproach::EXCLUDE);
+
     FILE* open_data_file(Monitor* monitor, char* path);
     int   get_data_file_path(char* path) const;
     json_t* parameters_to_json() const;

server/core/monitor.cc

@@ -72,6 +72,7 @@ using std::set;
 using Guard = std::lock_guard<std::mutex>;
 using maxscale::Monitor;
 using maxscale::MonitorServer;
+using ConnectResult = maxscale::MonitorServer::ConnectResult;
 
 const char CN_BACKEND_CONNECT_ATTEMPTS[] = "backend_connect_attempts";
 const char CN_BACKEND_CONNECT_TIMEOUT[] = "backend_connect_timeout";
@@ -926,7 +927,7 @@ const char* MonitorServer::get_event_name()
     return Monitor::get_event_name((mxs_monitor_event_t) server->last_event);
 }
 
-void Monitor::append_node_names(char* dest, int len, int status, credentials_approach_t approach)
+void Monitor::append_node_names(char* dest, int len, int status, CredentialsApproach approach)
 {
     const char* separator = "";
     // Some extra space for port and separator
@@ -938,7 +939,7 @@ void Monitor::append_node_names(char* dest, int len, int status, credentials_app
         Server* server = static_cast<Server*>((*iter)->server);
         if (status == 0 || server->status & status)
         {
-            if (approach == CREDENTIALS_EXCLUDE)
+            if (approach == CredentialsApproach::EXCLUDE)
             {
                 snprintf(arr,
                          sizeof(arr),
@@ -1102,7 +1103,7 @@ int Monitor::launch_command(MonitorServer* ptr, EXTERNCMD* cmd)
     if (externcmd_matches(cmd, "$CREDENTIALS"))
     {
         // We provide the credentials for _all_ servers.
-        append_node_names(nodelist, sizeof(nodelist), 0, CREDENTIALS_INCLUDE);
+        append_node_names(nodelist, sizeof(nodelist), 0, CredentialsApproach::INCLUDE);
         externcmd_substitute_arg(cmd, "[$]CREDENTIALS", nodelist);
     }
 
@@ -1231,8 +1232,8 @@ int Monitor::launch_script(MonitorServer* ptr)
     return rv;
 }
 
-mxs_connect_result_t Monitor::ping_or_connect_to_db(const MonitorServer::ConnectionSettings& sett,
+MonitorServer::ConnectResult
-                                                    SERVER& server, MYSQL** ppConn)
+Monitor::ping_or_connect_to_db(const MonitorServer::ConnectionSettings& sett, SERVER& server, MYSQL** ppConn)
 {
     mxb_assert(ppConn);
     auto pConn = *ppConn;
@@ -1241,13 +1242,13 @@ mxs_connect_result_t Monitor::ping_or_connect_to_db(const MonitorServer::Connect
         /** Return if the connection is OK */
         if (mysql_ping(pConn) == 0)
         {
-            return MONITOR_CONN_EXISTING_OK;
+            return ConnectResult::OLDCONN_OK;
         }
         /** Otherwise close the handle. */
         mysql_close(pConn);
     }
 
-    mxs_connect_result_t conn_result = MONITOR_CONN_REFUSED;
+    ConnectResult conn_result = ConnectResult::REFUSED;
     if ((pConn = mysql_init(NULL)) != nullptr)
     {
         string uname = sett.username;
@@ -1276,14 +1277,14 @@ mxs_connect_result_t Monitor::ping_or_connect_to_db(const MonitorServer::Connect
 
             if (result)
             {
-                conn_result = MONITOR_CONN_NEWCONN_OK;
+                conn_result = ConnectResult::NEWCONN_OK;
                 break;
             }
         }
 
-        if (conn_result == MONITOR_CONN_REFUSED && difftime(end, start) >= sett.connect_timeout)
+        if (conn_result == ConnectResult::REFUSED && difftime(end, start) >= sett.connect_timeout)
         {
-            conn_result = MONITOR_CONN_TIMEOUT;
+            conn_result = ConnectResult::TIMEOUT;
         }
         MXS_FREE(dpwd);
     }
@@ -1292,7 +1293,7 @@ mxs_connect_result_t Monitor::ping_or_connect_to_db(const MonitorServer::Connect
     return conn_result;
 }
 
-mxs_connect_result_t MonitorServer::ping_or_connect(const ConnectionSettings& settings)
+ConnectResult MonitorServer::ping_or_connect(const ConnectionSettings& settings)
 {
     return Monitor::ping_or_connect_to_db(settings, *server, &con);
 }
@@ -1303,9 +1304,9 @@ mxs_connect_result_t MonitorServer::ping_or_connect(const ConnectionSettings& se
  * @param connect_result Return value of mon_ping_or_connect_to_db
  * @return True of connection is ok
  */
-bool Monitor::connection_is_ok(mxs_connect_result_t connect_result)
+bool Monitor::connection_is_ok(ConnectResult connect_result)
 {
-    return connect_result == MONITOR_CONN_EXISTING_OK || connect_result == MONITOR_CONN_NEWCONN_OK;
+    return connect_result == ConnectResult::OLDCONN_OK || connect_result == ConnectResult::NEWCONN_OK;
 }
 
 string Monitor::get_server_monitor(const SERVER* server)
@@ -1324,12 +1325,12 @@ bool Monitor::is_admin_thread()
  *
  * @param rval Return value of mon_ping_or_connect_to_db
  */
-void MonitorServer::log_connect_error(mxs_connect_result_t rval)
+void MonitorServer::log_connect_error(ConnectResult rval)
 {
     mxb_assert(!Monitor::connection_is_ok(rval));
     const char TIMED_OUT[] = "Monitor timed out when connecting to server %s[%s:%d] : '%s'";
     const char REFUSED[] = "Monitor was unable to connect to server %s[%s:%d] : '%s'";
-    MXS_ERROR(rval == MONITOR_CONN_TIMEOUT ? TIMED_OUT : REFUSED,
+    MXS_ERROR(rval == ConnectResult::TIMEOUT ? TIMED_OUT : REFUSED,
               server->name(),
               server->address,
               server->port,
@@ -2189,7 +2190,7 @@ void MonitorWorkerSimple::tick()
             pMs->mon_prev_status = pMs->server->status;
             pMs->pending_status = pMs->server->status;
 
-            mxs_connect_result_t rval = pMs->ping_or_connect(settings().conn_settings);
+            ConnectResult rval = pMs->ping_or_connect(settings().conn_settings);
 
             if (connection_is_ok(rval))
             {

server/core/monitormanager.cc

@@ -343,20 +343,16 @@ bool MonitorManager::create_monitor_config(const Monitor* monitor, const char* f
         return false;
     }
 
+    const MXS_MODULE* mod = get_module(monitor->m_module.c_str(), NULL);
+    mxb_assert(mod);
+
+    string config = generate_config_string(monitor->m_name, monitor->parameters(),
+                                           config_monitor_params, mod->parameters);
+
+    if (dprintf(file, "%s", config.c_str()) == -1)
     {
-        Guard guard(monitor->m_lock);
+        MXS_ERROR("Could not write serialized configuration to file '%s': %d, %s",
-
+                  filename, errno, mxs_strerror(errno));
-        const MXS_MODULE* mod = get_module(monitor->m_module.c_str(), NULL);
-        mxb_assert(mod);
-
-        string config = generate_config_string(monitor->m_name, monitor->parameters(),
-                                               config_monitor_params, mod->parameters);
-
-        if (dprintf(file, "%s", config.c_str()) == -1)
-        {
-            MXS_ERROR("Could not write serialized configuration to file '%s': %d, %s",
-                      filename, errno, mxs_strerror(errno));
-        }
     }
 
     close(file);
@@ -484,29 +480,28 @@ json_t* MonitorManager::monitor_list_to_json(const char* host)
 
 json_t* MonitorManager::monitor_relations_to_server(const SERVER* server, const char* host)
 {
+    mxb_assert(Monitor::is_admin_thread());
     std::vector<std::string> names;
     this_unit.foreach_monitor([&names, server](Monitor* mon) {
-                                  Guard guard(mon->m_lock);
+        // The serverlist of an individual monitor should not change while a monitor is running.
-                                  for (MonitorServer* db : mon->m_servers)
+        for (MonitorServer* db : mon->m_servers)
-                                  {
+        {
-                                      if (db->server == server)
+            if (db->server == server)
-                                      {
+            {
-                                          names.push_back(mon->m_name);
+                names.push_back(mon->m_name);
-                                          break;
+                break;
-                                      }
+            }
-                                  }
+        }
-                                  return true;
+        return true;
-                              });
+    });
 
     json_t* rel = NULL;
     if (!names.empty())
     {
         rel = mxs_json_relationship(host, MXS_JSON_API_MONITORS);
-
+        for (auto& name : names)
-        for (std::vector<std::string>::iterator it = names.begin();
-             it != names.end(); it++)
         {
-            mxs_json_add_relation(rel, it->c_str(), CN_MONITORS);
+            mxs_json_add_relation(rel, name.c_str(), CN_MONITORS);
         }
     }
 

server/modules/monitor/clustrixmon/clustrix.cc

@@ -208,7 +208,7 @@ bool Clustrix::ping_or_connect_to_hub(const char* zName,
                                       MYSQL** ppCon)
 {
     bool connected = false;
-    mxs_connect_result_t rv = Monitor::ping_or_connect_to_db(settings, server, ppCon);
+    MonitorServer::ConnectResult rv = Monitor::ping_or_connect_to_db(settings, server, ppCon);
 
     if (Monitor::connection_is_ok(rv))
     {

server/modules/monitor/mariadbmon/mariadbserver.cc

@@ -29,6 +29,7 @@ using maxbase::StopWatch;
 using maxsql::QueryResult;
 using Guard = std::lock_guard<std::mutex>;
 using maxscale::MonitorServer;
+using ConnectResult = maxscale::MonitorServer::ConnectResult;
 
 MariaDBServer::MariaDBServer(MonitorServer* monitored_server, int config_index,
                              const SharedSettings& settings)
@@ -2191,13 +2192,13 @@ void MariaDBServer::update_server(bool time_to_update_disk_space,
 {
     auto server = this;
     MonitorServer* mon_srv = server->m_server_base;
-    mxs_connect_result_t conn_status = mon_srv->ping_or_connect(conn_settings);
+    ConnectResult conn_status = mon_srv->ping_or_connect(conn_settings);
     MYSQL* conn = mon_srv->con;     // mon_ping_or_connect_to_db() may have reallocated the MYSQL struct.
 
     if (mxs::Monitor::connection_is_ok(conn_status))
     {
         server->set_status(SERVER_RUNNING);
-        if (conn_status == MONITOR_CONN_NEWCONN_OK)
+        if (conn_status == ConnectResult::NEWCONN_OK)
         {
             // Is a new connection or a reconnection. Check server version.
             server->update_server_version();
@@ -2207,7 +2208,7 @@ void MariaDBServer::update_server(bool time_to_update_disk_space,
             || server->m_srv_type == MariaDBServer::server_type::BINLOG_ROUTER)
         {
             // Check permissions if permissions failed last time or if this is a new connection.
-            if (server->had_status(SERVER_AUTH_ERROR) || conn_status == MONITOR_CONN_NEWCONN_OK)
+            if (server->had_status(SERVER_AUTH_ERROR) || conn_status == ConnectResult::NEWCONN_OK)
             {
                 server->check_permissions();
             }