MXS-1775 Make general cleanup

Document and rearrrange so that things look nice and tidy.

include/maxscale/monitor.hh

@@ -28,9 +28,47 @@ public:
 
     virtual ~MonitorInstance();
 
-    bool start(const MXS_CONFIG_PARAMETER* param);
+    /**
+     * @brief Starts the monitor.
+     *
+     * - Calls @c has_sufficient_permissions(), if it has not been done earlier.
+     * - Updates the 'script' and 'events' configuration paramameters.
+     * - Calls @c configure().
+     * - Starts the monitor thread.
+     *
+     * @param param  The parameters of the monitor.
+     *
+     * @return True, if the monitor started, false otherwise.
+     */
+    bool start(const MXS_CONFIG_PARAMETER* params);
+
+    /**
+     * @brief Stops the monitor.
+     *
+     * When the function returns, the monitor has stopped.
+     */
     void stop();
 
+    /**
+     * @brief Write diagnostics
+     *
+     * The implementation should write diagnostic information to the
+     * provided dcb.
+     *
+     * @param dcb  The dcb to write to.
+     */
+    virtual void diagnostics(DCB* dcb) const = 0;
+
+    /**
+     * @brief Obtain diagnostics
+     *
+     * The implementation should create a JSON object and fill it with diagnostics
+     * information.
+     *
+     * @return An object, if there is information to return, NULL otherwise.
+     */
+    virtual json_t* diagnostics_json() const = 0;
+
 protected:
     MonitorInstance(MXS_MONITOR* pMonitor);
 
@@ -38,12 +76,13 @@ protected:
     uint64_t           events() const { return m_events; }
 
     /**
-     * @brief Update server information
+     * @brief Configure the monitor.
      *
-     * The implementation should probe the server in question and update
+     * When the monitor is started, this function will be called in order
-     * the server status bits.
+     * to allow the concrete implementation to configure itself from
+     * configuration parameters. The default implementation does nothing.
      */
-    virtual void update_server_status(MXS_MONITORED_SERVER* pMonitored_server) = 0;
+    virtual void configure(const MXS_CONFIG_PARAMETER* pParams);
 
     /**
      * @brief Check whether the monitor has sufficient rights
@@ -56,13 +95,12 @@ protected:
     virtual bool has_sufficient_permissions() const;
 
     /**
-     * @brief Configure the monitor.
+     * @brief Update server information
      *
-     * When the monitor is started, this function will be called in order
+     * The implementation should probe the server in question and update
-     * to allow the concrete implementation to configure itself from
+     * the server status bits.
-     * configuration parameters. The default implementation does nothing.
      */
-    virtual void configure(const MXS_CONFIG_PARAMETER* pParams);
+    virtual void update_server_status(MXS_MONITORED_SERVER* pMonitored_server) = 0;
 
     /**
      * @brief Flush pending server status to each server.

server/modules/monitor/auroramon/auroramon.cc

@@ -40,6 +40,26 @@ AuroraMonitor* AuroraMonitor::create(MXS_MONITOR* monitor)
     return new AuroraMonitor(monitor);
 }
 
+void AuroraMonitor::diagnostics(DCB *dcb) const
+{
+}
+
+json_t* AuroraMonitor::diagnostics_json() const
+{
+    return NULL;
+}
+
+void AuroraMonitor::configure(const MXS_CONFIG_PARAMETER* params)
+{
+}
+
+bool AuroraMonitor::has_sufficient_permissions() const
+{
+    return check_monitor_permissions(m_monitor, "SELECT @@aurora_server_id, server_id FROM "
+                                     "information_schema.replica_host_status "
+                                     "WHERE session_id = 'MASTER_SESSION_ID'");
+}
+
 /**
  * @brief Update the status of a server
  *
@@ -80,38 +100,6 @@ void AuroraMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server)
     }
 }
 
-bool AuroraMonitor::has_sufficient_permissions() const
-{
-    return check_monitor_permissions(m_monitor, "SELECT @@aurora_server_id, server_id FROM "
-                                     "information_schema.replica_host_status "
-                                     "WHERE session_id = 'MASTER_SESSION_ID'");
-}
-
-void AuroraMonitor::configure(const MXS_CONFIG_PARAMETER* params)
-{
-}
-
-/**
- * Diagnostic interface
- *
- * @param dcb   DCB to send output
- * @param mon   The monitor
- */
-void AuroraMonitor::diagnostics(DCB *dcb) const
-{
-}
-
-/**
- * Diagnostic interface
- *
- * @param dcb   DCB to send output
- * @param mon   The monitor
- */
-json_t* AuroraMonitor::diagnostics_json() const
-{
-    return NULL;
-}
-
 /**
  * The module entry point routine. It is this routine that must populate the
  * structure that is referred to as the "module object", this is a structure

server/modules/monitor/auroramon/auroramon.hh

@@ -31,11 +31,11 @@ public:
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;
 
-private:
+protected:
-    AuroraMonitor(MXS_MONITOR* monitor);
+    void configure(const MXS_CONFIG_PARAMETER* params);
-
+    bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
-    bool has_sufficient_permissions() const;
+private:
-    void configure(const MXS_CONFIG_PARAMETER* params);
+    AuroraMonitor(MXS_MONITOR* monitor);
 };

server/modules/monitor/galeramon/galeramon.cc

@@ -40,11 +40,11 @@ static bool warn_erange_on_local_index = true;
 
 static MXS_MONITORED_SERVER *set_cluster_master(MXS_MONITORED_SERVER *, MXS_MONITORED_SERVER *, int);
 static void disableMasterFailback(void *, int);
-bool isGaleraEvent(mxs_monitor_event_t event);
 static int compare_node_index(const void*, const void*);
 static int compare_node_priority(const void*, const void*);
 static GALERA_NODE_INFO *nodeval_dup(const GALERA_NODE_INFO *);
 static void nodeval_free(GALERA_NODE_INFO *);
+static bool using_xtrabackup(MXS_MONITORED_SERVER *database, const char* server_string);
 
 GaleraMonitor::GaleraMonitor(MXS_MONITOR *mon)
     : maxscale::MonitorInstance(mon)
@@ -89,31 +89,6 @@ GaleraMonitor* GaleraMonitor::create(MXS_MONITOR* monitor)
     return new GaleraMonitor(monitor);
 }
 
-bool GaleraMonitor::has_sufficient_permissions() const
-{
-    return check_monitor_permissions(m_monitor, "SHOW STATUS LIKE 'wsrep_local_state'");
-}
-
-void GaleraMonitor::configure(const MXS_CONFIG_PARAMETER* params)
-{
-    m_disableMasterFailback = config_get_bool(params, "disable_master_failback");
-    m_availableWhenDonor = config_get_bool(params, "available_when_donor");
-    m_disableMasterRoleSetting = config_get_bool(params, "disable_master_role_setting");
-    m_root_node_as_master = config_get_bool(params, "root_node_as_master");
-    m_use_priority = config_get_bool(params, "use_priority");
-    m_set_donor_nodes = config_get_bool(params, "set_donor_nodes");
-    m_log_no_members = true;
-
-    /* Reset all data in the hashtable */
-    reset_cluster_info();
-}
-
-/**
- * Diagnostic interface
- *
- * @param dcb   DCB to send output
- * @param arg   The monitor handle
- */
 void GaleraMonitor::diagnostics(DCB *dcb) const
 {
     dcb_printf(dcb, "Master Failback:\t%s\n", (m_disableMasterFailback == 1) ? "off" : "on");
@@ -132,11 +107,6 @@ void GaleraMonitor::diagnostics(DCB *dcb) const
     }
 }
 
-/**
- * Diagnostic interface
- *
- * @param arg   The monitor handle
- */
 json_t* GaleraMonitor::diagnostics_json() const
 {
     json_t* rval = json_object();
@@ -160,48 +130,25 @@ json_t* GaleraMonitor::diagnostics_json() const
     return rval;
 }
 
-static bool using_xtrabackup(MXS_MONITORED_SERVER *database, const char* server_string)
+void GaleraMonitor::configure(const MXS_CONFIG_PARAMETER* params)
 {
-    bool rval = false;
+    m_disableMasterFailback = config_get_bool(params, "disable_master_failback");
-    MYSQL_RES* result;
+    m_availableWhenDonor = config_get_bool(params, "available_when_donor");
+    m_disableMasterRoleSetting = config_get_bool(params, "disable_master_role_setting");
+    m_root_node_as_master = config_get_bool(params, "root_node_as_master");
+    m_use_priority = config_get_bool(params, "use_priority");
+    m_set_donor_nodes = config_get_bool(params, "set_donor_nodes");
+    m_log_no_members = true;
 
-    if (mxs_mysql_query(database->con, "SHOW VARIABLES LIKE 'wsrep_sst_method'") == 0
+    /* Reset all data in the hashtable */
-        && (result = mysql_store_result(database->con)) != NULL)
+    reset_cluster_info();
-    {
+}
-        if (mysql_field_count(database->con) < 2)
+
-        {
+bool GaleraMonitor::has_sufficient_permissions() const
-            mysql_free_result(result);
+{
-            MXS_ERROR("Unexpected result for \"SHOW VARIABLES LIKE "
+    return check_monitor_permissions(m_monitor, "SHOW STATUS LIKE 'wsrep_local_state'");
-                      "'wsrep_sst_method'\". Expected 2 columns."
-                      " MySQL Version: %s", server_string);
-        }
-
-        MYSQL_ROW row;
-
-        while ((row = mysql_fetch_row(result)))
-        {
-            if (row[1] && strncmp(row[1], "xtrabackup", 10) == 0)
-            {
-                rval = true;
-            }
-        }
-        mysql_free_result(result);
-    }
-    else
-    {
-        mon_report_query_error(database);
-    }
-
-    return rval;
 }
 
-/**
- * Monitor an individual server. Does not deal with the setting of master or
- * slave bits, except for clearing them when a server is not joined to the
- * cluster.
- *
- * @param monitored_server  The server to probe.
- */
 void GaleraMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server)
 {
     MYSQL_ROW row;
@@ -436,6 +383,41 @@ void GaleraMonitor::tick()
     }
 }
 
+static bool using_xtrabackup(MXS_MONITORED_SERVER *database, const char* server_string)
+{
+    bool rval = false;
+    MYSQL_RES* result;
+
+    if (mxs_mysql_query(database->con, "SHOW VARIABLES LIKE 'wsrep_sst_method'") == 0
+        && (result = mysql_store_result(database->con)) != NULL)
+    {
+        if (mysql_field_count(database->con) < 2)
+        {
+            mysql_free_result(result);
+            MXS_ERROR("Unexpected result for \"SHOW VARIABLES LIKE "
+                      "'wsrep_sst_method'\". Expected 2 columns."
+                      " MySQL Version: %s", server_string);
+        }
+
+        MYSQL_ROW row;
+
+        while ((row = mysql_fetch_row(result)))
+        {
+            if (row[1] && strncmp(row[1], "xtrabackup", 10) == 0)
+            {
+                rval = true;
+            }
+        }
+        mysql_free_result(result);
+    }
+    else
+    {
+        mon_report_query_error(database);
+    }
+
+    return rval;
+}
+
 /**
  * get candidate master from all nodes
  *

server/modules/monitor/galeramon/galeramon.hh

@@ -63,6 +63,12 @@ public:
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;
 
+protected:
+    void configure(const MXS_CONFIG_PARAMETER* param);
+    bool has_sufficient_permissions() const;
+    void update_server_status(MXS_MONITORED_SERVER* monitored_server);
+    void tick();
+
 private:
     unsigned long m_id;                 /**< Monitor ID */
     int m_disableMasterFailback;        /**< Monitor flag for Galera Cluster Master failback */
@@ -83,13 +89,8 @@ private:
                              const char *candidate_uuid,
                              const int candidate_size);
     MXS_MONITORED_SERVER *get_candidate_master();
-    void update_server_status(MXS_MONITORED_SERVER* monitored_server);
     void reset_cluster_info();
     void set_cluster_members();
     void set_galera_cluster();
     void update_sst_donor_nodes(int is_cluster);
-
-    bool has_sufficient_permissions() const;
-    void configure(const MXS_CONFIG_PARAMETER* param);
-    void tick();
 };

server/modules/monitor/grmon/grmon.cc

@@ -40,15 +40,6 @@ GRMon* GRMon::create(MXS_MONITOR* monitor)
     return new GRMon(monitor);
 }
 
-bool GRMon::has_sufficient_permissions() const
-{
-    return true;
-}
-
-void GRMon::configure(const MXS_CONFIG_PARAMETER* params)
-{
-}
-
 void GRMon::diagnostics(DCB* dcb) const
 {
 }
@@ -58,6 +49,15 @@ json_t* GRMon::diagnostics_json() const
     return NULL;
 }
 
+void GRMon::configure(const MXS_CONFIG_PARAMETER* params)
+{
+}
+
+bool GRMon::has_sufficient_permissions() const
+{
+    return true;
+}
+
 static inline bool is_false(const char* value)
 {
     return strcasecmp(value, "0") == 0 ||

server/modules/monitor/grmon/grmon.hh

@@ -31,11 +31,11 @@ public:
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;
 
-private:
+protected:
-    GRMon(MXS_MONITOR* monitor);
+    void configure(const MXS_CONFIG_PARAMETER* params);
-
+    bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
-    bool has_sufficient_permissions() const;
+private:
-    void configure(const MXS_CONFIG_PARAMETER* params);
+    GRMon(MXS_MONITOR* monitor);
 };

server/modules/monitor/mmmon/mmmon.cc

@@ -52,32 +52,11 @@ MMMonitor* MMMonitor::create(MXS_MONITOR* monitor)
     return new MMMonitor(monitor);
 }
 
-bool MMMonitor::has_sufficient_permissions() const
-{
-    return check_monitor_permissions(m_monitor, "SHOW SLAVE STATUS");
-}
-
-void MMMonitor::configure(const MXS_CONFIG_PARAMETER* params)
-{
-    m_detectStaleMaster = config_get_bool(params, "detect_stale_master");
-}
-
-/**
- * Diagnostic interface
- *
- * @param dcb   DCB to print diagnostics
- * @param arg   The monitor handle
- */
 void MMMonitor::diagnostics(DCB *dcb) const
 {
     dcb_printf(dcb, "Detect Stale Master:\t%s\n", (m_detectStaleMaster == 1) ? "enabled" : "disabled");
 }
 
-/**
- * Diagnostic interface
- *
- * @param arg   The monitor handle
- */
 json_t* MMMonitor::diagnostics_json() const
 {
     json_t* rval = json_object();
@@ -85,11 +64,16 @@ json_t* MMMonitor::diagnostics_json() const
     return rval;
 }
 
-/**
+void MMMonitor::configure(const MXS_CONFIG_PARAMETER* params)
- * Monitor an individual server
+{
- *
+    m_detectStaleMaster = config_get_bool(params, "detect_stale_master");
- * @param monitored_server  The server to probe
+}
- */
+
+bool MMMonitor::has_sufficient_permissions() const
+{
+    return check_monitor_permissions(m_monitor, "SHOW SLAVE STATUS");
+}
+
 void MMMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server)
 {
     MYSQL_ROW row;

server/modules/monitor/mmmon/mmmon.hh

@@ -31,6 +31,12 @@ public:
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;
 
+protected:
+    void configure(const MXS_CONFIG_PARAMETER* params);
+    bool has_sufficient_permissions() const;
+    void update_server_status(MXS_MONITORED_SERVER* monitored_server);
+    void tick();
+
 private:
     unsigned long m_id;      /**< Monitor ID */
     int m_detectStaleMaster; /**< Monitor flag for Stale Master detection */
@@ -38,10 +44,4 @@ private:
     MMMonitor(MXS_MONITOR* monitor);
 
     MXS_MONITORED_SERVER *get_current_master();
-
-    void update_server_status(MXS_MONITORED_SERVER* monitored_server);
-
-    bool has_sufficient_permissions() const;
-    void configure(const MXS_CONFIG_PARAMETER* params);
-    void tick();
 };

server/modules/monitor/ndbclustermon/ndbclustermon.cc

@@ -21,7 +21,6 @@
 #include <maxscale/alloc.h>
 #include <maxscale/mysql_utils.h>
 
-bool isNdbEvent(mxs_monitor_event_t event);
 
 NDBCMonitor::NDBCMonitor(MXS_MONITOR *monitor)
     : maxscale::MonitorInstance(monitor)
@@ -39,42 +38,24 @@ NDBCMonitor* NDBCMonitor::create(MXS_MONITOR* monitor)
     return new NDBCMonitor(monitor);
 }
 
-bool NDBCMonitor::has_sufficient_permissions() const
+void NDBCMonitor::diagnostics(DCB *dcb) const
 {
-    return check_monitor_permissions(m_monitor, "SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'");
+}
+
+json_t* NDBCMonitor::diagnostics_json() const
+{
+    return NULL;
 }
 
 void NDBCMonitor::configure(const MXS_CONFIG_PARAMETER* params)
 {
 }
 
-/**
+bool NDBCMonitor::has_sufficient_permissions() const
- * Diagnostic interface
- *
- * @param dcb   DCB to send output
- * @param arg   The monitor handle
- */
-static void
-diagnostics(const MXS_MONITOR_INSTANCE *mon, DCB *dcb)
 {
+    return check_monitor_permissions(m_monitor, "SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'");
 }
 
-/**
- * Diagnostic interface
- *
- * @param dcb   DCB to send output
- * @param arg   The monitor handle
- */
-static json_t* diagnostics_json(const MXS_MONITOR_INSTANCE *mon)
-{
-    return NULL;
-}
-
-/**
- * Monitor an individual server
- *
- * @param database  The database to probe
- */
 void NDBCMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server)
 {
     MYSQL_ROW row;

server/modules/monitor/ndbclustermon/ndbclustermon.hh

@@ -31,13 +31,13 @@ public:
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;
 
+protected:
+    void configure(const MXS_CONFIG_PARAMETER* params);
+    bool has_sufficient_permissions() const;
+    void update_server_status(MXS_MONITORED_SERVER* monitored_server);
+
 private:
     unsigned long m_id; /**< Monitor ID */
 
     NDBCMonitor(MXS_MONITOR* monitor);
-
-    void update_server_status(MXS_MONITORED_SERVER* monitored_server);
-
-    bool has_sufficient_permissions() const;
-    void configure(const MXS_CONFIG_PARAMETER* params);
 };