From c7eb0a9958533216ea088ca067d3ca8145ea700f Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Wed, 16 May 2018 12:40:50 +0300 Subject: [PATCH] MXS-1775 Thread starting is now handled by MonitorInstance --- include/maxscale/monitor.hh | 5 +- server/core/monitor.cc | 44 +++++++++++++++- server/modules/monitor/auroramon/auroramon.cc | 50 ------------------ server/modules/monitor/auroramon/auroramon.hh | 1 - server/modules/monitor/galeramon/galeramon.cc | 48 ----------------- server/modules/monitor/galeramon/galeramon.hh | 1 - server/modules/monitor/grmon/grmon.cc | 38 +------------- server/modules/monitor/grmon/grmon.hh | 1 - server/modules/monitor/mmmon/mmmon.cc | 51 ------------------- server/modules/monitor/mmmon/mmmon.hh | 1 - .../monitor/ndbclustermon/ndbclustermon.cc | 50 ------------------ .../monitor/ndbclustermon/ndbclustermon.hh | 1 - 12 files changed, 48 insertions(+), 243 deletions(-) diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index 7ddc9503f..e49e5e91d 100644 --- a/include/maxscale/monitor.hh +++ b/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. */ }; /** diff --git a/server/core/monitor.cc b/server/core/monitor.cc index a543eb04b..37a565f26 100644 --- a/server/core/monitor.cc +++ b/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; diff --git a/server/modules/monitor/auroramon/auroramon.cc b/server/modules/monitor/auroramon/auroramon.cc index 2c0ec29fe..277291b73 100644 --- a/server/modules/monitor/auroramon/auroramon.cc +++ b/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); } /** diff --git a/server/modules/monitor/auroramon/auroramon.hh b/server/modules/monitor/auroramon/auroramon.hh index c56e1d334..a7d24ca4f 100644 --- a/server/modules/monitor/auroramon/auroramon.hh +++ b/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; diff --git a/server/modules/monitor/galeramon/galeramon.cc b/server/modules/monitor/galeramon/galeramon.cc index 0966241a0..a654306a5 100644 --- a/server/modules/monitor/galeramon/galeramon.cc +++ b/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 */ diff --git a/server/modules/monitor/galeramon/galeramon.hh b/server/modules/monitor/galeramon/galeramon.hh index 7e21fe380..1ed57ee97 100644 --- a/server/modules/monitor/galeramon/galeramon.hh +++ b/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; diff --git a/server/modules/monitor/grmon/grmon.cc b/server/modules/monitor/grmon/grmon.cc index 523bd3cee..87240189d 100644 --- a/server/modules/monitor/grmon/grmon.cc +++ b/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; } diff --git a/server/modules/monitor/grmon/grmon.hh b/server/modules/monitor/grmon/grmon.hh index 04fdb0b25..75b26d23c 100644 --- a/server/modules/monitor/grmon/grmon.hh +++ b/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; diff --git a/server/modules/monitor/mmmon/mmmon.cc b/server/modules/monitor/mmmon/mmmon.cc index 68c74c77e..ed7dd50e1 100644 --- a/server/modules/monitor/mmmon/mmmon.cc +++ b/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"); } diff --git a/server/modules/monitor/mmmon/mmmon.hh b/server/modules/monitor/mmmon/mmmon.hh index f0a0e724d..9bec27bfd 100644 --- a/server/modules/monitor/mmmon/mmmon.hh +++ b/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; diff --git a/server/modules/monitor/ndbclustermon/ndbclustermon.cc b/server/modules/monitor/ndbclustermon/ndbclustermon.cc index 7e48a99e2..73524686f 100644 --- a/server/modules/monitor/ndbclustermon/ndbclustermon.cc +++ b/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); } /** diff --git a/server/modules/monitor/ndbclustermon/ndbclustermon.hh b/server/modules/monitor/ndbclustermon/ndbclustermon.hh index be0204cc0..0a9b502a6 100644 --- a/server/modules/monitor/ndbclustermon/ndbclustermon.hh +++ b/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;