From bc4a52acb024793e80ac9cfc6ca929e68bbc43df Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 28 Jan 2019 15:22:02 +0200 Subject: [PATCH] MXS-2273 Set SERVER_BEING_DRAINED properly The maintenance and being-drained modes are now set using the same mechanism. --- include/maxscale/monitor.hh | 8 ++-- server/core/monitor.cc | 80 ++++++++++++++++++++++++------------- 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index 31f076f9e..11806dac6 100644 --- a/include/maxscale/monitor.hh +++ b/include/maxscale/monitor.hh @@ -134,9 +134,11 @@ public: /** * Maintenance mode request constants. */ - static const int SERVER_NO_CHANGE = 0; - static const int SERVER_MAINT_OFF = 1; - static const int SERVER_MAINT_ON = 2; + static const int SERVER_NO_CHANGE = 0; + static const int SERVER_MAINT_OFF = 1; + static const int SERVER_MAINT_ON = 2; + static const int SERVER_BEING_DRAINED_OFF = 3; + static const int SERVER_BEING_DRAINED_ON = 4; SERVER* server = nullptr; /**< The server being monitored */ MYSQL* con = nullptr; /**< The MySQL connection */ diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 60e5fac2c..7ec9c3e3e 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -1588,8 +1588,7 @@ void monitor_check_maintenance_requests(Monitor* monitor) { /* In theory, the admin may be modifying the server maintenance status during this function. The overall * maintenance flag should be read-written atomically to prevent missing a value. */ - int flags_changed = atomic_exchange_int(&monitor->check_status_flag, - Monitor::STATUS_FLAG_NOCHECK); + int flags_changed = atomic_exchange_int(&monitor->check_status_flag, Monitor::STATUS_FLAG_NOCHECK); if (flags_changed != Monitor::STATUS_FLAG_NOCHECK) { for (auto ptr : monitor->m_servers) @@ -1597,14 +1596,31 @@ void monitor_check_maintenance_requests(Monitor* monitor) // The only server status bit the admin may change is the [Maintenance] bit. int admin_msg = atomic_exchange_int(&ptr->status_request, MXS_MONITORED_SERVER::SERVER_NO_CHANGE); - if (admin_msg == MXS_MONITORED_SERVER::SERVER_MAINT_ON) + + switch (admin_msg) { + case MXS_MONITORED_SERVER::SERVER_MAINT_ON: // TODO: Change to writing MONITORED_SERVER->pending status instead once cleanup done. ptr->server->set_status(SERVER_MAINT); - } - else if (admin_msg == MXS_MONITORED_SERVER::SERVER_MAINT_OFF) - { + break; + + case MXS_MONITORED_SERVER::SERVER_MAINT_OFF: ptr->server->clear_status(SERVER_MAINT); + break; + + case MXS_MONITORED_SERVER::SERVER_BEING_DRAINED_ON: + ptr->server->set_status(SERVER_BEING_DRAINED); + break; + + case MXS_MONITORED_SERVER::SERVER_BEING_DRAINED_OFF: + ptr->server->clear_status(SERVER_BEING_DRAINED); + break; + + case MXS_MONITORED_SERVER::SERVER_NO_CHANGE: + break; + + default: + mxb_assert(!true); } } } @@ -2383,12 +2399,23 @@ bool Monitor::set_server_status(SERVER* srv, int bit, string* errmsg_out) *errmsg_out = ERR_CANNOT_MODIFY; } } - else if (bit & SERVER_MAINT) + else { - /* Maintenance is set/cleared using a special variable which the monitor reads when - * starting the next update cycle. */ - int previous_request = atomic_exchange_int(&msrv->status_request, - MXS_MONITORED_SERVER::SERVER_MAINT_ON); + /* Maintenance and being-drained are set/cleared using a special variable which the + * monitor reads when starting the next update cycle. */ + + int request; + if (bit & SERVER_MAINT) + { + request = MXS_MONITORED_SERVER::SERVER_MAINT_ON; + } + else + { + mxb_assert(bit & SERVER_BEING_DRAINED); + request = MXS_MONITORED_SERVER::SERVER_BEING_DRAINED_ON; + } + + int previous_request = atomic_exchange_int(&msrv->status_request, request); written = true; // Warn if the previous request hasn't been read. if (previous_request != MXS_MONITORED_SERVER::SERVER_NO_CHANGE) @@ -2398,12 +2425,6 @@ bool Monitor::set_server_status(SERVER* srv, int bit, string* errmsg_out) // Also set a flag so the next loop happens sooner. atomic_store_int(&this->check_status_flag, Monitor::STATUS_FLAG_CHECK); } - else - { - // TODO: This should be set the same way the maintenance bit is set. - srv->set_status(bit); - written = true; - } } else { @@ -2439,24 +2460,29 @@ bool Monitor::clear_server_status(SERVER* srv, int bit, string* errmsg_out) *errmsg_out = ERR_CANNOT_MODIFY; } } - else if (bit & SERVER_MAINT) + else { - // Warn if the previous request hasn't been read. - int previous_request = atomic_exchange_int(&msrv->status_request, - MXS_MONITORED_SERVER::SERVER_MAINT_OFF); + int request; + if (bit & SERVER_MAINT) + { + request = MXS_MONITORED_SERVER::SERVER_MAINT_OFF; + } + else + { + mxb_assert(bit & SERVER_BEING_DRAINED); + request = MXS_MONITORED_SERVER::SERVER_BEING_DRAINED_OFF; + } + + int previous_request = atomic_exchange_int(&msrv->status_request, request); written = true; + // Warn if the previous request hasn't been read. if (previous_request != MXS_MONITORED_SERVER::SERVER_NO_CHANGE) { MXS_WARNING(WRN_REQUEST_OVERWRITTEN); } + // Also set a flag so the next loop happens sooner. atomic_store_int(&this->check_status_flag, Monitor::STATUS_FLAG_CHECK); } - else - { - // TODO: This should be set the same way the maintenance bit is set. - srv->clear_status(bit); - written = true; - } } else {