Commit dc430336 authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[3971] Using TimerMgr in the Memfile backend.

parent ab966d88
......@@ -19,6 +19,7 @@
#include <hooks/hooks_manager.h>
#include <dhcp4/json_config_parser.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/timer_mgr.h>
#include <config/command_mgr.h>
#include <stats/stats_mgr.h>
......@@ -123,7 +124,9 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
return (isc::config::createAnswer(1, err.str()));
}
TimerMgr::instance()->stopThread();
ConstElementPtr answer = configureDhcp4Server(*srv, config);
TimerMgr::instance()->startThread();
// Check that configuration was successful. If not, do not reopen sockets
// and don't bother with DDNS stuff.
......@@ -157,6 +160,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
CfgMgr::instance().getStagingCfg()->getCfgIface()->
openSockets(AF_INET, srv->getPort(), getInstance()->useBroadcast());
return (answer);
}
......
......@@ -312,13 +312,6 @@ be sent to the client in the DHCPACK message. The first argument
contains the client and the transaction identification information. The
second argument contains the allocated IPv4 address.
% DHCP4_LEASE_DATABASE_TIMERS_EXEC_FAIL failed to execute timer-based functions for lease database: %1
A warning message executed when a server process is unable to execute
the periodic actions for the lease database. An example of the periodic
action is a Lease File Cleanup. One of the reasons for the failure is
a misconfiguration of the lease database, whereby the lease database
hasn't been selected.
% DHCP4_NAME_GEN_UPDATE_FAIL %1: failed to update the lease after generating name %2 for a client: %3
This message indicates the failure when trying to update the lease and/or
options in the server's response with the hostname generated by the server
......
......@@ -365,16 +365,7 @@ Dhcpv4Srv::run() {
Pkt4Ptr rsp;
try {
// The lease database backend may install some timers for which
// the handlers need to be executed periodically. Retrieve the
// maximum interval at which the handlers must be executed from
// the lease manager.
uint32_t timeout = LeaseMgrFactory::instance().getIOServiceExecInterval();
// If the returned value is zero it means that there are no
// timers installed, so use a default value.
if (timeout == 0) {
timeout = 1000;
}
uint32_t timeout = 1000;
LOG_DEBUG(packet4_logger, DBG_DHCP4_DETAIL, DHCP4_BUFFER_WAIT).arg(timeout);
query = receivePacket(timeout);
......@@ -429,15 +420,6 @@ Dhcpv4Srv::run() {
.arg(e.what());
}
// Execute ready timers for the lease database, e.g. Lease File Cleanup.
try {
LeaseMgrFactory::instance().getIOService()->poll();
} catch (const std::exception& ex) {
LOG_WARN(dhcp4_logger, DHCP4_LEASE_DATABASE_TIMERS_EXEC_FAIL)
.arg(ex.what());
}
// Timeout may be reached or signal received, which breaks select()
// with no reception occurred. No need to log anything here because
// we have logged right after the call to receivePacket().
......
......@@ -16,6 +16,8 @@
CFG_FILE=@abs_top_builddir@/src/bin/dhcp4/tests/test_config.json
# Path to the Kea log file.
LOG_FILE=@abs_top_builddir@/src/bin/dhcp4/tests/test.log
# Path to the Kea lease file.
LEASE_FILE=@abs_top_builddir@/src/bin/dhcp4/tests/test_leases.csv
# Expected version
EXPECTED_VERSION="@PACKAGE_VERSION@"
# Kea configuration to be stored in the configuration file.
......@@ -31,7 +33,9 @@ CONFIG="{
\"lease-database\":
{
\"type\": \"memfile\",
\"persist\": false
\"name\": \"$LEASE_FILE\",
\"persist\": false,
\"lfc-interval\": 1
},
\"subnet4\": [
{
......@@ -276,9 +280,49 @@ shutdown_test() {
test_finish 0
}
# This test verifies that DHCPv4 can be reconfigured with a SIGHUP signal.
lfc_timer_test() {
# Log the start of the test and print test name.
test_start "dhcpv4_srv.lfc_timer_test"
# Remove dangling Kea instances and remove log files.
cleanup
# Create new configuration file.
create_config "${CONFIG}"
# Instruct Kea to log to the specific file.
set_logger
# Start Kea.
start_kea ${bin_path}/${bin}
# Wait up to 20s for Kea to start.
wait_for_kea 20
if [ ${_WAIT_FOR_KEA} -eq 0 ]; then
printf "ERROR: timeout waiting for Kea to start.\n"
clean_exit 1
fi
# Check if it is still running. It could have terminated (e.g. as a result
# of configuration failure).
get_pids ${bin}
if [ ${_GET_PIDS_NUM} -ne 1 ]; then
printf "ERROR: expected one Kea process to be started. Found %d processes\
started.\n" ${_GET_PIDS_NUM}
clean_exit 1
fi
wait_for_message 10 "DHCPSRV_MEMFILE_EXECUTE" 1
if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then
printf "ERROR: Server did not execute LFC.\n"
clean_exit 1
fi
# All ok. Shut down Kea and exit.
test_finish 0
}
server_pid_file_test "${CONFIG}" DHCP4_ALREADY_RUNNING
dynamic_reconfiguration_test
shutdown_test "dhcpv4.sigterm_test" 15
shutdown_test "dhcpv4.sigint_test" 2
version_test "dhcpv4.version"
logger_vars_test "dhcpv4.variables"
lfc_timer_test
......@@ -376,13 +376,6 @@ The first argument holds the client and the transaction identification
information. The second argument holds the IAID. The third argument
holds the detailed lease information.
% DHCP6_LEASE_DATABASE_TIMERS_EXEC_FAIL failed to execute timer-based functions for lease database: %1
A warning message executed when a server process is unable to execute
the periodic actions for the lease database. An example of the periodic
action is a Lease File Cleanup. One of the reasons for the failure is
a misconfiguration of the lease database, whereby the lease database
hasn't been selected.
% DHCP6_LEASE_NA_WITHOUT_DUID %1: address lease for address %2 does not have a DUID
This error message indicates a database consistency problem. The lease
database has an entry indicating that the given address is in use,
......
......@@ -317,16 +317,7 @@ bool Dhcpv6Srv::run() {
Pkt6Ptr rsp;
try {
// The lease database backend may install some timers for which
// the handlers need to be executed periodically. Retrieve the
// maximum interval at which the handlers must be executed from
// the lease manager.
uint32_t timeout = LeaseMgrFactory::instance().getIOServiceExecInterval();
// If the returned value is zero it means that there are no
// timers installed, so use a default value.
if (timeout == 0) {
timeout = 1000;
}
uint32_t timeout = 1000;
LOG_DEBUG(packet6_logger, DBG_DHCP6_DETAIL, DHCP6_BUFFER_WAIT).arg(timeout);
query = receivePacket(timeout);
......@@ -385,15 +376,6 @@ bool Dhcpv6Srv::run() {
.arg(e.what());
}
// Execute ready timers for the lease database, e.g. Lease File Cleanup.
try {
LeaseMgrFactory::instance().getIOService()->poll();
} catch (const std::exception& ex) {
LOG_WARN(dhcp6_logger, DHCP6_LEASE_DATABASE_TIMERS_EXEC_FAIL)
.arg(ex.what());
}
// Timeout may be reached or signal received, which breaks select()
// with no packet received
if (!query) {
......
......@@ -346,6 +346,12 @@ failure persist however, the size of the lease file will increase without bound.
An informational message issued when the Memfile lease database backend
starts the periodic Lease File Cleanup.
% DHCPSRV_MEMFILE_LFC_UNREGISTER_TIMER_FAILED failed to unregister timer 'memfile-lfc': %1
This error message is logged when Memfile backend fails to unregister
timer used for lease file cleanup scheduling. This is highly unlikely
and indicates programming error. The message include the reason for this
error.
% DHCPSRV_MEMFILE_NO_STORAGE running in non-persistent mode, leases will be lost after restart
A warning message issued when writes of leases to disk have been disabled
in the configuration. This mode is useful for some kinds of performance
......
......@@ -118,16 +118,6 @@ public:
/// be used directly, but rather specialized derived class should be used
/// instead.
///
/// This class creates an instance of the @c asiolink::IOService in the
/// constructor. This object is required to execute the
/// @c asiolink::IntervalTimer for the operations triggered periodically
/// by the lease database backends which implement @c LeaseMgr interface.
/// In order to execute the timers installed by the particular backend,
/// the owner of the backend (e.g. DHCP server) should retrieve the pointer
/// to the @c asiolink::IOService object by calling @c LeaseMgr::getIOService
/// and call the appropriate functions, e.g. @c poll_one or @c run_one in a
/// main loop.
///
/// This class throws no exceptions. However, methods in concrete
/// implementations of this class may throw exceptions: see the documentation
/// of those classes for details.
......@@ -145,7 +135,7 @@ public:
/// @param parameters A data structure relating keywords and values
/// concerned with the database.
LeaseMgr(const ParameterMap& parameters)
: parameters_(parameters), io_service_(new asiolink::IOService())
: parameters_(parameters)
{}
/// @brief Destructor
......@@ -439,31 +429,6 @@ public:
/// @brief returns value of the parameter
virtual std::string getParameter(const std::string& name) const;
/// @brief Returns the interval at which the @c IOService events should
/// be released.
///
/// The implementations of this class may install the timers which
/// periodically trigger event handlers defined for them. Depending
/// on the intervals specified for these timers the @c IOService::poll,
/// @c IOService::run etc. have to be executed to allow the timers
/// for checking whether they have already expired and the handler
/// must be executed. Running the @c IOService with a lower interval
/// would cause the desynchronization of timers with the clock.
///
/// @return A maximum interval in seconds at which the @c IOService
/// should be executed. A value of 0 means that no timers are installed
/// and that there is no requirement for the @c IOService to be
/// executed at any specific interval.
virtual uint32_t getIOServiceExecInterval() const {
return (0);
}
/// @brief Returns a reference to the @c IOService object used
/// by the Lease Manager.
const asiolink::IOServicePtr& getIOService() const {
return (io_service_);
}
private:
/// @brief list of parameters passed in dbconfig
///
......@@ -471,10 +436,6 @@ private:
/// password and other parameters required for DB access. It is not
/// intended to keep any DHCP-related parameters.
ParameterMap parameters_;
/// @brief Pointer to the IO service object used by the derived classes
/// to trigger interval timers.
asiolink::IOServicePtr io_service_;
};
}; // end of isc::dhcp namespace
......
......@@ -17,6 +17,7 @@
#include <dhcpsrv/dhcpsrv_log.h>
#include <dhcpsrv/lease_file_loader.h>
#include <dhcpsrv/memfile_lease_mgr.h>
#include <dhcpsrv/timer_mgr.h>
#include <exceptions/exceptions.h>
#include <util/pid_file.h>
#include <util/process_spawn.h>
......@@ -73,9 +74,12 @@ public:
/// @c Memfile_LeaseMgr class.
///
/// @param callback A pointer to the callback function.
/// @param io_service An io service used to create the interval timer.
LFCSetup(asiolink::IntervalTimer::Callback callback,
asiolink::IOService& io_service);
LFCSetup(asiolink::IntervalTimer::Callback callback);
/// @brief Destructor.
///
/// Unregisters LFC timer.
~LFCSetup();
/// @brief Sets the new configuration for the %Lease File Cleanup.
///
......@@ -92,11 +96,6 @@ public:
/// @brief Spawns a new process.
void execute();
/// @brief Returns interval at which the cleanup is performed.
///
/// @return Interval in milliseconds.
long getInterval() const;
/// @brief Checks if the lease file cleanup is in progress.
///
/// @return true if the lease file cleanup is being executed.
......@@ -107,9 +106,6 @@ public:
private:
/// @brief Interval timer for LFC.
asiolink::IntervalTimer timer_;
/// @brief A pointer to the @c ProcessSpawn object used to execute
/// the LFC.
boost::scoped_ptr<util::ProcessSpawn> process_;
......@@ -119,11 +115,36 @@ private:
/// @brief A PID of the last executed LFC process.
pid_t pid_;
/// @brief Pointer to the Timer Manager.
TimerMgrPtr timer_mgr_;
};
LFCSetup::LFCSetup(asiolink::IntervalTimer::Callback callback,
asiolink::IOService& io_service)
: timer_(io_service), process_(), callback_(callback), pid_(0) {
LFCSetup::LFCSetup(asiolink::IntervalTimer::Callback callback)
: process_(), callback_(callback), pid_(0),
timer_mgr_(TimerMgr::instance()) {
}
LFCSetup::~LFCSetup() {
try {
// If we're here it means that we are reconfiguring the server or
// the server is terminating. In both cases the thread must
// be already stopped or must stop now.
timer_mgr_->stopThread();
// This may throw exception if the timer hasn't been registered
// but if the LFC Setup instance exists it means that the timer
// must have been registered or such registration have been
// attempted. The registration may fail if the duplicate timer
// exists or if the TimerMgr's worker thread is running but if
// this happens it is a programming error. In any case, we
// don't want exceptions being thrown from the destructor so
// we just log an error here.
timer_mgr_->unregisterTimer("memfile-lfc");
} catch (const std::exception& ex) {
LOG_ERROR(dhcpsrv_logger, DHCPSRV_MEMFILE_LFC_UNREGISTER_TIMER_FAILED)
.arg(ex.what());
}
}
void
......@@ -143,13 +164,6 @@ LFCSetup::setup(const uint32_t lfc_interval,
executable = c_executable;
}
// Set the timer to call callback function periodically.
LOG_INFO(dhcpsrv_logger, DHCPSRV_MEMFILE_LFC_SETUP).arg(lfc_interval);
// Multiple the lfc_interval value by 1000 as this value specifies
// a timeout in seconds, whereas the setup() method expects the
// timeout in milliseconds.
timer_.setup(callback_, lfc_interval * 1000);
// Start preparing the command line for kea-lfc.
// Gather the base file name.
......@@ -187,12 +201,17 @@ LFCSetup::setup(const uint32_t lfc_interval,
// Create the process (do not start it yet).
process_.reset(new util::ProcessSpawn(executable, args));
}
}
long
LFCSetup::getInterval() const {
return (timer_.getInterval());
// Set the timer to call callback function periodically.
LOG_INFO(dhcpsrv_logger, DHCPSRV_MEMFILE_LFC_SETUP).arg(lfc_interval);
// Multiple the lfc_interval value by 1000 as this value specifies
// a timeout in seconds, whereas the setup() method expects the
// timeout in milliseconds.
timer_mgr_->registerTimer("memfile-lfc", callback_, lfc_interval * 1000,
asiolink::IntervalTimer::REPEATING);
timer_mgr_->setup("memfile-lfc");
}
}
void
......@@ -227,9 +246,7 @@ const int Memfile_LeaseMgr::MAJOR_VERSION;
const int Memfile_LeaseMgr::MINOR_VERSION;
Memfile_LeaseMgr::Memfile_LeaseMgr(const ParameterMap& parameters)
: LeaseMgr(parameters),
lfc_setup_(new LFCSetup(boost::bind(&Memfile_LeaseMgr::lfcCallback, this),
*getIOService()))
: LeaseMgr(parameters), lfc_setup_()
{
// Check the universe and use v4 file or v6 file.
std::string universe = getParameter("universe");
......@@ -255,7 +272,7 @@ Memfile_LeaseMgr::Memfile_LeaseMgr(const ParameterMap& parameters)
LOG_WARN(dhcpsrv_logger, DHCPSRV_MEMFILE_NO_STORAGE);
} else {
lfcSetup();
lfcSetup();
}
}
......@@ -785,12 +802,6 @@ Memfile_LeaseMgr::appendSuffix(const std::string& file_name,
return (name);
}
uint32_t
Memfile_LeaseMgr::getIOServiceExecInterval() const {
return (static_cast<uint32_t>(lfc_setup_->getInterval() / 1000));
}
std::string
Memfile_LeaseMgr::getDefaultLeaseFilePath(Universe u) const {
std::ostringstream s;
......@@ -943,6 +954,7 @@ Memfile_LeaseMgr::lfcSetup() {
}
if (lfc_interval > 0) {
lfc_setup_.reset(new LFCSetup(boost::bind(&Memfile_LeaseMgr::lfcCallback, this)));
lfc_setup_->setup(lfc_interval, lease_file4_, lease_file6_);
}
}
......
......@@ -49,12 +49,7 @@ class LFCSetup;
/// The backend installs an @c asiolink::IntervalTimer to periodically execute
/// the @c Memfile_LeaseMgr::lfcCallback. This callback function controls
/// the startup of the background process which removes redundant information
/// from the lease file(s). Note that the @c asiolink::IntervalTimer uses
/// @c asiolink::IOService to execute the callback. The @c LeaseMgr class
/// creates this object, which can be obtained by the caller using the
/// @c LeaseMgr::getIOService. The caller should later call an appropriate
/// method, @c boost::asio::io_service::poll_one to execute the callback when
/// the timer is ready.
/// from the lease file(s).
///
/// When the backend is starting up, it reads leases from the lease file (one
/// by one) and adds them to the in-memory container as follows:
......@@ -458,19 +453,6 @@ public:
/// about the state of the backend.
//@{
/// @brief Returns the interval at which the @c IOService events should
/// be released.
///
/// The Memfile backend may install a timer to execute the %Lease File
/// Cleanup periodically. If this timer is installed, the method returns
/// the LFC interval in milliseconds.
///
/// @return A maximum interval (in seconds) at which the @c IOService
/// should be executed. A value of 0 means that no timers are installed
/// and that there is no requirement for the @c IOService to be
/// executed at any specific interval.
virtual uint32_t getIOServiceExecInterval() const;
/// @brief Returns default path to the lease file.
///
/// @param u Universe (V4 or V6).
......
......@@ -14,12 +14,15 @@
#include <config.h>
#include <boost/asio.hpp>
#include <asiolink/io_address.h>
#include <dhcp/duid.h>
#include <dhcp/iface_mgr.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/lease_mgr.h>
#include <dhcpsrv/lease_mgr_factory.h>
#include <dhcpsrv/memfile_lease_mgr.h>
#include <dhcpsrv/timer_mgr.h>
#include <dhcpsrv/tests/lease_file_io.h>
#include <dhcpsrv/tests/test_utils.h>
#include <dhcpsrv/tests/generic_lease_mgr_unittest.h>
......@@ -106,8 +109,9 @@ public:
MemfileLeaseMgrTest() :
io4_(getLeaseFilePath("leasefile4_0.csv")),
io6_(getLeaseFilePath("leasefile6_0.csv")),
io_service_(),
fail_on_callback_(false) {
io_service_(new IOService()),
fail_on_callback_(false),
timeout_(false) {
std::ostringstream s;
s << KEA_LFC_BUILD_DIR << "/kea-lfc";
......@@ -134,21 +138,10 @@ public:
///
/// destroys lease manager backend.
virtual ~MemfileLeaseMgrTest() {
// Explicitly destroy the timer and the IO service. Note that they
// must be destroyed in this order because test_timer_ depends on
// the io_service_ as it is passed its reference during the
// construction. It usually doesn't matter unless the timer is
// running (hasn't been cancelled). Destroying an underlying
// IO service before cancelling the timer relying on it may lead
// to a crash. This has been proven on CentOS 6, running boost
// version 1.41. Note that destroying the timer also cancels it.
// In theory, we could avoid this explicit release of these objects
// and rely on the order in which they are declared in the class.
// But, this seems to be better as it makes it more visible
// what we are doing here.
test_timer_.reset();
io_service_.reset();
// Stop TimerMgr worker thread if it is running.
TimerMgr::instance()->stopThread();
// Make sure there are no timers registered.
TimerMgr::instance()->unregisterTimers();
LeaseMgrFactory::destroy();
// Remove lease files and products of Lease File Cleanup.
removeFiles(getLeaseFilePath("leasefile4_0.csv"));
......@@ -224,10 +217,24 @@ public:
boost::bind(&MemfileLeaseMgrTest::testTimerCallback, this);
test_timer_.reset(new IntervalTimer(*io_service_));
test_timer_->setup(cb, ms, IntervalTimer::ONE_SHOT);
// The timeout flag will be set by the timeoutCallback if the test
// lasts for too long. In this case we will return from here.
while (!timeout_) {
// Block for one 1 millisecond.
IfaceMgr::instance().receive6(0, 1000);
// Run ready handlers from the local IO service to execute
// the timeout callback if necessary.
io_service_->get_io_service().poll_one();
}
timeout_ = false;
}
/// @brief Test timer callback function.
void testTimerCallback() {
timeout_ = true;
io_service_->stop();
if (fail_on_callback_) {
FAIL() << "Test timeout reached";
......@@ -267,6 +274,9 @@ public:
/// @brief Indicates if the @c testTimerCallback should cause test failure.
bool fail_on_callback_;
/// @brief Boolean flag indicating if the test timeout occurred.
bool timeout_;
};
// This test checks if the LeaseMgr can be instantiated and that it
......@@ -368,16 +378,16 @@ TEST_F(MemfileLeaseMgrTest, lfcTimer) {
boost::scoped_ptr<LFCMemfileLeaseMgr>
lease_mgr(new LFCMemfileLeaseMgr(pmap));
// Check that the interval is correct.
EXPECT_EQ(1, lease_mgr->getIOServiceExecInterval());
io_service_ = lease_mgr->getIOService();
// Start worker thread to execute LFC periodically.
TimerMgr::instance()->startThread();
// Run the test for at most 2.9 seconds.
setTestTime(2900);
// Run the IO service to execute timers.
io_service_->run();
// Stop worker thread to make sure it is not running when lease
// manager is destroyed. The lease manager will be unable to
// unregster timer when the thread is active.
TimerMgr::instance()->stopThread();
// Within 2.9 we should record two LFC executions.
EXPECT_EQ(2, lease_mgr->getLFCCount());
......@@ -397,15 +407,16 @@ TEST_F(MemfileLeaseMgrTest, lfcTimerDisabled) {
boost::scoped_ptr<LFCMemfileLeaseMgr>
lease_mgr(new LFCMemfileLeaseMgr(pmap));
EXPECT_EQ(0, lease_mgr->getIOServiceExecInterval());
io_service_ = lease_mgr->getIOService();
// Start worker thread to execute LFC periodically.
TimerMgr::instance()->startThread();
// Run the test for at most 1.9 seconds.
setTestTime(1900);
// Run the IO service to execute timers.
io_service_->run();
// Stop worker thread to make sure it is not running when lease
// manager is destroyed. The lease manager will be unable to
// unregster timer when the thread is active.
TimerMgr::instance()->stopThread();
// There should be no LFC execution recorded.
EXPECT_EQ(0, lease_mgr->getLFCCount());
......@@ -739,34 +750,6 @@ TEST_F(MemfileLeaseMgrTest, leaseFileCopy) {
ASSERT_FALSE(input_file.exists());
}
// Test that the backend returns a correct value of the interval
// at which the IOService must be executed to run the handlers
// for the installed timers.
TEST_F(MemfileLeaseMgrTest, getIOServiceExecInterval) {
LeaseMgr::ParameterMap pmap;
pmap["type"] = "memfile";
pmap["universe"] = "4";
pmap["name"] = getLeaseFilePath("leasefile4_0.csv");
// The lfc-interval is not set, so the returned value should be 0.
boost::scoped_ptr<LFCMemfileLeaseMgr> lease_mgr(new LFCMemfileLeaseMgr(pmap));
EXPECT_EQ(0, lease_mgr->getIOServiceExecInterval());
// lfc-interval = 10
pmap["lfc-interval"] = "10";
lease_mgr.reset();
lease_mgr.reset(new LFCMemfileLeaseMgr(pmap));
EXPECT_EQ(10, lease_mgr->getIOServiceExecInterval());
// lfc-interval = 20
pmap["lfc-interval"] = "20";
lease_mgr.reset();