diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index 847f38ab8..712a09ab1 100644 --- a/include/maxscale/monitor.hh +++ b/include/maxscale/monitor.hh @@ -337,6 +337,26 @@ protected: */ void detect_handle_state_changes(); + /** + * @brief Called when a server has been added to the monitor. + * + * The default implementation will add the server to associated + * services. + * + * @param server A server. + */ + virtual void server_added(SERVER* server); + + /** + * @brief Called when a server has been removed from the monitor. + * + * The default implementation will remove the server from associated + * services. + * + * @param server A server. + */ + virtual void server_removed(SERVER* server); + /** * Contains monitor base class settings. Since monitors are stopped before a setting change, * the items cannot be modified while a monitor is running. No locking required. @@ -363,6 +383,11 @@ protected: Settings m_settings; +private: + void add_server(SERVER* server); + + void remove_server(SERVER* server); + private: friend class MonitorManager; @@ -395,7 +420,34 @@ private: int launch_command(MXS_MONITORED_SERVER* ptr, EXTERNCMD* cmd); /** - * Populate services with the servers of the monitor. + * @brief Add a server to a monitor. + * + * Add a server to a monitor, provided the server is not currently + * being monitored by any monitor. Before adding the server to the + * monitor, the monitor is stopped if it is running and after the + * addition it is restarted if it was running. + * + * @param monitor A monitor. + * @param server A server. + * + * @return True, if the monitor was added, false otherwise. + */ + static bool add_server(Monitor* mon, SERVER* server); + + /** + * @brief Remove a server from a monitor. + * + * If the server is being monitored by the server, remove it. + * Before removing, the monitor is stopped if it is running and after + * the removal it is restarted if it was running. + * + * @param monitor A monitor. + * @param server A server. + */ + static void remove_server(Monitor* mon, SERVER* server); + + /** + * @brief The monitor should populate associated services. */ virtual void populate_services(); diff --git a/server/core/config.cc b/server/core/config.cc index 2f3d118b8..596cb5093 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -1406,7 +1406,7 @@ std::unordered_set get_dependencies(const std::vectorparameters, CN_CLUSTER)) { - rval.insert(name_to_object(objects, obj, config_get_string(obj->parameters, CN_CLUSTER))); + rval.insert(name_to_object(objects, obj, obj->parameters->get_string(CN_CLUSTER))); } if ((type == CN_MONITOR || type == CN_SERVICE) && config_get_value(obj->parameters, CN_SERVERS)) diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 150db5b07..aefe0d67f 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -165,7 +165,7 @@ bool runtime_link_server(Server* server, const char* target) } else if (monitor) { - if (monitor_add_server(monitor, server)) + if (MonitorManager::add_server(monitor, server)) { monitor_serialize(monitor); rval = true; @@ -213,7 +213,7 @@ bool runtime_unlink_server(Server* server, const char* target) } else if (monitor) { - monitor_remove_server(monitor, server); + MonitorManager::remove_server(monitor, server); monitor_serialize(monitor); rval = true; } @@ -1434,7 +1434,7 @@ bool runtime_destroy_monitor(Monitor* monitor) while (!monitor->m_servers.empty()) { - monitor_remove_server(monitor, monitor->m_servers[0]->server); + MonitorManager::remove_server(monitor, monitor->m_servers[0]->server); } monitor_deactivate(monitor); MXS_NOTICE("Destroyed monitor '%s'", monitor->m_name); diff --git a/server/core/internal/monitor.hh b/server/core/internal/monitor.hh index 0f26c7fac..54121f728 100644 --- a/server/core/internal/monitor.hh +++ b/server/core/internal/monitor.hh @@ -112,6 +112,16 @@ public: * @brief Populate services with the servers of the monitors. */ static void populate_services(); + + static bool add_server(Monitor* mon, SERVER* server) + { + return Monitor::add_server(mon, server); + } + + static void remove_server(Monitor* mon, SERVER* server) + { + Monitor::remove_server(mon, server); + } }; @@ -139,9 +149,6 @@ void monitor_show_all(DCB*); void monitor_list(DCB*); -bool monitor_add_server(Monitor* mon, SERVER* server); -void monitor_remove_server(Monitor* mon, SERVER* server); - void monitor_set_journal_max_age(Monitor* mon, time_t value); /** diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 48138cefc..0b7bdebbc 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -206,7 +206,7 @@ bool Monitor::configure_base(const MXS_CONFIG_PARAMETER* params) for (auto elem : servers_temp) { // This function checks if server is already monitored. TODO: This should be a config error. - monitor_add_server(this, elem); + Monitor::add_server(this, elem); } /* The previous config values were normal types and were checked by the config manager @@ -350,14 +350,8 @@ void monitor_stop_all() }); } -/** - * Add a server to a monitor. Simply register the server that needs to be - * monitored to the running monitor module. - * - * @param mon The Monitor instance - * @param server The Server to add to the monitoring - */ -bool monitor_add_server(Monitor* mon, SERVER* server) +//static +bool Monitor::add_server(Monitor* mon, SERVER* server) { mxb_assert(mon && server); bool rval = false; @@ -369,8 +363,6 @@ bool monitor_add_server(Monitor* mon, SERVER* server) else { rval = true; - MXS_MONITORED_SERVER* db = new (std::nothrow) MXS_MONITORED_SERVER(server); - MXS_ABORT_IF_NULL(db); monitor_state_t old_state = mon->m_state; @@ -379,12 +371,7 @@ bool monitor_add_server(Monitor* mon, SERVER* server) monitor_stop(mon); } - { - Guard guard(mon->m_lock); - mon->m_servers.push_back(db); - } - - service_add_server(mon, server); + mon->add_server(server); if (old_state == MONITOR_STATE_RUNNING) { @@ -395,6 +382,42 @@ bool monitor_add_server(Monitor* mon, SERVER* server) return rval; } +/** + * @brief Add a server to the monitor. + * + * It is assumed that the monitor is currently not running and that the + * server is not currently being monitored. + * + * @param server A server. + */ +void Monitor::add_server(SERVER* server) +{ + mxb_assert(m_state != MONITOR_STATE_RUNNING); + mxb_assert(!monitor_server_in_use(server)); + + MXS_MONITORED_SERVER* db = new (std::nothrow) MXS_MONITORED_SERVER(server); + MXS_ABORT_IF_NULL(db); + + using Guard = std::unique_lock; + Guard guard(m_lock); + + m_servers.push_back(db); + + guard.unlock(); + + server_added(server); +} + +void Monitor::server_added(SERVER* server) +{ + service_add_server(this, server); +} + +void Monitor::server_removed(SERVER* server) +{ + service_remove_server(this, server); +} + static void monitor_server_free(MXS_MONITORED_SERVER* tofree) { if (tofree) @@ -426,7 +449,8 @@ static void monitor_server_free_all(std::vector& servers) * @param mon The Monitor instance * @param server The Server to remove */ -void monitor_remove_server(Monitor* mon, SERVER* server) +//static +void Monitor::remove_server(Monitor* mon, SERVER* server) { monitor_state_t old_state = mon->m_state; @@ -435,26 +459,7 @@ void monitor_remove_server(Monitor* mon, SERVER* server) monitor_stop(mon); } - MXS_MONITORED_SERVER* ptr = nullptr; - { - Guard guard(mon->m_lock); - for (auto iter = mon->m_servers.begin(); iter != mon->m_servers.end(); ++iter) - { - if ((*iter)->server == server) - { - ptr = *iter; - mon->m_servers.erase(iter); - break; - } - } - } - - if (ptr) - { - monitor_server_free(ptr); - - service_remove_server(mon, server); - } + mon->remove_server(server); if (old_state == MONITOR_STATE_RUNNING) { @@ -462,6 +467,43 @@ void monitor_remove_server(Monitor* mon, SERVER* server) } } +/** + * @brief Remove a server from the monitor. + * + * It is assumed that the monitor is currently not running. + * + * @param server A server + */ +void Monitor::remove_server(SERVER* server) +{ + mxb_assert(m_state != MONITOR_STATE_RUNNING); + + MXS_MONITORED_SERVER* ptr = nullptr; + + using Guard = std::unique_lock; + Guard guard(m_lock); + + for (auto it = m_servers.begin(); it != m_servers.end(); ++it) + { + if ((*it)->server == server) + { + ptr = *it; + m_servers.erase(it); + break; + } + } + + guard.unlock(); + + if (ptr) + { + monitor_server_free(ptr); + + server_removed(server); + } + +} + void Monitor::set_user(const string& user) { m_settings.conn_settings.username = user; diff --git a/server/modules/monitor/clustrixmon/clustrixmonitor.cc b/server/modules/monitor/clustrixmon/clustrixmonitor.cc index d1a9c2708..30dc2e334 100644 --- a/server/modules/monitor/clustrixmon/clustrixmonitor.cc +++ b/server/modules/monitor/clustrixmon/clustrixmonitor.cc @@ -119,6 +119,18 @@ bool ClustrixMonitor::unsoftfail(SERVER* pServer, json_t** ppError) return true; } +void ClustrixMonitor::server_added(SERVER* pServer) +{ + // The servers explicitly added to the Cluster monitor are only used + // as bootstrap servers, so they are not added to any services. +} + +void ClustrixMonitor::server_removed(SERVER* pServer) +{ + // @see server_added(), no action is needed. +} + + void ClustrixMonitor::pre_loop() { make_health_check(); diff --git a/server/modules/monitor/clustrixmon/clustrixmonitor.hh b/server/modules/monitor/clustrixmon/clustrixmonitor.hh index 18dafb06c..9b4500a2c 100644 --- a/server/modules/monitor/clustrixmon/clustrixmonitor.hh +++ b/server/modules/monitor/clustrixmon/clustrixmonitor.hh @@ -64,11 +64,15 @@ public: bool configure(const MXS_CONFIG_PARAMETER* pParams) override; - void populate_services() override; - bool softfail(SERVER* pServer, json_t** ppError); bool unsoftfail(SERVER* pServer, json_t** ppError); +protected: + void populate_services() override; + + void server_added(SERVER* pServer) override; + void server_removed(SERVER* pServer) override; + private: ClustrixMonitor(const std::string& name, const std::string& module);