From be12cab16df4bf8ca4a56a27f8e200d473d131ce Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Wed, 21 Nov 2018 09:21:54 +0200 Subject: [PATCH] MXS-2178: Provide workaround for watchdog notifications The systemd watchdog mechanism requries notifications at regular intervals. If a synchronous operation of some kind is performed by a worker, then those notfications will not be generated. This change provides each worker with a secondary thread that can be used for triggering those notifications when the worker itself is busy doing other stuff. The effect is that there will be an additional thread for each worker, but most of the time that thread will be idle. Sofar only the mechanism; in subsequent changes the mechanism will be taken into use. --- include/maxscale/routingworker.hh | 89 ++++++++++++++++++++++++-- server/core/routingworker.cc | 102 ++++++++++++++++++++++++++++++ 2 files changed, 187 insertions(+), 4 deletions(-) diff --git a/include/maxscale/routingworker.hh b/include/maxscale/routingworker.hh index 19dfe2438..c659c2f4a 100644 --- a/include/maxscale/routingworker.hh +++ b/include/maxscale/routingworker.hh @@ -432,7 +432,38 @@ public: * To be called from the initial (parent) thread if the systemd watchdog is on. */ static void set_watchdog_interval(uint64_t microseconds); + + /** + * The internal watchdog interval. + * + * @return The interval in seconds, 0 if not enabled. + */ + static uint32_t get_internal_watchdog_interval() + { + return s_watchdog_interval.secs(); + } + + // TODO: Public now ONLY so that it can be used in the workaround for the + // TODO "long max[admin|ctrl] operations may trigger the watchdog" problem. + void check_systemd_watchdog(); + + /** + * Turn on the watchdog workaround for this worker. + * + * @see mxs::WatchdogWorkaround + */ + void start_watchdog_workaround(); + + /** + * Turn off the watchdog workaround for this worker. + * + * @see mxs::WatchdogWorkaround + */ + void stop_watchdog_workaround(); + private: + class WatchdogNotifier; + const int m_id; /*< The id of the worker. */ SessionsById m_sessions; /*< A mapping of session_id->MXS_SESSION. The map * should contain sessions exclusive to this @@ -453,14 +484,64 @@ private: void epoll_tick(); // override void delete_zombies(); - void check_systemd_watchdog(); + //void check_systemd_watchdog(); static uint32_t epoll_instance_handler(MXB_POLL_DATA* data, MXB_WORKER* worker, uint32_t events); uint32_t handle_epoll_events(uint32_t events); - static maxbase::Duration s_watchdog_interval; /*< Duration between notifications, if any. */ - static maxbase::TimePoint s_watchdog_next_check;/*< Next time to notify systemd. */ - std::atomic m_alive; /*< Set to true in epoll_tick(), false on notification. */ + static maxbase::Duration s_watchdog_interval; /*< Duration between notifications, if any. */ + static maxbase::TimePoint s_watchdog_next_check; /*< Next time to notify systemd. */ + std::atomic m_alive; /*< Set to true in epoll_tick(), false on notification. */ + WatchdogNotifier* m_pWatchdog_notifier; /*< Watchdog notifier, if systemd enabled. */ +}; + +/** + * @class WatchdogWorkaround + * + * RAII-class using which the systemd watchdog notification can be + * handled during synchronous worker activity that causes the epoll + * event handling to be stalled. + * + * The constructor turns on the workaround and the destructor + * turns it off. + */ +class WatchdogWorkaround +{ + WatchdogWorkaround(const WatchdogWorkaround&); + WatchdogWorkaround& operator=(const WatchdogWorkaround&); + +public: + /** + * Turns on the watchdog workaround for a specific worker. + * + * @param pWorker The worker for which the systemd notification + * should be arranged. Need not be the calling worker. + */ + WatchdogWorkaround(RoutingWorker* pWorker) + : m_pWorker(pWorker) + { + mxb_assert(pWorker); + m_pWorker->start_watchdog_workaround(); + } + + /** + * Turns on the watchdog workaround for the calling worker. + */ + WatchdogWorkaround() + : WatchdogWorkaround(RoutingWorker::get_current()) + { + } + + /** + * Turns off the watchdog workaround. + */ + ~WatchdogWorkaround() + { + m_pWorker->stop_watchdog_workaround(); + } + +private: + RoutingWorker* m_pWorker; }; // Data local to a routing worker diff --git a/server/core/routingworker.cc b/server/core/routingworker.cc index 23aaac277..b909de73c 100644 --- a/server/core/routingworker.cc +++ b/server/core/routingworker.cc @@ -171,16 +171,102 @@ maxbase::Duration RoutingWorker::s_watchdog_interval {0}; // static maxbase::TimePoint RoutingWorker::s_watchdog_next_check; +class RoutingWorker::WatchdogNotifier +{ + WatchdogNotifier(const WatchdogNotifier&) = delete; + WatchdogNotifier& operator=(const WatchdogNotifier&) = delete; + +public: + WatchdogNotifier(mxs::RoutingWorker* pOwner) + : m_owner(*pOwner) + , m_nClients(0) + , m_terminate(false) + { + m_thread = std::thread([this] { + uint32_t interval = mxs::RoutingWorker::get_internal_watchdog_interval(); + timespec timeout = { interval, 0 }; + + while (!mxb::atomic::load(&m_terminate, mxb::atomic::RELAXED)) + { + // We will wakeup when someone wants the notifier to run, + // or when MaxScale is going down. + m_sem_start.wait(); + + if (!mxb::atomic::load(&m_terminate, mxb::atomic::RELAXED)) + { + // If MaxScale is not going down... + do + { + // we check the systemd watchdog... + m_owner.check_systemd_watchdog(); + } while (!m_sem_stop.timedwait(timeout)); + // until the semaphore is actually posted, which it will be + // once the notification should stop. + } + } + }); + } + + ~WatchdogNotifier() + { + mxb_assert(m_nClients == 0); + mxb::atomic::store(&m_terminate, true, mxb::atomic::RELAXED); + m_sem_start.post(); + m_thread.join(); + } + + void start() + { + Guard guard(m_lock); + mxb::atomic::add(&m_nClients, 1, mxb::atomic::RELAXED); + + if (m_nClients == 1) + { + m_sem_start.post(); + } + } + + void stop() + { + Guard guard(m_lock); + mxb::atomic::add(&m_nClients, -1, mxb::atomic::RELAXED); + mxb_assert(m_nClients >= 0); + + if (m_nClients == 0) + { + m_sem_stop.post(); + } + } + +private: + using Guard = std::lock_guard; + + mxs::RoutingWorker& m_owner; + int m_nClients; + bool m_terminate; + std::thread m_thread; + std::mutex m_lock; + mxb::Semaphore m_sem_start; + mxb::Semaphore m_sem_stop; +}; + RoutingWorker::RoutingWorker() : m_id(next_worker_id()) , m_alive(true) + , m_pWatchdog_notifier(nullptr) { MXB_POLL_DATA::handler = &RoutingWorker::epoll_instance_handler; MXB_POLL_DATA::owner = this; + + if (s_watchdog_interval.count() != 0) + { + m_pWatchdog_notifier = new WatchdogNotifier(this); + } } RoutingWorker::~RoutingWorker() { + delete m_pWatchdog_notifier; } // static @@ -994,6 +1080,22 @@ void maxscale::RoutingWorker::set_watchdog_interval(uint64_t microseconds) s_watchdog_next_check = maxbase::Clock::now(); } +void maxscale::RoutingWorker::start_watchdog_workaround() +{ + if (m_pWatchdog_notifier) + { + m_pWatchdog_notifier->start(); + } +} + +void maxscale::RoutingWorker::stop_watchdog_workaround() +{ + if (m_pWatchdog_notifier) + { + m_pWatchdog_notifier->stop(); + } +} + // A note about the below code. While the main worker is turning the "m_alive" values to false, // it is a possibility that another RoutingWorker sees the old value of "s_watchdog_next_check" // but its new "m_alive==false" value, marks itself alive and promptly hangs. This would cause a