MXS-1775 Thread starting is now handled by MonitorInstance

include/maxscale/monitor.hh

@@ -27,6 +27,7 @@ public:
 
     virtual ~MonitorInstance();
 
+    bool start(const MXS_CONFIG_PARAMETER* param);
     void stop();
 
 protected:
@@ -40,12 +41,14 @@ protected:
     static void main(void* pArg);
 
     int          m_status;   /**< The current status of the monitor. */
-    THREAD       m_thread;   /**< The thread handle of the monitoring thread. */
     MXS_MONITOR* m_monitor;  /**< The generic monitor structure. */
     int32_t      m_shutdown; /**< Non-zero if the monitor should shut down. */
     char*        m_script;   /**< Launchable script. */
     uint64_t     m_events;   /**< Enabled monitor events. */
     bool         m_checked;  /**< Whether server access has been checked. */
+
+private:
+    THREAD       m_thread;   /**< The thread handle of the monitoring thread. */
 };
 
 /**

server/core/monitor.cc

@@ -2508,11 +2508,11 @@ namespace maxscale
 
 MonitorInstance::MonitorInstance(MXS_MONITOR* pMonitor)
     : m_status(0)
-    , m_thread(0)
     , m_monitor(pMonitor)
     , m_shutdown(0)
     , m_script(NULL)
     , m_events(0)
+    , m_thread(0)
 {
 }
 
@@ -2535,6 +2535,48 @@ void MonitorInstance::stop()
     m_script = NULL;
 }
 
+bool MonitorInstance::start(const MXS_CONFIG_PARAMETER* pParams)
+{
+    bool started = false;
+
+    ss_dassert(!m_shutdown);
+    ss_dassert(!m_thread);
+    ss_dassert(!m_script);
+
+    if (!m_checked)
+    {
+        if (!has_sufficient_permissions())
+        {
+            MXS_ERROR("Failed to start monitor. See earlier errors for more information.");
+        }
+        else
+        {
+            m_checked = true;
+        }
+    }
+
+    if (m_checked)
+    {
+        m_script = config_copy_string(pParams, "script");
+        m_events = config_get_enum(pParams, "events", mxs_monitor_event_enum_values);
+
+        configure(pParams);
+
+        if (thread_start(&m_thread, &maxscale::MonitorInstance::main, this, 0) == NULL)
+        {
+            MXS_ERROR("Failed to start monitor thread for monitor '%s'.", m_monitor->name);
+            MXS_FREE(m_script);
+            m_script = NULL;
+        }
+        else
+        {
+            started = true;
+        }
+    }
+
+    return started;
+}
+
 bool MonitorInstance::has_sufficient_permissions() const
 {
     return true;

server/modules/monitor/auroramon/auroramon.cc

@@ -172,54 +172,6 @@ void AuroraMonitor::main()
     mysql_thread_end();
 }
 
-/**
- * @brief Start the monitor
- *
- * This function initializes the monitor and starts the monitoring thread.
- *
- * @param arg The MONITOR structure for this monitor
- * @param opt The configuration parameters for this monitor
- * @return Monitor handle
- */
-bool AuroraMonitor::start(const MXS_CONFIG_PARAMETER *params)
-{
-    bool started = false;
-
-    ss_dassert(!m_shutdown);
-    ss_dassert(!m_thread);
-    ss_dassert(!m_script);
-
-    if (!m_checked)
-    {
-        if (!has_sufficient_permissions())
-        {
-            MXS_ERROR("Failed to start monitor. See earlier errors for more information.");
-        }
-        else
-        {
-            m_checked = true;
-        }
-    }
-
-    if (m_checked)
-    {
-        configure(params);
-
-        if (thread_start(&m_thread, &maxscale::MonitorInstance::main, this, 0) == NULL)
-        {
-            MXS_ERROR("Failed to start monitor thread for monitor '%s'.", m_monitor->name);
-            MXS_FREE(m_script);
-            m_script = NULL;
-        }
-        else
-        {
-            started = true;
-        }
-    }
-
-    return started;
-}
-
 bool AuroraMonitor::has_sufficient_permissions() const
 {
     return check_monitor_permissions(m_monitor, "SELECT @@aurora_server_id, server_id FROM "
@@ -229,8 +181,6 @@ bool AuroraMonitor::has_sufficient_permissions() const
 
 void AuroraMonitor::configure(const MXS_CONFIG_PARAMETER* params)
 {
-    m_script = config_copy_string(params, "script");
-    m_events = config_get_enum(params, "events", mxs_monitor_event_enum_values);
 }
 
 /**

server/modules/monitor/auroramon/auroramon.hh

@@ -28,7 +28,6 @@ public:
 
     static AuroraMonitor* create(MXS_MONITOR* monitor);
     void destroy();
-    bool start(const MXS_CONFIG_PARAMETER* param);
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;
 

server/modules/monitor/galeramon/galeramon.cc

@@ -148,52 +148,6 @@ void GaleraMonitor::destroy()
     delete this;
 }
 
-/**
- * Start the instance of the monitor, returning a handle on the monitor.
- *
- * This function creates a thread to execute the actual monitoring.
- *
- * @return A handle to use when interacting with the monitor
- */
-bool GaleraMonitor::start(const MXS_CONFIG_PARAMETER *params)
-{
-    bool started = false;
-
-    ss_dassert(!m_shutdown);
-    ss_dassert(!m_thread);
-    ss_dassert(!m_script);
-
-    if (!m_checked)
-    {
-        if (!has_sufficient_permissions())
-        {
-            MXS_ERROR("Failed to start monitor. See earlier errors for more information.");
-        }
-        else
-        {
-            m_checked = true;
-        }
-    }
-
-    if (m_checked)
-    {
-        configure(params);
-
-        if (thread_start(&m_thread, &maxscale::MonitorInstance::main, this, 0) == NULL)
-        {
-            MXS_ERROR("Failed to start monitor thread for monitor '%s'.", m_monitor->name);
-            MXS_FREE(m_script);
-            m_script = NULL;
-        }
-        else
-        {
-            started = true;
-        }
-    }
-
-    return started;
-}
-
 bool GaleraMonitor::has_sufficient_permissions() const
 {
     return check_monitor_permissions(m_monitor, "SHOW STATUS LIKE 'wsrep_local_state'");
@@ -206,8 +160,6 @@ void GaleraMonitor::configure(const MXS_CONFIG_PARAMETER* params)
     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_script = config_copy_string(params, "script");
-    m_events = config_get_enum(params, "events", mxs_monitor_event_enum_values);
     m_set_donor_nodes = config_get_bool(params, "set_donor_nodes");
 
     /* Reset all data in the hashtable */

server/modules/monitor/galeramon/galeramon.hh

@@ -60,7 +60,6 @@ public:
 
     static GaleraMonitor* create(MXS_MONITOR* monitor);
     void destroy();
-    bool start(const MXS_CONFIG_PARAMETER* param);
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;
 

server/modules/monitor/grmon/grmon.cc

@@ -47,42 +47,6 @@ void GRMon::destroy()
     delete this;
 }
 
-bool GRMon::start(const MXS_CONFIG_PARAMETER* params)
-{
-    bool started = false;
-
-    ss_dassert(!m_shutdown);
-    ss_dassert(!m_thread);
-
-    if (!m_checked)
-    {
-        if (!has_sufficient_permissions())
-        {
-            MXS_ERROR("Failed to start monitor. See earlier errors for more information.");
-        }
-        else
-        {
-            m_checked = true;
-        }
-    }
-
-    if (m_checked)
-    {
-        configure(params);
-
-        if (thread_start(&m_thread, &maxscale::MonitorInstance::main, this, 0) == NULL)
-        {
-            ;
-        }
-        else
-        {
-            started = true;
-        }
-    }
-
-    return started;
-}
-
 bool GRMon::has_sufficient_permissions() const
 {
     return true;
@@ -90,8 +54,8 @@ bool GRMon::has_sufficient_permissions() const
 
 void GRMon::configure(const MXS_CONFIG_PARAMETER* params)
 {
+    // TODO: Turn MonitorInstance::m_script into a std::string and remove GRMon::m_script
     m_script = config_get_string(params, "script");
-    m_events = config_get_enum(params, "events", mxs_monitor_event_enum_values);
     m_master = NULL;
 }
 

server/modules/monitor/grmon/grmon.hh

@@ -28,7 +28,6 @@ public:
 
     static GRMon* create(MXS_MONITOR* monitor);
     void destroy();
-    bool start(const MXS_CONFIG_PARAMETER* params);
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;
 

server/modules/monitor/mmmon/mmmon.cc

@@ -94,8 +94,6 @@ MMMonitor::MMMonitor(MXS_MONITOR *monitor)
 
 MMMonitor::~MMMonitor()
 {
-    ss_dassert(!m_thread);
-    ss_dassert(!m_script);
 }
 
 // static
@@ -109,53 +107,6 @@ void MMMonitor::destroy()
     delete this;
 }
 
-/**
- * Start the instance of the monitor, returning a handle on the monitor.
- *
- * This function creates a thread to execute the actual monitoring.
- *
- * @param arg   The current handle - NULL if first start
- * @return A handle to use when interacting with the monitor
- */
-bool MMMonitor::start(const MXS_CONFIG_PARAMETER *params)
-{
-    bool started = false;
-
-    ss_dassert(!m_shutdown);
-    ss_dassert(!m_thread);
-    ss_dassert(!m_script);
-
-    if (!m_checked)
-    {
-        if (!has_sufficient_permissions())
-        {
-            MXS_ERROR("Failed to start monitor. See earlier errors for more information.");
-        }
-        else
-        {
-            m_checked = true;
-        }
-    }
-
-    if (m_checked)
-    {
-        configure(params);
-
-        if (thread_start(&m_thread, &maxscale::MonitorInstance::main, this, 0) == NULL)
-        {
-            MXS_ERROR("Failed to start monitor thread for monitor '%s'.", m_monitor->name);
-            MXS_FREE(m_script);
-            m_script = NULL;
-        }
-        else
-        {
-            started = true;
-        }
-    }
-
-    return started;
-}
-
 bool MMMonitor::has_sufficient_permissions() const
 {
     return check_monitor_permissions(m_monitor, "SHOW SLAVE STATUS");
@@ -163,8 +114,6 @@ bool MMMonitor::has_sufficient_permissions() const
 
 void MMMonitor::configure(const MXS_CONFIG_PARAMETER* params)
 {
-    m_script = config_copy_string(params, "script");
-    m_events = config_get_enum(params, "events", mxs_monitor_event_enum_values);
     m_detectStaleMaster = config_get_bool(params, "detect_stale_master");
 }
 

server/modules/monitor/mmmon/mmmon.hh

@@ -28,7 +28,6 @@ public:
 
     static MMMonitor* create(MXS_MONITOR* monitor);
     void destroy();
-    bool start(const MXS_CONFIG_PARAMETER* param);
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;
 

server/modules/monitor/ndbclustermon/ndbclustermon.cc

@@ -79,8 +79,6 @@ NDBCMonitor::NDBCMonitor(MXS_MONITOR *monitor)
 
 NDBCMonitor::~NDBCMonitor()
 {
-    ss_dassert(!m_thread);
-    ss_dassert(!m_script);
 }
 
 // static
@@ -94,52 +92,6 @@ void NDBCMonitor::destroy()
     delete this;
 }
 
-/**
- * Start the instance of the monitor, returning a handle on the monitor.
- *
- * This function creates a thread to execute the actual monitoring.
- *
- * @return A handle to use when interacting with the monitor
- */
-bool NDBCMonitor::start(const MXS_CONFIG_PARAMETER *params)
-{
-    bool started = false;
-
-    ss_dassert(!m_shutdown);
-    ss_dassert(!m_thread);
-    ss_dassert(!m_script);
-
-    if (!m_checked)
-    {
-        if (!has_sufficient_permissions())
-        {
-            MXS_ERROR("Failed to start monitor. See earlier errors for more information.");
-        }
-        else
-        {
-            m_checked = true;
-        }
-    }
-
-    if (m_checked)
-    {
-        configure(params);
-
-        if (thread_start(&m_thread, &maxscale::MonitorInstance::main, this, 0) == NULL)
-        {
-            MXS_ERROR("Failed to start monitor thread for monitor '%s'.", m_monitor->name);
-            MXS_FREE(m_script);
-            m_script = NULL;
-        }
-        else
-        {
-            started = true;
-        }
-    }
-
-    return started;
-}
-
 bool NDBCMonitor::has_sufficient_permissions() const
 {
     return check_monitor_permissions(m_monitor, "SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'");
@@ -147,8 +99,6 @@ bool NDBCMonitor::has_sufficient_permissions() const
 
 void NDBCMonitor::configure(const MXS_CONFIG_PARAMETER* params)
 {
-    m_script = config_copy_string(params, "script");
-    m_events = config_get_enum(params, "events", mxs_monitor_event_enum_values);
 }
 
 /**

server/modules/monitor/ndbclustermon/ndbclustermon.hh

@@ -28,7 +28,6 @@ public:
 
     static NDBCMonitor* create(MXS_MONITOR* monitor);
     void destroy();
-    bool start(const MXS_CONFIG_PARAMETER* param);
     void diagnostics(DCB* dcb) const;
     json_t* diagnostics_json() const;