diff options
| -rw-r--r-- | AUTHORS | 4 | ||||
| -rw-r--r-- | src/lib/dhcpsrv/multi_threading_utils.cc | 8 | ||||
| -rw-r--r-- | src/lib/dhcpsrv/multi_threading_utils.h | 24 | ||||
| -rw-r--r-- | src/lib/dhcpsrv/tests/Makefile.am | 1 | ||||
| -rw-r--r-- | src/lib/dhcpsrv/tests/multi_threading_utils_unittest.cc | 94 | ||||
| -rw-r--r-- | src/lib/util/multi_threading_mgr.cc | 24 | ||||
| -rw-r--r-- | src/lib/util/multi_threading_mgr.h | 26 | ||||
| -rw-r--r-- | src/lib/util/tests/multi_threading_mgr_unittest.cc | 41 |
8 files changed, 203 insertions, 19 deletions
@@ -13,12 +13,12 @@ Primary developers: control agent, shared networks, high availability, config backend) - Thomas Markwalder (DDNS, user_chk, global host reservations, stat commands, - congestion handling, config backend) + congestion handling, config backend) - Jeremy C. Reed (documentation, build system, testing, release engineering) - Wlodek Wencel (testing, release engineering) - Francis Dupont (crypto, flex/bison parsers, perfdhcp, control agent, radius, netconf, config backend) - - Brian Reid (logo design) + - Brian Reid (logo design) - Shawn Routhier (lease file cleanup) - Michal Nowikowski (testing, hammer, release engineering) - Razvan Becheriu (cassandra, sysrepo) diff --git a/src/lib/dhcpsrv/multi_threading_utils.cc b/src/lib/dhcpsrv/multi_threading_utils.cc index 293c53f1b3..2aff3754bf 100644 --- a/src/lib/dhcpsrv/multi_threading_utils.cc +++ b/src/lib/dhcpsrv/multi_threading_utils.cc @@ -19,8 +19,9 @@ namespace dhcp { void MultiThreadingCriticalSection::stopPktProcessing() { auto& thread_pool = MultiThreadingMgr::instance().getPktThreadPool(); + bool override = MultiThreadingMgr::instance().getOverride(); auto size = MultiThreadingMgr::instance().getPktThreadPoolSize(); - if (size) { + if (size && !override) { thread_pool.stop(); } } @@ -28,8 +29,9 @@ MultiThreadingCriticalSection::stopPktProcessing() { void MultiThreadingCriticalSection::startPktProcessing() { auto& thread_pool = MultiThreadingMgr::instance().getPktThreadPool(); + bool override = MultiThreadingMgr::instance().getOverride(); auto size = MultiThreadingMgr::instance().getPktThreadPoolSize(); - if (size) { + if (size && !override) { thread_pool.start(size); } } @@ -38,9 +40,11 @@ MultiThreadingCriticalSection::MultiThreadingCriticalSection() { if (MultiThreadingMgr::instance().getMode()) { stopPktProcessing(); } + MultiThreadingMgr::instance().incrementOverride(); } MultiThreadingCriticalSection::~MultiThreadingCriticalSection() { + MultiThreadingMgr::instance().decrementOverride(); if (MultiThreadingMgr::instance().getMode()) { startPktProcessing(); } diff --git a/src/lib/dhcpsrv/multi_threading_utils.h b/src/lib/dhcpsrv/multi_threading_utils.h index a37f376bf3..533a7f84ca 100644 --- a/src/lib/dhcpsrv/multi_threading_utils.h +++ b/src/lib/dhcpsrv/multi_threading_utils.h @@ -15,42 +15,40 @@ namespace dhcp { /// @note: everything here MUST be used ONLY from the main thread. /// When called from a thread of the pool it can deadlock. -/// @brief Function stopping and joining all threads of the pool. -/// @throw isc::NotImplemented until is implemented. -void stopPktProcessing(); - -/// @brief Function (re)starting threads of the pool. -/// @throw isc::NotImplemented until is implemented. -void startPktProcessing(); - /// @brief RAII class creating a critical section. /// /// @note: the multi-threading mode MUST NOT be changed in the RAII /// @c MultiThreadingCriticalSection body. /// @note: starting and stopping the packet thread pool should be handled /// in the main thread, if done on one of the processing threads will cause a -/// dead-lock +/// deadlock +/// This is mainly useful in hook commands which handle configuration +/// changes. class MultiThreadingCriticalSection : public boost::noncopyable { public: /// @brief Constructor. /// - /// Entering the critical section. + /// Entering the critical section. The packet thread pool instance will be + /// stopped so that configuration changes can be safely applied. MultiThreadingCriticalSection(); /// @brief Destructor. /// - /// Leaving the critical section. + /// Leaving the critical section. The packet thread pool instance will be + /// started according to new configuration. virtual ~MultiThreadingCriticalSection(); +private: + /// @brief Class method stopping and joining all threads of the pool. /// - /// @throw isc::NotImplemented until is implemented. + /// Stop the packet thread pool if running. static void stopPktProcessing(); /// @brief Class method (re)starting threads of the pool. /// - /// @throw isc::NotImplemented until is implemented. + /// Start the packet thread pool according to current configuration. static void startPktProcessing(); }; diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index d9cac94776..9599e9de18 100644 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -102,6 +102,7 @@ libdhcpsrv_unittests_SOURCES += lease_mgr_factory_unittest.cc libdhcpsrv_unittests_SOURCES += lease_mgr_unittest.cc libdhcpsrv_unittests_SOURCES += generic_lease_mgr_unittest.cc generic_lease_mgr_unittest.h libdhcpsrv_unittests_SOURCES += memfile_lease_mgr_unittest.cc +libdhcpsrv_unittests_SOURCES += multi_threading_utils_unittest.cc libdhcpsrv_unittests_SOURCES += dhcp_parsers_unittest.cc libdhcpsrv_unittests_SOURCES += ncr_generator_unittest.cc if HAVE_MYSQL diff --git a/src/lib/dhcpsrv/tests/multi_threading_utils_unittest.cc b/src/lib/dhcpsrv/tests/multi_threading_utils_unittest.cc new file mode 100644 index 0000000000..f50e4b7b8b --- /dev/null +++ b/src/lib/dhcpsrv/tests/multi_threading_utils_unittest.cc @@ -0,0 +1,94 @@ +// Copyright (C) 2020 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include <config.h> + +#include <dhcpsrv/multi_threading_utils.h> +#include <util/multi_threading_mgr.h> + +#include <gtest/gtest.h> + +using namespace isc::dhcp; +using namespace isc::util; + +namespace { + +TEST(MultiThreadingUtil, constructorAndDestructor) { + // get the thread pool instance + auto & thread_pool = MultiThreadingMgr::instance().getPktThreadPool(); + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + // apply multi-threading configuration with 16 threads + MultiThreadingMgr::instance().apply(true, 16); + // thread count should match + EXPECT_EQ(thread_pool.size(), 16); + // use scope to test constructor and destructor + { + MultiThreadingCriticalSection cs; + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + // use scope to test constructor and destructor + { + MultiThreadingCriticalSection inner_cs; + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + } + } + // thread count should match + EXPECT_EQ(thread_pool.size(), 16); + // use scope to test constructor and destructor + { + MultiThreadingCriticalSection cs; + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + // apply multi-threading configuration with 64 threads + MultiThreadingMgr::instance().apply(true, 64); + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + } + // thread count should match + EXPECT_EQ(thread_pool.size(), 64); + // use scope to test constructor and destructor + { + MultiThreadingCriticalSection cs; + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + // apply multi-threading configuration with 0 threads + MultiThreadingMgr::instance().apply(false, 64); + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + } + // thread count should match + EXPECT_EQ(thread_pool.size(), 0); + // use scope to test constructor and destructor + { + MultiThreadingCriticalSection cs; + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + // use scope to test constructor and destructor + { + MultiThreadingCriticalSection inner_cs; + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + } + } + // thread count should match + EXPECT_EQ(thread_pool.size(), 0); + // use scope to test constructor and destructor + { + MultiThreadingCriticalSection cs; + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + // apply multi-threading configuration with 64 threads + MultiThreadingMgr::instance().apply(true, 64); + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + } + // thread count should match + EXPECT_EQ(thread_pool.size(), 64); +} + +} // namespace diff --git a/src/lib/util/multi_threading_mgr.cc b/src/lib/util/multi_threading_mgr.cc index 5fbf339387..aa5c79a425 100644 --- a/src/lib/util/multi_threading_mgr.cc +++ b/src/lib/util/multi_threading_mgr.cc @@ -9,7 +9,7 @@ namespace isc { namespace util { -MultiThreadingMgr::MultiThreadingMgr() : enabled_(false) { +MultiThreadingMgr::MultiThreadingMgr() : enabled_(false), override_(0) { } MultiThreadingMgr::~MultiThreadingMgr() { @@ -31,6 +31,24 @@ MultiThreadingMgr::setMode(bool enabled) { enabled_ = enabled; } +void +MultiThreadingMgr::incrementOverride() { + override_++; +} + +void +MultiThreadingMgr::decrementOverride() { + if (override_ == 0) { + isc_throw(InvalidOperation, "invalid negative value for override"); + } + override_--; +} + +bool +MultiThreadingMgr::getOverride() { + return (override_ != 0); +} + ThreadPool<std::function<void()>>& MultiThreadingMgr::getPktThreadPool() { return pkt_thread_pool_; @@ -71,7 +89,9 @@ MultiThreadingMgr::apply(bool enabled, uint32_t thread_count) { } setPktThreadPoolSize(thread_count); setMode(true); - pkt_thread_pool_.start(thread_count); + if (!getOverride()) { + pkt_thread_pool_.start(thread_count); + } } else { pkt_thread_pool_.reset(); setMode(false); diff --git a/src/lib/util/multi_threading_mgr.h b/src/lib/util/multi_threading_mgr.h index 74d21da762..856e7b231e 100644 --- a/src/lib/util/multi_threading_mgr.h +++ b/src/lib/util/multi_threading_mgr.h @@ -66,6 +66,24 @@ public: /// @param enabled The new mode. void setMode(bool enabled); + /// @brief Increment override + /// + /// When entering @ref MultiThreadingCriticalSection, increment override + /// so that any configuration change that might start the packet thread pool + /// is delayed until exiting the respective section + void incrementOverride(); + + /// @brief Decrement override + /// + /// When exiting @ref MultiThreadingCriticalSection, decrement override + /// so that the packet thread pool can be started according to configuration + void decrementOverride(); + + /// @brief Get override + /// + /// Get the override flag + bool getOverride(); + /// @brief Get the packet thread pool. /// /// @return The packet thread pool of this binary instance. @@ -109,6 +127,14 @@ private: /// @brief The current mode. bool enabled_; + /// @brief The override mode. + /// + /// In case the configuration is applied within a + /// @ref MultiThreadingCriticalSection, the thread pool should not be + /// started until the section is over. + /// This also handles multiple interleaved sections. + int override_; + /// @brief The configured size of the packet thread pool. uint32_t pkt_thread_pool_size_; diff --git a/src/lib/util/tests/multi_threading_mgr_unittest.cc b/src/lib/util/tests/multi_threading_mgr_unittest.cc index 9e036d38d8..71802a3ae4 100644 --- a/src/lib/util/tests/multi_threading_mgr_unittest.cc +++ b/src/lib/util/tests/multi_threading_mgr_unittest.cc @@ -6,11 +6,13 @@ #include <config.h> +#include <exceptions/exceptions.h> #include <util/multi_threading_mgr.h> #include <gtest/gtest.h> using namespace isc::util; +using namespace isc; /// @brief Verifies that the default mode is false (MT disabled). TEST(MultiThreadingMgrTest, default) { @@ -100,3 +102,42 @@ TEST(MultiThreadingMgrTest, applyConfig) { EXPECT_EQ(thread_pool.size(), 0); } +/// @brief Verifies that override flag works. +TEST(MultiThreadingMgrTest, override) { + // get the thread pool + auto& thread_pool = MultiThreadingMgr::instance().getPktThreadPool(); + // MT should be disabled + EXPECT_FALSE(MultiThreadingMgr::instance().getMode()); + // override should be disabled + EXPECT_FALSE(MultiThreadingMgr::instance().getOverride()); + // thread count should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 0); + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + // increment override + EXPECT_NO_THROW(MultiThreadingMgr::instance().incrementOverride()); + // override should be enabled + EXPECT_TRUE(MultiThreadingMgr::instance().getOverride()); + // enable MT with 16 threads + EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(true, 16)); + // MT should be enabled + EXPECT_TRUE(MultiThreadingMgr::instance().getMode()); + // thread count should be 16 + EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 16); + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); + // decrement override + EXPECT_NO_THROW(MultiThreadingMgr::instance().decrementOverride()); + // override should be disabled + EXPECT_FALSE(MultiThreadingMgr::instance().getOverride()); + // decrement override + EXPECT_THROW(MultiThreadingMgr::instance().decrementOverride(), InvalidOperation); + // disable MT + EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(false, 0)); + // MT should be disabled + EXPECT_FALSE(MultiThreadingMgr::instance().getMode()); + // thread count should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 0); + // thread pool should be stopped + EXPECT_EQ(thread_pool.size(), 0); +} |