Rearrange MariaDB-Monitor settings variables

Moves some of the settings to a dedicated container to separate them
from other fields.

server/modules/monitor/mariadbmon/cluster_discovery.cc

@@ -490,7 +490,7 @@ void MariaDBMonitor::assign_server_roles()
     // Check the the master node, label it as the [Master] if
     // 1) the node has slaves, even if their slave sql threads are stopped
     // 2) or detect standalone master is on.
-    if (m_master && (!m_master->m_node.children.empty() || m_detect_standalone_master))
+    if (m_master && (!m_master->m_node.children.empty() || m_settings.detect_standalone_master))
     {
         if (m_master->is_running())
         {
@@ -508,7 +508,7 @@ void MariaDBMonitor::assign_server_roles()
                 m_master->set_status(SERVER_MASTER | SERVER_WAS_MASTER);
             }
         }
-        else if (m_detect_stale_master && (m_master->had_status(SERVER_WAS_MASTER)))
+        else if (m_settings.detect_stale_master && (m_master->had_status(SERVER_WAS_MASTER)))
         {
             // The master is not running but it was the master last round and
             // may have running slaves who have up-to-date events.
@@ -520,7 +520,7 @@ void MariaDBMonitor::assign_server_roles()
         assign_slave_and_relay_master(m_master);
     }
 
-    if (!m_ignore_external_masters)
+    if (!m_settings.ignore_external_masters)
     {
         // Do a sweep through all the nodes in the cluster (even the master) and mark external slaves.
         for (MariaDBServer* server : m_servers)
@@ -563,7 +563,7 @@ void MariaDBMonitor::assign_slave_and_relay_master(MariaDBServer* start_node)
     QueueElement start = {start_node, start_node->is_running()};
     open_set.push(start);
     int next_index = NodeData::INDEX_FIRST;
-    const bool allow_stale_slaves = m_detect_stale_slave;
+    const bool allow_stale_slaves = m_settings.detect_stale_slave;
 
     while (!open_set.empty())
     {
@@ -695,12 +695,12 @@ bool MariaDBMonitor::master_is_valid(std::string* reason_out)
     else if (m_master->is_down())
     {
         // These two conditionals are separate since cases 4&5 should not apply if master is down.
-        if (m_master->m_server_base->mon_err_count > m_failcount && running_slaves(m_master) == 0)
+        if (m_master->m_server_base->mon_err_count > m_settings.failcount && running_slaves(m_master) == 0)
         {
             rval = false;
             reason = string_printf("it has been down over %d (failcount) monitor updates and "
                                    "it does not have any running slaves",
-                                   m_failcount);
+                                   m_settings.failcount);
         }
     }
     // 4) The master was a non-replicating master (not in a cycle) but now has a slave connection.

server/modules/monitor/mariadbmon/cluster_manipulation.cc

@@ -1445,9 +1445,10 @@ void MariaDBMonitor::handle_auto_failover()
     Duration event_age;
     Duration delay_time;
 
-    if (m_failcount > 1 && m_warn_master_down)
+    const auto failcount = m_settings.failcount;
+    if (failcount > 1 && m_warn_master_down)
     {
-        int monitor_passes = m_failcount - master_down_count;
+        int monitor_passes = failcount - master_down_count;
         MXS_WARNING("Master has failed. If master status does not change in %d monitor passes, failover "
                     "begins.",
                     (monitor_passes > 1) ? monitor_passes : 1);
@@ -1461,7 +1462,7 @@ void MariaDBMonitor::handle_auto_failover()
                    "seconds ago. Delaying failover for at least %.1f seconds.",
                    connected_slave->name(), m_master->name(), event_age.secs(), delay_time.secs());
     }
-    else if (master_down_count >= m_failcount)
+    else if (master_down_count >= failcount)
     {
         // Failover is required, but first we should check if preconditions are met.
         Log log_mode = m_warn_failover_precond ? Log::ON : Log::OFF;
@@ -1861,13 +1862,14 @@ ServerArray MariaDBMonitor::get_redirectables(const MariaDBServer* old_master,
 
 void MariaDBMonitor::delay_auto_cluster_ops()
 {
-    if (m_auto_failover || m_auto_rejoin || m_enforce_read_only_slaves || m_switchover_on_low_disk_space)
+    if (m_settings.auto_failover || m_settings.auto_rejoin || m_settings.enforce_read_only_slaves
+        || m_settings.switchover_on_low_disk_space)
     {
         const char DISABLING_AUTO_OPS[] = "Disabling automatic cluster operations for %i monitor ticks.";
-        MXS_NOTICE(DISABLING_AUTO_OPS, m_failcount);
+        MXS_NOTICE(DISABLING_AUTO_OPS, m_settings.failcount);
     }
     // + 1 because the start of next tick subtracts 1.
-    cluster_operation_disable_timer = m_failcount + 1;
+    cluster_operation_disable_timer = m_settings.failcount + 1;
 }
 
 bool MariaDBMonitor::can_perform_cluster_ops()

server/modules/monitor/mariadbmon/mariadbmon.cc

@@ -213,23 +213,23 @@ bool MariaDBMonitor::configure(const MXS_CONFIG_PARAMETER* params)
         return false;
     }
 
-    m_detect_stale_master = params->get_bool("detect_stale_master");
+    m_settings.detect_stale_master = params->get_bool("detect_stale_master");
-    m_detect_stale_slave = params->get_bool("detect_stale_slave");
+    m_settings.detect_stale_slave = params->get_bool("detect_stale_slave");
-    m_ignore_external_masters = params->get_bool("ignore_external_masters");
+    m_settings.detect_standalone_master = params->get_bool(CN_DETECT_STANDALONE_MASTER);
-    m_detect_standalone_master = params->get_bool(CN_DETECT_STANDALONE_MASTER);
+    m_settings.ignore_external_masters = params->get_bool("ignore_external_masters");
     m_assume_unique_hostnames = params->get_bool(CN_ASSUME_UNIQUE_HOSTNAMES);
-    m_failcount = params->get_integer(CN_FAILCOUNT);
+    m_settings.failcount = params->get_integer(CN_FAILCOUNT);
     m_failover_timeout = params->get_duration<std::chrono::seconds>(CN_FAILOVER_TIMEOUT).count();
     m_switchover_timeout = params->get_duration<std::chrono::seconds>(CN_SWITCHOVER_TIMEOUT).count();
-    m_auto_failover = params->get_bool(CN_AUTO_FAILOVER);
+    m_settings.auto_failover = params->get_bool(CN_AUTO_FAILOVER);
-    m_auto_rejoin = params->get_bool(CN_AUTO_REJOIN);
+    m_settings.auto_rejoin = params->get_bool(CN_AUTO_REJOIN);
-    m_enforce_read_only_slaves = params->get_bool(CN_ENFORCE_READONLY);
+    m_settings.enforce_read_only_slaves = params->get_bool(CN_ENFORCE_READONLY);
     m_verify_master_failure = params->get_bool(CN_VERIFY_MASTER_FAILURE);
     m_master_failure_timeout = params->get_duration<std::chrono::seconds>(CN_MASTER_FAILURE_TIMEOUT).count();
     m_promote_sql_file = params->get_string(CN_PROMOTION_SQL_FILE);
     m_demote_sql_file = params->get_string(CN_DEMOTION_SQL_FILE);
-    m_switchover_on_low_disk_space = params->get_bool(CN_SWITCHOVER_ON_LOW_DISK_SPACE);
+    m_settings.switchover_on_low_disk_space = params->get_bool(CN_SWITCHOVER_ON_LOW_DISK_SPACE);
-    m_maintenance_on_low_disk_space = params->get_bool(CN_MAINTENANCE_ON_LOW_DISK_SPACE);
+    m_settings.maintenance_on_low_disk_space = params->get_bool(CN_MAINTENANCE_ON_LOW_DISK_SPACE);
     m_handle_event_scheduler = params->get_bool(CN_HANDLE_EVENTS);
     m_replication_ssl = params->get_bool(CN_REPLICATION_MASTER_SSL);
 
@@ -264,17 +264,17 @@ bool MariaDBMonitor::configure(const MXS_CONFIG_PARAMETER* params)
     if (!m_assume_unique_hostnames)
     {
         const char requires[] = "%s requires that %s is on.";
-        if (m_auto_failover)
+        if (m_settings.auto_failover)
         {
             MXB_ERROR(requires, CN_AUTO_FAILOVER, CN_ASSUME_UNIQUE_HOSTNAMES);
             settings_ok = false;
         }
-        if (m_switchover_on_low_disk_space)
+        if (m_settings.switchover_on_low_disk_space)
         {
             MXB_ERROR(requires, CN_SWITCHOVER_ON_LOW_DISK_SPACE, CN_ASSUME_UNIQUE_HOSTNAMES);
             settings_ok = false;
         }
-        if (m_auto_rejoin)
+        if (m_settings.auto_rejoin)
         {
             MXB_ERROR(requires, CN_AUTO_REJOIN, CN_ASSUME_UNIQUE_HOSTNAMES);
             settings_ok = false;
@@ -299,14 +299,16 @@ string MariaDBMonitor::diagnostics_to_string() const
     string rval;
     rval.reserve(1000);     // Enough for basic output.
 
-    rval += string_printf("Automatic failover:     %s\n", m_auto_failover ? "Enabled" : "Disabled");
+    auto bool_to_zstr = [](bool val) -> const char* {
-    rval += string_printf("Failcount:              %d\n", m_failcount);
+        return val ? "Enabled" : "Disabled";
+    };
+    rval += string_printf("Automatic failover:     %s\n", bool_to_zstr(m_settings.auto_failover));
+    rval += string_printf("Failcount:              %i\n", m_settings.failcount);
     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("Automatic rejoin:       %s\n", bool_to_zstr(m_settings.auto_rejoin));
-    rval += string_printf("Enforce read-only:      %s\n", m_enforce_read_only_slaves ? "Enabled" :
+    rval += string_printf("Enforce read-only:      %s\n", bool_to_zstr(m_settings.enforce_read_only_slaves));
-                          "Disabled");
+    rval += string_printf("Detect stale master:    %s\n", bool_to_zstr(m_settings.detect_stale_master));
-    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): ");
@@ -429,7 +431,7 @@ void MariaDBMonitor::tick()
         update_topology();
         m_cluster_topology_changed = false;
         // If cluster operations are enabled, check topology support and disable if needed.
-        if (m_auto_failover || m_switchover_on_low_disk_space || m_auto_rejoin)
+        if (m_settings.auto_failover || m_settings.switchover_on_low_disk_space || m_settings.auto_rejoin)
         {
             check_cluster_operations_support();
         }
@@ -448,7 +450,7 @@ void MariaDBMonitor::tick()
 
     /* Set low disk space slaves to maintenance. This needs to happen after roles have been assigned.
      * Is not a real cluster operation, since nothing on the actual backends is changed. */
-    if (m_maintenance_on_low_disk_space)
+    if (m_settings.maintenance_on_low_disk_space)
     {
         set_low_disk_slaves_maintenance();
     }
@@ -504,14 +506,14 @@ void MariaDBMonitor::process_state_changes()
 
     if (can_perform_cluster_ops())
     {
-        if (m_auto_failover)
+        if (m_settings.auto_failover)
         {
             handle_auto_failover();
         }
 
         // Do not auto-join servers on this monitor loop if a failover (or any other cluster modification)
         // has been performed, as server states have not been updated yet. It will happen next iteration.
-        if (m_auto_rejoin && cluster_can_be_joined() && can_perform_cluster_ops())
+        if (m_settings.auto_rejoin && cluster_can_be_joined() && can_perform_cluster_ops())
         {
             // Check if any servers should be autojoined to the cluster and try to join them.
             handle_auto_rejoin();
@@ -520,13 +522,13 @@ void MariaDBMonitor::process_state_changes()
         /* Check if any slave servers have read-only off and turn it on if user so wishes. Again, do not
          * perform this if cluster has been modified this loop since it may not be clear which server
          * should be a slave. */
-        if (m_enforce_read_only_slaves && can_perform_cluster_ops())
+        if (m_settings.enforce_read_only_slaves && can_perform_cluster_ops())
         {
             enforce_read_only_on_slaves();
         }
 
         /* Check if the master server is on low disk space and act on it. */
-        if (m_switchover_on_low_disk_space && can_perform_cluster_ops())
+        if (m_settings.switchover_on_low_disk_space && can_perform_cluster_ops())
         {
             handle_low_disk_space_master();
         }

server/modules/monitor/mariadbmon/mariadbmon.hh

@@ -191,29 +191,41 @@ private:
     std::string m_external_master_host;                     /* External master host, for fail/switchover */
     int         m_external_master_port = PORT_UNKNOWN;      /* External master port */
 
+    // MariaDB-Monitor specific settings
+    class Settings
+    {
+    public:
         /* The default setting values given here may not be the actual defaults given by
          * the module configuration. */
 
         // Replication topology detection settings.
-    bool m_detect_stale_master = true;      /* Allow stale masters. TODO: Remove this */
+
-    bool m_detect_stale_slave = true;       /* Allow stale slaves: a running slave behind a downed
+        bool detect_stale_master {true};      /* Allow stale masters. TODO: Remove this */
+        bool detect_stale_slave {true};       /* Allow stale slaves: a running slave behind a downed
                                                * master/relay is still a valid slave */
-    bool m_detect_standalone_master = true; /* Allow writes to a master without any slaves.
+        bool detect_standalone_master {true}; /* Allow writes to a master without any slaves.
                                                * TODO: think about removing */
-    bool m_ignore_external_masters = false; /* Ignore masters outside of the monitor configuration.
+        bool ignore_external_masters {false}; /* Ignore masters outside of the monitor configuration.
                                                * TODO: requires work */
-    bool m_assume_unique_hostnames = true;  /* Are server hostnames consistent between MaxScale and servers */
+
-    int  m_failcount = 1;                   /* Number of ticks master must be down before it's considered
+        int failcount {1};  /* Number of ticks master must be down before it's considered
                              * totally down, allowing failover or master change. */
 
         // Cluster operations activation settings
-    bool m_auto_failover = false;                   /* Automatic master failover enabled? */
+
-    bool m_auto_rejoin = false;                     /* Automatic rejoin enabled? */
+        bool auto_failover {false};                 /* Automatic master failover enabled? */
-    bool m_switchover_on_low_disk_space = false;    /* Automatically switch over a master low on disk space */
+        bool auto_rejoin {false};                   /* Automatic rejoin enabled? */
-    bool m_maintenance_on_low_disk_space = false;   /* Automatically set slave and unreplicating servers low
+        bool switchover_on_low_disk_space {false};  /* Automatically switch over a master low on disk space */
+        bool maintenance_on_low_disk_space {false}; /* Automatically set slave and unreplicating servers low
                                                      * on disk space to maintenance. */
-    bool m_enforce_read_only_slaves = false;        /* If true, the monitor checks and enforces every tick
+        bool enforce_read_only_slaves {false};      /* If true, the monitor checks and enforces every tick
                                                      * that all slaves are in read-only-mode. */
+    };
+
+    Settings m_settings;
+
+    bool m_assume_unique_hostnames {true};  /* Are server hostnames consistent between MaxScale and servers */
+
     // Cluster operations additional settings
     std::string m_replication_user;             /* Replication user for CHANGE MASTER TO-commands */
     std::string m_replication_password;         /* Replication password for CHANGE MASTER TO-commands */