Monitor cleanup

Removes unused code, compacts lines, moves code.
No functional changes.

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;
                     }

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,
+int MariaDBMonitor::redirect_slaves(MariaDBServer* new_master, const ServerArray& slaves,
-                                    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,
+bool MariaDBMonitor::is_candidate_better(const MariaDBServer* candidate, const MariaDBServer* current_best,
-                                         const MariaDBServer* current_best,
+                                         const MariaDBServer* demotion_target, uint32_t gtid_domain,
-                                         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<ClusterOperation> MariaDBMonitor::failover_prepare(Log log_mode, json
 }
 
 /**
- * @brief Process possible failover event
+ * Check if failover is required and perform it if so.
- *
- * 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.
  */
 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,
+void MariaDBMonitor::handle_auto_rejoin()
-                                        const string& setting_name,
+{
+    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,
+bool MariaDBMonitor::check_gtid_replication(Log log_mode, const MariaDBServer* demotion_target,
-                                            const MariaDBServer* demotion_target,
                                             json_t** error_out)
 {
     bool gtid_domain_ok = false;

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",
+    rval += string_printf("Enforce read-only:      %s\n", m_enforce_read_only_slaves ? "Enabled" :
-                          m_enforce_read_only_slaves ?
+                          "Disabled");
-                          "Enabled" : "Disabled");
+    rval += string_printf("Detect stale master:    %s\n", m_detect_stale_master ? "Enabled" : "Disabled");
-    rval += string_printf("Detect stale master:    %s\n",
-                          (m_detect_stale_master == 1) ?
-                          "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,
+                   m_master_gtid_domain, 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<void(void)> command, j
     return rval;
 }
 
-bool MariaDBMonitor::run_manual_switchover(SERVER* promotion_server,
+bool MariaDBMonitor::run_manual_switchover(SERVER* promotion_server, SERVER* demotion_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,
+    modulecmd_register_command(MXS_MODULE_NAME, "switchover", MODULECMD_TYPE_ACTIVE,
-                               "switchover",
+                               handle_manual_switchover, MXS_ARRAY_NELEMS(switchover_argv), switchover_argv,
-                               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,
+    modulecmd_register_command(MXS_MODULE_NAME, "failover", MODULECMD_TYPE_ACTIVE,
-                               "failover",
+                               handle_manual_failover, MXS_ARRAY_NELEMS(failover_argv), failover_argv,
-                               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,
+    modulecmd_register_command(MXS_MODULE_NAME, "rejoin", MODULECMD_TYPE_ACTIVE,
-                               "rejoin",
+                               handle_manual_rejoin, MXS_ARRAY_NELEMS(rejoin_argv), rejoin_argv,
-                               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,
+    modulecmd_register_command(MXS_MODULE_NAME, "reset-replication", MODULECMD_TYPE_ACTIVE,
-                               "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",
+                "detect_replication_lag",            MXS_MODULE_PARAM_BOOL,   "false"
-                MXS_MODULE_PARAM_BOOL,
-                "false"
             },
             {
-                "detect_stale_master",
+                "detect_stale_master",               MXS_MODULE_PARAM_BOOL,   "true"
-                MXS_MODULE_PARAM_BOOL,
-                "true"
             },
             {
-                "detect_stale_slave",
+                "detect_stale_slave",                MXS_MODULE_PARAM_BOOL,   "true"
-                MXS_MODULE_PARAM_BOOL,
-                "true"
             },
             {
-                "mysql51_replication",
+                "mysql51_replication",               MXS_MODULE_PARAM_BOOL,   "false",
-                MXS_MODULE_PARAM_BOOL,
-                "false",
                 MXS_MODULE_OPT_DEPRECATED
             },
             {
-                "multimaster",
+                "multimaster",                       MXS_MODULE_PARAM_BOOL,   "false",
-                MXS_MODULE_PARAM_BOOL,
-                "false",
                 MXS_MODULE_OPT_DEPRECATED
             },
             {
-                CN_DETECT_STANDALONE_MASTER,
+                CN_DETECT_STANDALONE_MASTER,         MXS_MODULE_PARAM_BOOL,   "true"
-                MXS_MODULE_PARAM_BOOL,
-                "true"
             },
             {
-                CN_FAILCOUNT,
+                CN_FAILCOUNT,                        MXS_MODULE_PARAM_COUNT,  "5"
-                MXS_MODULE_PARAM_COUNT,
-                "5"
             },
             {
-                "allow_cluster_recovery",
+                "allow_cluster_recovery",            MXS_MODULE_PARAM_BOOL,   "true",
-                MXS_MODULE_PARAM_BOOL,
-                "true",
                 MXS_MODULE_OPT_DEPRECATED
             },
             {
-                "ignore_external_masters",
+                "ignore_external_masters",           MXS_MODULE_PARAM_BOOL,   "false"
-                MXS_MODULE_PARAM_BOOL,
-                "false"
             },
             {
-                CN_AUTO_FAILOVER,
+                CN_AUTO_FAILOVER,                    MXS_MODULE_PARAM_BOOL,   "false"
-                MXS_MODULE_PARAM_BOOL,
-                "false"
             },
             {
-                CN_FAILOVER_TIMEOUT,
+                CN_FAILOVER_TIMEOUT,                 MXS_MODULE_PARAM_COUNT,  "90"
-                MXS_MODULE_PARAM_COUNT,
-
-                "90"
             },
             {
-                CN_SWITCHOVER_TIMEOUT,
+                CN_SWITCHOVER_TIMEOUT,               MXS_MODULE_PARAM_COUNT,  "90"
-                MXS_MODULE_PARAM_COUNT,
-                "90"
             },
             {
-                CN_REPLICATION_USER,
+                CN_REPLICATION_USER,                 MXS_MODULE_PARAM_STRING
-                MXS_MODULE_PARAM_STRING
             },
             {
-                CN_REPLICATION_PASSWORD,
+                CN_REPLICATION_PASSWORD,             MXS_MODULE_PARAM_STRING
-                MXS_MODULE_PARAM_STRING
             },
             {
-                CN_VERIFY_MASTER_FAILURE,
+                CN_VERIFY_MASTER_FAILURE,            MXS_MODULE_PARAM_BOOL,   "true"
-                MXS_MODULE_PARAM_BOOL,
-                "true"
             },
             {
-                CN_MASTER_FAILURE_TIMEOUT,
+                CN_MASTER_FAILURE_TIMEOUT,           MXS_MODULE_PARAM_COUNT,  "10"
-                MXS_MODULE_PARAM_COUNT,
-                "10"
             },
             {
-                CN_AUTO_REJOIN,
+                CN_AUTO_REJOIN,                      MXS_MODULE_PARAM_BOOL,   "false"
-                MXS_MODULE_PARAM_BOOL,
-                "false"
             },
             {
-                CN_ENFORCE_READONLY,
+                CN_ENFORCE_READONLY,                 MXS_MODULE_PARAM_BOOL,   "false"
-                MXS_MODULE_PARAM_BOOL,
-                "false"
             },
             {
-                CN_NO_PROMOTE_SERVERS,
+                CN_NO_PROMOTE_SERVERS,               MXS_MODULE_PARAM_SERVERLIST
-                MXS_MODULE_PARAM_SERVERLIST
             },
             {
-                CN_PROMOTION_SQL_FILE,
+                CN_PROMOTION_SQL_FILE,               MXS_MODULE_PARAM_PATH
-                MXS_MODULE_PARAM_PATH
             },
             {
-                CN_DEMOTION_SQL_FILE,
+                CN_DEMOTION_SQL_FILE,                MXS_MODULE_PARAM_PATH
-                MXS_MODULE_PARAM_PATH
             },
             {
-                CN_SWITCHOVER_ON_LOW_DISK_SPACE,
+                CN_SWITCHOVER_ON_LOW_DISK_SPACE,     MXS_MODULE_PARAM_BOOL,   "false"
-                MXS_MODULE_PARAM_BOOL,
-                "false"
             },
             {
-                CN_MAINTENANCE_ON_LOW_DISK_SPACE,
+                CN_MAINTENANCE_ON_LOW_DISK_SPACE,    MXS_MODULE_PARAM_BOOL,   "true"
-                MXS_MODULE_PARAM_BOOL,
-                "true"
             },
             {
-                CN_HANDLE_EVENTS,
+                CN_HANDLE_EVENTS,                    MXS_MODULE_PARAM_BOOL,   "true"
-                MXS_MODULE_PARAM_BOOL,
-                "true"
             },
             {MXS_END_MODULE_PARAMS}
         }

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);