[3668] Addressed review comments.

src/bin/d2/d2_process.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

src/bin/d2/d2_process.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

src/bin/d2/d2_update_mgr.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

src/bin/d2/tests/io_service_signal_unittests.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

src/bin/dhcp4/dhcp4_messages.mes

@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2014  Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 #
 # Permission to use, copy, modify, and/or distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
@@ -187,7 +187,7 @@ specified client after receiving a REQUEST message from it.  There are many
 possible reasons for such a failure. Additional messages will indicate the
 reason.
 
-% DHCP4_LEASE_DATABASE_TIMERS_EXEC_FAIL failed to execute the timers for lease database: %1
+% 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

src/bin/dhcp4/dhcp4_srv.cc

@@ -150,26 +150,21 @@ Dhcpv4Srv::sendPacket(const Pkt4Ptr& packet) {
 bool
 Dhcpv4Srv::run() {
     while (!shutdown_) {
-        /// @todo Currently we're using the fixed value of the timeout for
-        /// select. This value shouldn't be changed. Keeping it at 1s
-        /// guarantees that the main loop will be executed at least once
-        /// a seconds allowing for executing the interval timers associated
-        /// with the lease database backend in use. The intervals for these
-        /// timers are configured using the unit of 1 second. Bumping up
-        /// the select timeout would cause the timers to go out of sync
-        /// with the configured value.
-        /// Probing for the packets at this pace should not cause a
-        /// significant rise of the CPU usage. However, in the future we
-        /// should adjust the select timeout to the value reported by the
-        /// lease database backend as a maximum poll interval.
-        //cppcheck-suppress variableScope This is temporary anyway
-        const int timeout = 1;
-
         // client's message and server's response
         Pkt4Ptr query;
         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;
+            }
             query = receivePacket(timeout);
 
         } catch (const SignalInterruptOnSelect) {
@@ -197,7 +192,7 @@ Dhcpv4Srv::run() {
 
         // Execute ready timers for the lease database, e.g. Lease File Cleanup.
         try {
-            LeaseMgrFactory::instance().getIOService()->get_io_service().poll();
+            LeaseMgrFactory::instance().getIOService()->poll();
 
         } catch (const std::exception& ex) {
             LOG_WARN(dhcp4_logger, DHCP4_LEASE_DATABASE_TIMERS_EXEC_FAIL)

src/bin/dhcp6/dhcp6_messages.mes

@@ -275,7 +275,7 @@ failed to grant a non-temporary address lease for the client. There may
 be many reasons for such failure. Each failure is logged in a separate
 log entry.
 
-% DHCP6_LEASE_DATABASE_TIMERS_EXEC_FAIL failed to execute the timers for lease database: %1
+% 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

src/bin/dhcp6/dhcp6_srv.cc

@@ -230,26 +230,21 @@ Dhcpv6Srv::testUnicast(const Pkt6Ptr& pkt) const {
 
 bool Dhcpv6Srv::run() {
     while (!shutdown_) {
-        /// @todo Currently we're using the fixed value of the timeout for
-        /// select. This value shouldn't be changed. Keeping it at 1s
-        /// guarantees that the main loop will be executed at least once
-        /// a seconds allowing for executing the interval timers associated
-        /// with the lease database backend in use. The intervals for these
-        /// timers are configured using the unit of 1 second. Bumping up
-        /// the select timeout would cause the timers to go out of sync
-        /// with the configured value.
-        /// Probing for the packets at this pace should not cause a
-        /// significant rise of the CPU usage. However, in the future we
-        /// should adjust the select timeout to the value reported by the
-        /// lease database backend as a maximum poll interval.
-        //cppcheck-suppress variableScope This is temporary anyway
-        const int timeout = 1;
-
         // client's message and server's response
         Pkt6Ptr query;
         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;
+            }
             query = receivePacket(timeout);
 
         } catch (const SignalInterruptOnSelect) {
@@ -276,7 +271,7 @@ bool Dhcpv6Srv::run() {
 
         // Execute ready timers for the lease database, e.g. Lease File Cleanup.
         try {
-            LeaseMgrFactory::instance().getIOService()->get_io_service().poll();
+            LeaseMgrFactory::instance().getIOService()->poll();
 
         } catch (const std::exception& ex) {
             LOG_WARN(dhcp6_logger, DHCP6_LEASE_DATABASE_TIMERS_EXEC_FAIL)

src/lib/asiolink/interval_timer.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011, 2104 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

src/lib/asiolink/io_service.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011,2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011, 2013, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -59,14 +59,26 @@ public:
     ///
     /// This method does not return control to the caller until
     /// the \c stop() method is called via some handler.
-    void run() { io_service_.run(); };
+    void run() {
+        io_service_.run();
+    };
 
     /// \brief Run the underlying event loop for a single event.
     ///
     /// This method return control to the caller as soon as the
     /// first handler has completed.  (If no handlers are ready when
     /// it is run, it will block until one is.)
-    void run_one() { io_service_.run_one();} ;
+    void run_one() {
+        io_service_.run_one();
+    };
+
+    /// \brief Run the underlying event loop for a ready events.
+    ///
+    /// This method executes handlers for all ready events and returns.
+    /// It will return immediately if there are no ready events.
+    void poll() {
+        io_service_.poll();
+    };
 
     /// \brief Stop the underlying event loop.
     ///
@@ -108,6 +120,11 @@ IOService::run_one() {
 }
 
 void
+IOService::poll() {
+    io_impl_->poll();
+}
+
+void
 IOService::stop() {
     io_impl_->stop();
 }

src/lib/asiolink/io_service.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011,2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011, 2013, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -59,6 +59,12 @@ public:
     /// it is run, it will block until one is.)
     void run_one();
 
+    /// \brief Run the underlying event loop for a ready events.
+    ///
+    /// This method executes handlers for all ready events and returns.
+    /// It will return immediately if there are no ready events.
+    void poll();
+
     /// \brief Stop the underlying event loop.
     ///
     /// This will return the control to the caller of the \c run() method.

src/lib/asiolink/tests/io_service_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013, 2015  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -34,6 +34,7 @@ TEST(IOService, post) {
     // Post two events
     service.post(boost::bind(&postedEvent, &called, 1));
     service.post(boost::bind(&postedEvent, &called, 2));
+    service.post(boost::bind(&postedEvent, &called, 3));
     // They have not yet been called
     EXPECT_TRUE(called.empty());
     // Process two events
@@ -43,6 +44,11 @@ TEST(IOService, post) {
     ASSERT_EQ(2, called.size());
     EXPECT_EQ(1, called[0]);
     EXPECT_EQ(2, called[1]);
+
+    // Test that poll() executes the last handler.
+    service.poll();
+    ASSERT_EQ(3, called.size());
+    EXPECT_EQ(3, called[2]);
 }
 
 }

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -303,13 +303,13 @@ A debug message issued when DHCPv6 lease is being loaded from the file to
 memory.
 
 % DHCPSRV_MEMFILE_LFC_SETUP setting up the Lease File Cleanup interval to %1 sec
-An info message logged when the Memfile lease database backend configures
-the LFC to be executed periodically. An argument holds the interval in seconds
-in which the LFC will be executed.
+An informational message logged when the Memfile lease database backend
+configures the LFC to be executed periodically. The argument holds the
+interval in seconds in which the LFC will be executed.
 
 % DHCPSRV_MEMFILE_LFC_START starting Lease File Cleanup
-An info message issued when the Memfile lease database backend starts the
-periodic Lease File Cleanup.
+An informational message issued when the Memfile lease database backend
+starts the periodic Lease File Cleanup.
 
 % 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

src/lib/dhcpsrv/lease_mgr.h

@@ -391,6 +391,25 @@ 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 {

src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -49,7 +49,7 @@ Memfile_LeaseMgr::Memfile_LeaseMgr(const ParameterMap& parameters)
         LOG_WARN(dhcpsrv_logger, DHCPSRV_MEMFILE_NO_STORAGE);
 
     } else  {
-        initTimers(universe == "4" ? V4 : V6);
+        initTimers();
     }
 }
 
@@ -412,6 +412,11 @@ Memfile_LeaseMgr::rollback() {
               DHCPSRV_MEMFILE_ROLLBACK);
 }
 
+uint32_t
+Memfile_LeaseMgr::getIOServiceExecInterval() const {
+    return (static_cast<uint32_t>(lfc_timer_.getInterval() / 1000));
+}
+
 std::string
 Memfile_LeaseMgr::getDefaultLeaseFilePath(Universe u) const {
     std::ostringstream s;
@@ -472,7 +477,7 @@ Memfile_LeaseMgr::initLeaseFilePath(Universe u) {
 }
 
 void
-Memfile_LeaseMgr::initTimers(const Universe& universe) {
+Memfile_LeaseMgr::initTimers() {
     std::string lfc_interval_str = "0";
     try {
         lfc_interval_str = getParameter("lfc-interval");

src/lib/dhcpsrv/memfile_lease_mgr.h

@@ -323,6 +323,19 @@ public:
     /// support transactions, this is a no-op.
     virtual void rollback();
 
+    /// @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).
@@ -430,9 +443,7 @@ private:
     ///
     /// Currently only one timer is supported. This timer executes the
     /// Lease File Cleanup periodically.
-    ///
-    /// @param universe A V4 or V6 value.
-    void initTimers(const Universe& universe);
+    void initTimers();
 
     // This is a multi-index container, which holds elements that can
     // be accessed using different search indexes.

src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc

@@ -288,6 +288,9 @@ 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();
 
     // Run the test for at most 2.9 seconds.
@@ -314,6 +317,8 @@ TEST_F(MemfileLeaseMgrTest, lfcTimerDisabled) {
     boost::scoped_ptr<LFCMemfileLeaseMgr>
         lease_mgr(new LFCMemfileLeaseMgr(pmap));
 
+    EXPECT_EQ(0, lease_mgr->getIOServiceExecInterval());
+
     io_service_ = lease_mgr->getIOService();
 
     // Run the test for at most 1.9 seconds.
@@ -326,6 +331,31 @@ TEST_F(MemfileLeaseMgrTest, lfcTimerDisabled) {
     EXPECT_EQ(0, lease_mgr->getLFCCount());
 }
 
+// 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(new LFCMemfileLeaseMgr(pmap));
+    EXPECT_EQ(10, lease_mgr->getIOServiceExecInterval());
+
+    // lfc-interval = 20
+    pmap["lfc-interval"] = 20;
+    lease_mgr.reset(new LFCMemfileLeaseMgr(pmap));
+    EXPECT_EQ(10, lease_mgr->getIOServiceExecInterval());
+
+}
+
 // Checks that adding/getting/deleting a Lease6 object works.
 TEST_F(MemfileLeaseMgrTest, addGetDelete6) {
     startBackend(V6);