From 727d3feb3bcd1f769632f36b677698580417d1f7 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 28 Jan 2019 14:00:32 +0200 Subject: [PATCH] MXS-2273 Move code for setting maintenance bit to monitor.cc Applies to being-drained as well. Better that this special handling is handled by Monitor that needs it. --- include/maxscale/monitor.hh | 24 ++++++++ server/core/monitor.cc | 115 ++++++++++++++++++++++++++++++++++++ server/core/server.cc | 65 ++------------------ 3 files changed, 143 insertions(+), 61 deletions(-) diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index 47116d213..c76bdd8d6 100644 --- a/include/maxscale/monitor.hh +++ b/include/maxscale/monitor.hh @@ -193,6 +193,30 @@ public: void set_interval(int64_t interval); + /** + * Set status of monitored server. + * + * @param srv Server, must be monitored by this monitor. + * @param bit The server status bit to be sent. + * @errmsg_out If the setting of the bit fails, on return the human readable + * reason why it could not be set. + * + * @return True, if the bit could be set. + */ + bool set_server_status(SERVER* srv, int bit, std::string* errmsg_out); + + /** + * Clear status of monitored server. + * + * @param srv Server, must be monitored by this monitor. + * @param bit The server status bit to be cleared. + * @errmsg_out If the clearing of the bit fails, on return the human readable + * reason why it could not be cleared. + * + * @return True, if the bit could be cleared. + */ + bool clear_server_status(SERVER* srv, int bit, std::string* errmsg_out); + void show(DCB* dcb); const char* const m_name; /**< Monitor instance name. TODO: change to string */ diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 684c34a8e..accd1ccc3 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -2336,6 +2336,121 @@ bool Monitor::set_disk_space_threshold(const string& dst_setting) return rv; } +namespace +{ + +bool is_monitored_server(MXS_MONITORED_SERVER* pMs, SERVER* pServer) +{ + while (pMs && (pMs->server != pServer)) + { + pMs = pMs->next; + } + + return pMs != nullptr; +} + +const char ERR_CANNOT_MODIFY[] = + "The server is monitored, so only the maintenance status can be " + "set/cleared manually. Status was not modified."; +const char WRN_REQUEST_OVERWRITTEN[] = + "Previous maintenance request was not yet read by the monitor and was overwritten."; +} + +bool Monitor::set_server_status(SERVER* srv, int bit, string* errmsg_out) +{ + mxb_assert(is_monitored_server(this->monitored_servers, srv)); + + bool written = false; + + if (this->state == MONITOR_STATE_RUNNING) + { + /* This server is monitored, in which case modifying any other status bit than Maintenance is + * disallowed. */ + if (bit & ~(SERVER_MAINT | SERVER_BEING_DRAINED)) + { + MXS_ERROR(ERR_CANNOT_MODIFY); + if (errmsg_out) + { + *errmsg_out = ERR_CANNOT_MODIFY; + } + } + else if (bit & SERVER_MAINT) + { + /* Maintenance is set/cleared using a special variable which the monitor reads when + * starting the next update cycle. */ + int previous_request = atomic_exchange_int(&srv->maint_request, SERVER::MAINTENANCE_ON); + written = true; + // Warn if the previous request hasn't been read. + if (previous_request != SERVER::MAINTENANCE_NO_CHANGE) + { + MXS_WARNING(WRN_REQUEST_OVERWRITTEN); + } + // Also set a flag so the next loop happens sooner. + atomic_store_int(&this->check_maintenance_flag, SERVER::MAINTENANCE_FLAG_CHECK); + } + else + { + // TODO: This should be set the same way the maintenance bit is set. + srv->set_status(bit); + written = true; + } + } + else + { + /* The monitor is not running, the bit can be set directly */ + srv->set_status(bit); + written = true; + } + + return written; +} + +bool Monitor::clear_server_status(SERVER* srv, int bit, string* errmsg_out) +{ + mxb_assert(is_monitored_server(this->monitored_servers, srv)); + + bool written = false; + + if (this->state == MONITOR_STATE_RUNNING) + { + if (bit & ~(SERVER_MAINT | SERVER_BEING_DRAINED)) + { + MXS_ERROR(ERR_CANNOT_MODIFY); + if (errmsg_out) + { + *errmsg_out = ERR_CANNOT_MODIFY; + } + } + else if (bit & SERVER_MAINT) + { + // Warn if the previous request hasn't been read. + int previous_request = atomic_exchange_int(&srv->maint_request, SERVER::MAINTENANCE_OFF); + written = true; + if (previous_request != SERVER::MAINTENANCE_NO_CHANGE) + { + MXS_WARNING(WRN_REQUEST_OVERWRITTEN); + } + atomic_store_int(&this->check_maintenance_flag, SERVER::MAINTENANCE_FLAG_CHECK); + } + else + { + // TODO: This should be set the same way the maintenance bit is set. + srv->clear_status(bit); + written = true; + } + } + else + { + /* The monitor is not running, the bit can be cleared directly */ + srv->clear_status(bit); + written = true; + } + + return written; + + +} + void monitor_debug_wait() { using namespace std::chrono; diff --git a/server/core/server.cc b/server/core/server.cc index de4980827..011099655 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -108,10 +108,6 @@ private: ThisUnit this_unit; -const char ERR_CANNOT_MODIFY[] = "The server is monitored, so only the maintenance status can be " - "set/cleared manually. Status was not modified."; -const char WRN_REQUEST_OVERWRITTEN[] = "Previous maintenance request was not yet read by the monitor " - "and was overwritten."; const char ERR_TOO_LONG_CONFIG_VALUE[] = "The new value for %s is too long. Maximum length is %i characters."; // Converts Server::ConfigParam to MXS_CONFIG_PARAM and keeps them in the same order. Required for some @@ -968,38 +964,9 @@ bool mxs::server_set_status(SERVER* srv, int bit, string* errmsg_out) * but the race condition cannot cause significant harm. Monitors are never * freed so the pointer stays valid. */ Monitor* mon = monitor_server_in_use(srv); - if (mon && mon->m_state == MONITOR_STATE_RUNNING) + if (mon) { - /* This server is monitored, in which case modifying any other status bit than Maintenance is - * disallowed. */ - if (bit & ~(SERVER_MAINT | SERVER_BEING_DRAINED)) - { - MXS_ERROR(ERR_CANNOT_MODIFY); - if (errmsg_out) - { - *errmsg_out = ERR_CANNOT_MODIFY; - } - } - else if (bit & SERVER_MAINT) - { - /* Maintenance is set/cleared using a special variable which the monitor reads when - * starting the next update cycle. */ - int previous_request = atomic_exchange_int(&srv->maint_request, SERVER::MAINTENANCE_ON); - written = true; - // Warn if the previous request hasn't been read. - if (previous_request != SERVER::MAINTENANCE_NO_CHANGE) - { - MXS_WARNING(WRN_REQUEST_OVERWRITTEN); - } - // Also set a flag so the next loop happens sooner. - atomic_store_int(&mon->m_check_maintenance_flag, SERVER::MAINTENANCE_FLAG_CHECK); - } - else - { - // TODO: This should be set the same way the maintenance bit is set. - srv->set_status(bit); - written = true; - } + written = mon->set_server_status(srv, bit, errmsg_out); } else { @@ -1016,33 +983,9 @@ bool mxs::server_clear_status(SERVER* srv, int bit, string* errmsg_out) // See server_set_status(). bool written = false; Monitor* mon = monitor_server_in_use(srv); - if (mon && mon->m_state == MONITOR_STATE_RUNNING) + if (mon) { - if (bit & ~(SERVER_MAINT | SERVER_BEING_DRAINED)) - { - MXS_ERROR(ERR_CANNOT_MODIFY); - if (errmsg_out) - { - *errmsg_out = ERR_CANNOT_MODIFY; - } - } - else if (bit & SERVER_MAINT) - { - // Warn if the previous request hasn't been read. - int previous_request = atomic_exchange_int(&srv->maint_request, SERVER::MAINTENANCE_OFF); - written = true; - if (previous_request != SERVER::MAINTENANCE_NO_CHANGE) - { - MXS_WARNING(WRN_REQUEST_OVERWRITTEN); - } - atomic_store_int(&mon->m_check_maintenance_flag, SERVER::MAINTENANCE_FLAG_CHECK); - } - else - { - // TODO: This should be set the same way the maintenance bit is set. - srv->clear_status(bit); - written = true; - } + written = mon->clear_server_status(srv, bit, errmsg_out); } else {