complete implementation of statistics-interval.

it involves refactoring of the AuthSrv class: It now internally constructs
IOService; the timer is also constructed internally, using the IOService
object.

src/bin/auth/auth_srv.cc

@@ -23,6 +23,8 @@
 #include <iostream>
 #include <vector>
 
+#include <boost/bind.hpp>
+
 #include <asiolink/asiolink.h>
 
 #include <config/ccsession.h>
@@ -87,6 +89,8 @@ public:
     bool processNotify(const IOMessage& io_message, MessagePtr message,
                        OutputBufferPtr buffer);
 
+    IOService io_service_;
+
     /// Currently non-configurable, but will be.
     static const uint16_t DEFAULT_LOCAL_UDPSIZE = 4096;
 
@@ -102,6 +106,10 @@ public:
     /// Hot spot cache
     isc::datasrc::HotCache cache_;
 
+    /// Interval timer for periodic submission of statistics counters.
+    /// When NULL, the submission is disabled.
+    IntervalTimer statistics_timer_;
+
     /// Query counters for statistics
     AuthCounters counters_;
 private:
@@ -125,6 +133,7 @@ AuthSrvImpl::AuthSrvImpl(const bool use_cache,
     config_session_(NULL), verbose_mode_(false),
     xfrin_session_(NULL),
     memory_datasrc_class_(RRClass::IN()),
+    statistics_timer_(io_service_),
     counters_(verbose_mode_),
     xfrout_connected_(false),
     xfrout_client_(xfrout_client)
@@ -195,7 +204,6 @@ private:
 
 AuthSrv::AuthSrv(const bool use_cache, AbstractXfroutClient& xfrout_client) :
     impl_(new AuthSrvImpl(use_cache, xfrout_client)),
-    io_service_(NULL),
     checkin_(new ConfigChecker(this)),
     dns_lookup_(new MessageLookup(this)),
     dns_answer_(new MessageAnswer(this))
@@ -203,10 +211,7 @@ AuthSrv::AuthSrv(const bool use_cache, AbstractXfroutClient& xfrout_client) :
 
 void
 AuthSrv::stop() {
-    if (io_service_ == NULL) {
-        throw FatalError("Assumption failure; server is stopped before start");
-    }
-    io_service_->stop();
+    impl_->io_service_.stop();
 }
 
 AuthSrv::~AuthSrv() {
@@ -278,6 +283,11 @@ AuthSrv::getVerbose() const {
     return (impl_->verbose_mode_);
 }
 
+IOService&
+AuthSrv::getIOService() {
+    return (impl_->io_service_);
+}
+
 void
 AuthSrv::setCacheSlots(const size_t slots) {
     impl_->cache_.setSlots(slots);
@@ -341,6 +351,32 @@ AuthSrv::setMemoryDataSrc(const isc::dns::RRClass& rrclass,
     impl_->memory_datasrc_ = memory_datasrc;
 }
 
+uint32_t
+AuthSrv::getStatisticsTimerInterval() const {
+    return (impl_->statistics_timer_.getInterval());
+}
+
+void
+AuthSrv::setStatisticsTimerInterval(uint32_t interval) {
+    if (interval == impl_->statistics_timer_.getInterval()) {
+        return;
+    }
+    if (interval == 0) {
+        impl_->statistics_timer_.cancel();
+    } else {
+        impl_->statistics_timer_.setupTimer(
+            boost::bind(&AuthSrv::submitStatistics, this), interval);
+    }
+    if (impl_->verbose_mode_) {
+        if (interval == 0) {
+            cerr << "[b10-auth] Disabled statistics timer" << endl;
+        } else {
+            cerr << "[b10-auth] Set statistics timer to " << interval
+                 << " seconds" << endl;
+        }
+    }
+}
+
 void
 AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
                         OutputBufferPtr buffer, DNSServer* server)

src/bin/auth/auth_srv.h

@@ -91,9 +91,8 @@ public:
     ///
     /// It stops the internal event loop of the server and subsequently
     /// returns the control to the top level context.
-    /// The server must have been associated with an \c IOService object;
-    /// otherwise an exception of \c FatalError will be thrown.
-    /// This method never throws an exception otherwise.
+    ///
+    /// This method should never throw an exception.
     void stop();
 
     /// \brief Process an incoming DNS message, then signal 'server' to resume 
@@ -194,11 +193,8 @@ public:
     /// control commands and configuration updates.
     void setConfigSession(isc::config::ModuleCCSession* config_session);
 
-    /// \brief Assign an ASIO IO Service queue to this Resolver object
-    void setIOService(asiolink::IOService& ios) { io_service_ = &ios; }
-
     /// \brief Return this object's ASIO IO Service queue
-    asiolink::IOService& getIOService() const { return (*io_service_); }
+    asiolink::IOService& getIOService();
 
     /// \brief Return pointer to the DNS Lookup callback function
     asiolink::DNSLookup* getDNSLookupProvider() const { return (dns_lookup_); }
@@ -312,6 +308,28 @@ public:
     /// is shutdown.
     void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
 
+    /// Return the interval of periodic submission of statistics in seconds.
+    ///
+    /// If the statistics submission is disabled, it returns 0.
+    ///
+    /// This method never throws an exception.
+    uint32_t getStatisticsTimerInterval() const;
+
+    /// Set the interval of periodic submission of statistics.
+    ///
+    /// If the specified value is non 0, the \c AuthSrv object will submit
+    /// its statistics to the statistics module every \c interval seconds.
+    /// If it's 0, and \c AuthSrv currently submits statistics, the submission
+    /// will be disabled.
+    ///
+    /// This method should normally not throw an exception; however, its
+    /// underlying library routines may involve resource allocation, and
+    /// when it fails it would result in a corresponding standard exception.
+    ///
+    /// \param interval The submission interval in seconds if non 0;
+    /// or a value of 0 to disable the submission.
+    void setStatisticsTimerInterval(uint32_t interval);
+
     /// \brief Submit statistics counters to statistics module.
     ///
     /// This function can throw an exception from
@@ -338,7 +356,6 @@ public:
 
 private:
     AuthSrvImpl* impl_;
-    asiolink::IOService* io_service_;
     asiolink::SimpleCallback* checkin_;
     asiolink::DNSLookup* dns_lookup_;
     asiolink::DNSAnswer* dns_answer_;

src/bin/auth/config.cc

@@ -169,16 +169,25 @@ MemoryDatasourceConfig::build(ConstElementPtr config_value) {
     }
 }
 
+/// A derived \c AuthConfigParser class for the "statistics-internal"
+/// configuration identifier.
 class StatisticsIntervalConfig : public AuthConfigParser {
 public:
     StatisticsIntervalConfig(AuthSrv& server) :
-        server_(server)
+        server_(server), interval_(0)
     {}
-    //virtual void build(ConstElementPtr config_value);
-    virtual void build(ConstElementPtr) {}
-    virtual void commit() {}
+    virtual void build(ConstElementPtr config_value) {
+        interval_ = config_value->intValue();
+    }
+    virtual void commit() {
+        // setStatisticsTimerInterval() is not 100% exception free.  But
+        // exceptions should happen only in a very rare situation, so we
+        // let them be thrown and subsequently regard them as a fatal error.
+        server_.setStatisticsTimerInterval(interval_);
+    }
 private:
     AuthSrv& server_;
+    uint32_t interval_;
 };
 
 /// A special parser for testing: it throws from commit() despite the

src/bin/auth/main.cc

@@ -25,8 +25,6 @@
 #include <cassert>
 #include <iostream>
 
-#include <boost/bind.hpp>
-
 #include <exceptions/exceptions.h>
 
 #include <dns/buffer.h>
@@ -62,17 +60,11 @@ bool verbose_mode = false;
 // Default port current 5300 for testing purposes
 const char* DNSPORT = "5300";
 
-// Note: this value must be greater than 0.
-// TODO: make it configurable via command channel.
-const uint32_t STATISTICS_SEND_INTERVAL_SEC = 60;
-
 /* need global var for config/command handlers.
  * todo: turn this around, and put handlers in the authserver
  * class itself? */
 AuthSrv *auth_server;
 
-IOService io_service;
-
 ConstElementPtr
 my_config_handler(ConstElementPtr new_config) {
     return (auth_server->updateConfig(new_config));
@@ -98,12 +90,6 @@ usage() {
     cerr << "\t-v: verbose output" << endl;
     exit(1);
 }
-
-void
-statisticsTimerCallback(AuthSrv* auth_server) {
-    assert(auth_server != NULL);
-    auth_server->submitStatistics();
-}
 } // end of anonymous namespace
 
 int
@@ -170,7 +156,6 @@ main(int argc, char* argv[]) {
     Session* cc_session = NULL;
     Session* xfrin_session = NULL;
     Session* statistics_session = NULL;
-    IntervalTimer* itimer = NULL;
     bool xfrin_session_established = false; // XXX (see Trac #287)
     bool statistics_session_established = false; // XXX (see Trac #287)
     ModuleCCSession* config_session = NULL;
@@ -195,6 +180,7 @@ main(int argc, char* argv[]) {
         cout << "[b10-auth] Server created." << endl;
 
         SimpleCallback* checkin = auth_server->getCheckinProvider();
+        IOService& io_service = auth_server->getIOService();
         DNSLookup* lookup = auth_server->getDNSLookupProvider();
         DNSAnswer* answer = auth_server->getDNSAnswerProvider();
 
@@ -213,8 +199,7 @@ main(int argc, char* argv[]) {
                                          use_ipv6, checkin, lookup,
                                          answer);
         }
-        auth_server->setIOService(io_service);
-        cout << "[b10-auth] IOService created." << endl;
+        cout << "[b10-auth] DNSServices created." << endl;
 
         cc_session = new Session(io_service.get_io_service());
         cout << "[b10-auth] Configuration session channel created." << endl;
@@ -240,11 +225,6 @@ main(int argc, char* argv[]) {
         statistics_session_established = true;
         cout << "[b10-auth] Statistics session channel established." << endl;
 
-        // XXX: with the current interface to asiolink we have to create
-        // auth_server before io_service while Session needs io_service.
-        // In a next step of refactoring we should make asiolink independent
-        // from auth_server, and create io_service, auth_server, and
-        // sessions in that order.
         auth_server->setXfrinSession(xfrin_session);
         auth_server->setStatisticsSession(statistics_session);
 
@@ -256,14 +236,6 @@ main(int argc, char* argv[]) {
         configureAuthServer(*auth_server, config_session->getFullConfig());
         auth_server->updateConfig(ElementPtr());
 
-        // create interval timer instance
-        itimer = new IntervalTimer(io_service);
-        // set up interval timer
-        // register function to send statistics with interval
-        itimer->setupTimer(boost::bind(statisticsTimerCallback, auth_server),
-                           STATISTICS_SEND_INTERVAL_SEC);
-        cout << "[b10-auth] Interval timer to send statistics set." << endl;
-
         cout << "[b10-auth] Server started." << endl;
         io_service.run();
 
@@ -281,7 +253,6 @@ main(int argc, char* argv[]) {
         xfrin_session->disconnect();
     }
 
-    delete itimer;
     delete statistics_session;
     delete xfrin_session;
     delete config_session;

src/bin/auth/tests/auth_srv_unittest.cc

@@ -646,8 +646,7 @@ TEST_F(AuthSrvTest, stop) {
     // normal case is covered in command_unittest.cc.  we should primarily
     // test it here, but the current design of the stop test takes time,
     // so we consolidate the cases in the command tests.
-
-    // stop before start is prohibited.
-    EXPECT_THROW(server.stop(), FatalError);
+    // If/when the interval timer has finer granularity we'll probably add
+    // our own tests here, so we keep this empty test case.
 }
 }

src/bin/auth/tests/command_unittest.cc

@@ -97,11 +97,9 @@ AuthConmmandTest::stopServer() {
 }
 
 TEST_F(AuthConmmandTest, shutdown) {
-    asiolink::IOService io_service;
-    asiolink::IntervalTimer itimer(io_service);
-    server.setIOService(io_service);
+    asiolink::IntervalTimer itimer(server.getIOService());
     itimer.setupTimer(boost::bind(&AuthConmmandTest::stopServer, this), 1);
-    io_service.run();
+    server.getIOService().run();
     EXPECT_EQ(0, rcode);
 }
 

src/bin/auth/tests/config_unittest.cc

@@ -34,6 +34,7 @@
 using namespace isc::dns;
 using namespace isc::data;
 using namespace isc::datasrc;
+using namespace asiolink;
 
 namespace {
 class AuthConfigTest : public ::testing::Test {
@@ -325,18 +326,43 @@ class StatisticsIntervalConfigTest : public AuthConfigTest {
 protected:
     StatisticsIntervalConfigTest() :
         parser(createAuthConfigParser(server, "statistics-interval"))
-    {
-        server.setStatisticsSession(&statistics_session);
-    }
+    {}
     ~StatisticsIntervalConfigTest() {
         delete parser;
     }
-    MockSession statistics_session;
     AuthConfigParser* parser;
 };
 
 TEST_F(StatisticsIntervalConfigTest, setInterval) {
-    parser->build(Element::fromJSON("1"));
+    // initially the timer is not configured.
+    EXPECT_EQ(0, server.getStatisticsTimerInterval());
+
+    // initialize the timer
+    parser->build(Element::fromJSON("5"));
+    parser->commit();
+    EXPECT_EQ(5, server.getStatisticsTimerInterval());
+
+    // reset the timer with a new interval
+    delete parser;
+    parser = createAuthConfigParser(server, "statistics-interval");
+    ASSERT_NE(static_cast<void*>(NULL), parser);
+    parser->build(Element::fromJSON("10"));
     parser->commit();
+    EXPECT_EQ(10, server.getStatisticsTimerInterval());
+
+    // disable the timer again
+    delete parser;
+    parser = createAuthConfigParser(server, "statistics-interval");
+    ASSERT_NE(static_cast<void*>(NULL), parser);
+    parser->build(Element::fromJSON("0"));
+    parser->commit();
+    EXPECT_EQ(0, server.getStatisticsTimerInterval());
+}
+
+TEST_F(StatisticsIntervalConfigTest, badInterval) {
+    EXPECT_THROW(parser->build(Element::fromJSON("\"should be integer\"")),
+                 isc::data::TypeError);
+    EXPECT_THROW(parser->build(Element::fromJSON("2.5")),
+                 isc::data::TypeError);
 }
 }