From f2067fcf7cdf59ab8d0d7b44c7b59f88046d28be Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Tue, 9 Oct 2018 17:31:07 +0300 Subject: [PATCH] Monitor cleanup Removes unused code, compacts lines, moves code. No functional changes. --- .../monitor/mariadbmon/cluster_discovery.cc | 11 +- .../mariadbmon/cluster_manipulation.cc | 76 +++----- .../modules/monitor/mariadbmon/mariadbmon.cc | 167 ++++-------------- .../modules/monitor/mariadbmon/mariadbmon.hh | 8 - 4 files changed, 63 insertions(+), 199 deletions(-) diff --git a/server/modules/monitor/mariadbmon/cluster_discovery.cc b/server/modules/monitor/mariadbmon/cluster_discovery.cc index dfec1d4a7..1f32b3252 100644 --- a/server/modules/monitor/mariadbmon/cluster_discovery.cc +++ b/server/modules/monitor/mariadbmon/cluster_discovery.cc @@ -60,12 +60,6 @@ void topology_DFS(MariaDBServer* root, VisitorFunc& visitor) } } - -static bool server_config_compare(const MariaDBServer* lhs, const MariaDBServer* rhs) -{ - return lhs->m_config_index < rhs->m_config_index; -} - /** * @brief Visit a node in the graph * @@ -146,7 +140,10 @@ void MariaDBMonitor::tarjan_scc_visit_node(MariaDBServer* node, ServerArray& members = m_cycles[cycle_ind]; // Creates array if didn't exist members.push_back(cycle_server); // Sort the cycle members according to monitor config order. - std::sort(members.begin(), members.end(), server_config_compare); + std::sort(members.begin(), members.end(), + [](const MariaDBServer* lhs, const MariaDBServer* rhs) -> bool { + return lhs->m_config_index < rhs->m_config_index; + }); // All cycle elements popped. Next cycle... *next_cycle = cycle_ind + 1; } diff --git a/server/modules/monitor/mariadbmon/cluster_manipulation.cc b/server/modules/monitor/mariadbmon/cluster_manipulation.cc index f7dd887f1..ae9562b8c 100644 --- a/server/modules/monitor/mariadbmon/cluster_manipulation.cc +++ b/server/modules/monitor/mariadbmon/cluster_manipulation.cc @@ -35,10 +35,6 @@ const char FAILOVER_FAIL[] = "Failover '%s' -> '%s' failed."; const char SWITCHOVER_OK[] = "Switchover '%s' -> '%s' performed."; const char SWITCHOVER_FAIL[] = "Switchover %s -> %s failed"; -static void print_redirect_errors(MariaDBServer* first_server, - const ServerArray& servers, - json_t** err_out); - /** * Run a manual switchover, promoting a new master server and demoting the existing master. * @@ -401,8 +397,7 @@ string MariaDBMonitor::generate_change_master_cmd(const string& master_host, int * @param redirected_slaves A vector where to insert successfully redirected slaves. * @return The number of slaves successfully redirected. */ -int MariaDBMonitor::redirect_slaves(MariaDBServer* new_master, - const ServerArray& slaves, +int MariaDBMonitor::redirect_slaves(MariaDBServer* new_master, const ServerArray& slaves, ServerArray* redirected_slaves) { mxb_assert(redirected_slaves != NULL); @@ -559,7 +554,6 @@ bool MariaDBMonitor::cluster_can_be_joined() /** * Scan the servers in the cluster and add (re)joinable servers to an array. * - * @param mon Cluster monitor * @param output Array to save results to. Each element is a valid (re)joinable server according * to latest data. * @return False, if there were possible rejoinable servers but communications error to master server @@ -1111,10 +1105,8 @@ bool MariaDBMonitor::server_is_excluded(const MariaDBServer* server) * @param reason_out Why is the candidate better than current_best * @return True if candidate is better */ -bool MariaDBMonitor::is_candidate_better(const MariaDBServer* candidate, - const MariaDBServer* current_best, - const MariaDBServer* demotion_target, - uint32_t gtid_domain, +bool MariaDBMonitor::is_candidate_better(const MariaDBServer* candidate, const MariaDBServer* current_best, + const MariaDBServer* demotion_target, uint32_t gtid_domain, std::string* reason_out) { const SlaveStatus* cand_slave_conn = candidate->slave_connection_status(demotion_target); @@ -1288,11 +1280,7 @@ unique_ptr MariaDBMonitor::failover_prepare(Log log_mode, json } /** - * @brief Process possible failover event - * - * If a master failure has occurred and MaxScale is configured with failover functionality, this fuction - * executes failover to select and promote a new master server. This function should be called immediately - * after @c mon_process_state_changes. If an error occurs, this method disables automatic failover. + * Check if failover is required and perform it if so. */ void MariaDBMonitor::handle_auto_failover() { @@ -1463,39 +1451,6 @@ const MariaDBServer* MariaDBMonitor::slave_receiving_events(const MariaDBServer* return connected_slave; } -/** - * Print a redirect error to logs. If err_out exists, generate a combined error message by querying all - * the server parameters for connection errors and append these errors to err_out. - * - * @param demotion_target If not NULL, this is the first server to query. - * @param redirectable_slaves Other servers to query for errors. - * @param err_out If not null, the error output object. - */ -static void print_redirect_errors(MariaDBServer* first_server, - const ServerArray& servers, - json_t** err_out) -{ - // Individual server errors have already been printed to the log. - // For JSON, gather the errors again. - const char* const MSG = "Could not redirect any slaves to the new master."; - MXS_ERROR(MSG); - if (err_out) - { - ServerArray failed_slaves; - if (first_server) - { - failed_slaves.push_back(first_server); - } - for (auto iter = servers.begin(); iter != servers.end(); iter++) - { - failed_slaves.push_back(*iter); - } - - string combined_error = get_connection_errors(failed_slaves); - *err_out = mxs_json_error_append(*err_out, "%s Errors: %s.", MSG, combined_error.c_str()); - } -} - /** * Check cluster and parameters for suitability to switchover. Also writes found servers to output pointers. * @@ -1700,8 +1655,24 @@ void MariaDBMonitor::handle_low_disk_space_master() } } -void MariaDBMonitor::report_and_disable(const string& operation, - const string& setting_name, +void MariaDBMonitor::handle_auto_rejoin() +{ + ServerArray joinable_servers; + if (get_joinable_servers(&joinable_servers)) + { + uint32_t joins = do_rejoin(joinable_servers, NULL); + if (joins > 0) + { + MXS_NOTICE("%d server(s) redirected or rejoined the cluster.", joins); + } + } + else + { + MXS_ERROR("Query error to master '%s' prevented a possible rejoin operation.", m_master->name()); + } +} + +void MariaDBMonitor::report_and_disable(const string& operation, const string& setting_name, bool* setting_var) { string p1 = string_printf("Automatic %s failed, disabling automatic %s.", @@ -1723,8 +1694,7 @@ void MariaDBMonitor::report_and_disable(const string& operation, * @param error_out Error output * @return True if gtid is used */ -bool MariaDBMonitor::check_gtid_replication(Log log_mode, - const MariaDBServer* demotion_target, +bool MariaDBMonitor::check_gtid_replication(Log log_mode, const MariaDBServer* demotion_target, json_t** error_out) { bool gtid_domain_ok = false; diff --git a/server/modules/monitor/mariadbmon/mariadbmon.cc b/server/modules/monitor/mariadbmon/mariadbmon.cc index 4375bc0ee..023502ad6 100644 --- a/server/modules/monitor/mariadbmon/mariadbmon.cc +++ b/server/modules/monitor/mariadbmon/mariadbmon.cc @@ -269,12 +269,9 @@ string MariaDBMonitor::diagnostics_to_string() const rval += string_printf("Failover timeout: %u\n", m_failover_timeout); rval += string_printf("Switchover timeout: %u\n", m_switchover_timeout); rval += string_printf("Automatic rejoin: %s\n", m_auto_rejoin ? "Enabled" : "Disabled"); - rval += string_printf("Enforce read-only: %s\n", - m_enforce_read_only_slaves ? - "Enabled" : "Disabled"); - rval += string_printf("Detect stale master: %s\n", - (m_detect_stale_master == 1) ? - "Enabled" : "Disabled"); + rval += string_printf("Enforce read-only: %s\n", m_enforce_read_only_slaves ? "Enabled" : + "Disabled"); + rval += string_printf("Detect stale master: %s\n", m_detect_stale_master ? "Enabled" : "Disabled"); if (m_excluded_servers.size() > 0) { rval += string_printf("Non-promotable servers (failover): "); @@ -582,8 +579,7 @@ void MariaDBMonitor::update_gtid_domain() if (m_master_gtid_domain != GTID_DOMAIN_UNKNOWN && domain != m_master_gtid_domain) { MXS_NOTICE("Gtid domain id of master has changed: %" PRId64 " -> %" PRId64 ".", - m_master_gtid_domain, - domain); + m_master_gtid_domain, domain); } m_master_gtid_domain = domain; } @@ -665,23 +661,6 @@ void MariaDBMonitor::log_master_changes() } } -void MariaDBMonitor::handle_auto_rejoin() -{ - ServerArray joinable_servers; - if (get_joinable_servers(&joinable_servers)) - { - uint32_t joins = do_rejoin(joinable_servers, NULL); - if (joins > 0) - { - MXS_NOTICE("%d server(s) redirected or rejoined the cluster.", joins); - } - } - else - { - MXS_ERROR("Query error to master '%s' prevented a possible rejoin operation.", m_master->name()); - } -} - void MariaDBMonitor::assign_new_master(MariaDBServer* new_master) { m_master = new_master; @@ -778,8 +757,7 @@ bool MariaDBMonitor::execute_manual_command(std::function command, j return rval; } -bool MariaDBMonitor::run_manual_switchover(SERVER* promotion_server, - SERVER* demotion_server, +bool MariaDBMonitor::run_manual_switchover(SERVER* promotion_server, SERVER* demotion_server, json_t** error_out) { bool rval = false; @@ -949,21 +927,6 @@ string monitored_servers_to_string(const ServerArray& servers) return rval; } -string get_connection_errors(const ServerArray& servers) -{ - // Get errors from all connections, form a string. - string rval; - string separator; - for (auto iter = servers.begin(); iter != servers.end(); iter++) - { - const char* error = mysql_error((*iter)->m_server_base->con); - mxb_assert(*error); // Every connection should have an error. - rval += separator + (*iter)->name() + ": '" + error + "'"; - separator = ", "; - } - return rval; -} - /** * The module entry point routine. This routine populates the module object structure. * @@ -983,12 +946,8 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE() {MODULECMD_ARG_SERVER | MODULECMD_ARG_OPTIONAL, "Current master (optional)"} }; - modulecmd_register_command(MXS_MODULE_NAME, - "switchover", - MODULECMD_TYPE_ACTIVE, - handle_manual_switchover, - MXS_ARRAY_NELEMS(switchover_argv), - switchover_argv, + modulecmd_register_command(MXS_MODULE_NAME, "switchover", MODULECMD_TYPE_ACTIVE, + handle_manual_switchover, MXS_ARRAY_NELEMS(switchover_argv), switchover_argv, "Perform master switchover"); static modulecmd_arg_type_t failover_argv[] = @@ -999,12 +958,8 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE() }, }; - modulecmd_register_command(MXS_MODULE_NAME, - "failover", - MODULECMD_TYPE_ACTIVE, - handle_manual_failover, - MXS_ARRAY_NELEMS(failover_argv), - failover_argv, + modulecmd_register_command(MXS_MODULE_NAME, "failover", MODULECMD_TYPE_ACTIVE, + handle_manual_failover, MXS_ARRAY_NELEMS(failover_argv), failover_argv, "Perform master failover"); static modulecmd_arg_type_t rejoin_argv[] = @@ -1016,12 +971,8 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE() {MODULECMD_ARG_SERVER, "Joining server"} }; - modulecmd_register_command(MXS_MODULE_NAME, - "rejoin", - MODULECMD_TYPE_ACTIVE, - handle_manual_rejoin, - MXS_ARRAY_NELEMS(rejoin_argv), - rejoin_argv, + modulecmd_register_command(MXS_MODULE_NAME, "rejoin", MODULECMD_TYPE_ACTIVE, + handle_manual_rejoin, MXS_ARRAY_NELEMS(rejoin_argv), rejoin_argv, "Rejoin server to a cluster"); static modulecmd_arg_type_t reset_gtid_argv[] = @@ -1033,9 +984,7 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE() {MODULECMD_ARG_SERVER | MODULECMD_ARG_OPTIONAL, "Master server (optional)"} }; - modulecmd_register_command(MXS_MODULE_NAME, - "reset-replication", - MODULECMD_TYPE_ACTIVE, + modulecmd_register_command(MXS_MODULE_NAME, "reset-replication", MODULECMD_TYPE_ACTIVE, handle_manual_reset_replication, MXS_ARRAY_NELEMS(reset_gtid_argv), reset_gtid_argv, "Delete slave connections, delete binary logs and " @@ -1056,123 +1005,79 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE() NULL, /* Thread finish. */ { { - "detect_replication_lag", - MXS_MODULE_PARAM_BOOL, - "false" + "detect_replication_lag", MXS_MODULE_PARAM_BOOL, "false" }, { - "detect_stale_master", - MXS_MODULE_PARAM_BOOL, - "true" + "detect_stale_master", MXS_MODULE_PARAM_BOOL, "true" }, { - "detect_stale_slave", - MXS_MODULE_PARAM_BOOL, - "true" + "detect_stale_slave", MXS_MODULE_PARAM_BOOL, "true" }, { - "mysql51_replication", - MXS_MODULE_PARAM_BOOL, - "false", + "mysql51_replication", MXS_MODULE_PARAM_BOOL, "false", MXS_MODULE_OPT_DEPRECATED }, { - "multimaster", - MXS_MODULE_PARAM_BOOL, - "false", + "multimaster", MXS_MODULE_PARAM_BOOL, "false", MXS_MODULE_OPT_DEPRECATED }, { - CN_DETECT_STANDALONE_MASTER, - MXS_MODULE_PARAM_BOOL, - "true" + CN_DETECT_STANDALONE_MASTER, MXS_MODULE_PARAM_BOOL, "true" }, { - CN_FAILCOUNT, - MXS_MODULE_PARAM_COUNT, - "5" + CN_FAILCOUNT, MXS_MODULE_PARAM_COUNT, "5" }, { - "allow_cluster_recovery", - MXS_MODULE_PARAM_BOOL, - "true", + "allow_cluster_recovery", MXS_MODULE_PARAM_BOOL, "true", MXS_MODULE_OPT_DEPRECATED }, { - "ignore_external_masters", - MXS_MODULE_PARAM_BOOL, - "false" + "ignore_external_masters", MXS_MODULE_PARAM_BOOL, "false" }, { - CN_AUTO_FAILOVER, - MXS_MODULE_PARAM_BOOL, - "false" + CN_AUTO_FAILOVER, MXS_MODULE_PARAM_BOOL, "false" }, { - CN_FAILOVER_TIMEOUT, - MXS_MODULE_PARAM_COUNT, - - "90" + CN_FAILOVER_TIMEOUT, MXS_MODULE_PARAM_COUNT, "90" }, { - CN_SWITCHOVER_TIMEOUT, - MXS_MODULE_PARAM_COUNT, - "90" + CN_SWITCHOVER_TIMEOUT, MXS_MODULE_PARAM_COUNT, "90" }, { - CN_REPLICATION_USER, - MXS_MODULE_PARAM_STRING + CN_REPLICATION_USER, MXS_MODULE_PARAM_STRING }, { - CN_REPLICATION_PASSWORD, - MXS_MODULE_PARAM_STRING + CN_REPLICATION_PASSWORD, MXS_MODULE_PARAM_STRING }, { - CN_VERIFY_MASTER_FAILURE, - MXS_MODULE_PARAM_BOOL, - "true" + CN_VERIFY_MASTER_FAILURE, MXS_MODULE_PARAM_BOOL, "true" }, { - CN_MASTER_FAILURE_TIMEOUT, - MXS_MODULE_PARAM_COUNT, - "10" + CN_MASTER_FAILURE_TIMEOUT, MXS_MODULE_PARAM_COUNT, "10" }, { - CN_AUTO_REJOIN, - MXS_MODULE_PARAM_BOOL, - "false" + CN_AUTO_REJOIN, MXS_MODULE_PARAM_BOOL, "false" }, { - CN_ENFORCE_READONLY, - MXS_MODULE_PARAM_BOOL, - "false" + CN_ENFORCE_READONLY, MXS_MODULE_PARAM_BOOL, "false" }, { - CN_NO_PROMOTE_SERVERS, - MXS_MODULE_PARAM_SERVERLIST + CN_NO_PROMOTE_SERVERS, MXS_MODULE_PARAM_SERVERLIST }, { - CN_PROMOTION_SQL_FILE, - MXS_MODULE_PARAM_PATH + CN_PROMOTION_SQL_FILE, MXS_MODULE_PARAM_PATH }, { - CN_DEMOTION_SQL_FILE, - MXS_MODULE_PARAM_PATH + CN_DEMOTION_SQL_FILE, MXS_MODULE_PARAM_PATH }, { - CN_SWITCHOVER_ON_LOW_DISK_SPACE, - MXS_MODULE_PARAM_BOOL, - "false" + CN_SWITCHOVER_ON_LOW_DISK_SPACE, MXS_MODULE_PARAM_BOOL, "false" }, { - CN_MAINTENANCE_ON_LOW_DISK_SPACE, - MXS_MODULE_PARAM_BOOL, - "true" + CN_MAINTENANCE_ON_LOW_DISK_SPACE, MXS_MODULE_PARAM_BOOL, "true" }, { - CN_HANDLE_EVENTS, - MXS_MODULE_PARAM_BOOL, - "true" + CN_HANDLE_EVENTS, MXS_MODULE_PARAM_BOOL, "true" }, {MXS_END_MODULE_PARAMS} } diff --git a/server/modules/monitor/mariadbmon/mariadbmon.hh b/server/modules/monitor/mariadbmon/mariadbmon.hh index 99ba3993a..11406f9c6 100644 --- a/server/modules/monitor/mariadbmon/mariadbmon.hh +++ b/server/modules/monitor/mariadbmon/mariadbmon.hh @@ -317,11 +317,3 @@ private: * @return Server names */ std::string monitored_servers_to_string(const ServerArray& servers); - -/** - * Get MariaDB connection error strings from all the given servers, form one string. - * - * @param servers Servers with errors - * @return Concatenated string. - */ -std::string get_connection_errors(const ServerArray& servers);