From 08b4c2665208e2867babc88a6dfafc19549f49e4 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Tue, 14 May 2019 17:57:22 +0300 Subject: [PATCH] Monitor types and fields cleanup Moves some enums to class enums. Moves some free functions to class methods. --- include/maxscale/monitor.hh | 77 ++++++++----------- server/core/monitor.cc | 33 ++++---- server/core/monitormanager.cc | 51 ++++++------ .../modules/monitor/clustrixmon/clustrix.cc | 2 +- .../monitor/mariadbmon/mariadbserver.cc | 7 +- 5 files changed, 76 insertions(+), 94 deletions(-) diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index 057418a9f..fe7544aff 100644 --- a/include/maxscale/monitor.hh +++ b/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, - SERVER& server, MYSQL** ppConn); + static MonitorServer::ConnectResult + 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 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; diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 5bacecacb..ab6867b72 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -72,6 +72,7 @@ using std::set; using Guard = std::lock_guard; 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((*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, - SERVER& server, MYSQL** ppConn) +MonitorServer::ConnectResult +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)) { diff --git a/server/core/monitormanager.cc b/server/core/monitormanager.cc index 42b2db80f..c57f0490e 100644 --- a/server/core/monitormanager.cc +++ b/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); - - 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)); - } + 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 names; this_unit.foreach_monitor([&names, server](Monitor* mon) { - Guard guard(mon->m_lock); - for (MonitorServer* db : mon->m_servers) - { - if (db->server == server) - { - names.push_back(mon->m_name); - break; - } - } - return true; - }); + // The serverlist of an individual monitor should not change while a monitor is running. + for (MonitorServer* db : mon->m_servers) + { + if (db->server == server) + { + names.push_back(mon->m_name); + break; + } + } + return true; + }); json_t* rel = NULL; if (!names.empty()) { rel = mxs_json_relationship(host, MXS_JSON_API_MONITORS); - - for (std::vector::iterator it = names.begin(); - it != names.end(); it++) + for (auto& name : names) { - mxs_json_add_relation(rel, it->c_str(), CN_MONITORS); + mxs_json_add_relation(rel, name.c_str(), CN_MONITORS); } } diff --git a/server/modules/monitor/clustrixmon/clustrix.cc b/server/modules/monitor/clustrixmon/clustrix.cc index 876ee792e..b877a09c0 100644 --- a/server/modules/monitor/clustrixmon/clustrix.cc +++ b/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)) { diff --git a/server/modules/monitor/mariadbmon/mariadbserver.cc b/server/modules/monitor/mariadbmon/mariadbserver.cc index 520c6ad9c..010031041 100644 --- a/server/modules/monitor/mariadbmon/mariadbserver.cc +++ b/server/modules/monitor/mariadbmon/mariadbserver.cc @@ -29,6 +29,7 @@ using maxbase::StopWatch; using maxsql::QueryResult; using Guard = std::lock_guard; 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(); }