Merge branch 'trac513'

src/bin/auth/auth.spec.pre.in

@@ -52,6 +52,11 @@
 	    }
 	  ]
         }
+      },
+      { "item_name": "statistics-interval",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 60
       }
     ],
     "commands": [

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,9 @@ public:
     /// Hot spot cache
     isc::datasrc::HotCache cache_;
 
+    /// Interval timer for periodic submission of statistics counters.
+    IntervalTimer statistics_timer_;
+
     /// Query counters for statistics
     AuthCounters counters_;
 private:
@@ -125,6 +132,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 +203,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 +210,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 +282,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 +350,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,6 +169,32 @@ 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), interval_(0)
+    {}
+    virtual void build(ConstElementPtr config_value) {
+        const int32_t config_interval = config_value->intValue();
+        if (config_interval < 0) {
+            isc_throw(AuthConfigError, "negative statistics-interval value: "
+                      << config_interval);
+        }
+        interval_ = config_interval;
+    }
+    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
 /// suggested convention of the class interface.
 class ThrowerCommitConfig : public AuthConfigParser {
@@ -191,6 +217,8 @@ createAuthConfigParser(AuthSrv& server, const std::string& config_id,
     // that it can be dynamically customized.
     if (config_id == "datasources") {
         return (new DatasourcesConfig(server));
+    } else if (config_id == "statistics-interval") {
+        return (new StatisticsIntervalConfig(server));
     } else if (internal && config_id == "datasources/memory") {
         return (new MemoryDatasourceConfig(server));
     } else if (config_id == "_commit_throw") {

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 {
@@ -320,4 +321,49 @@ TEST_F(MemoryDatasrcConfigTest, badDatasrcType) {
                                                  " {\"type\": \"memory\"}]")),
                  AuthConfigError);
 }
+
+class StatisticsIntervalConfigTest : public AuthConfigTest {
+protected:
+    StatisticsIntervalConfigTest() :
+        parser(createAuthConfigParser(server, "statistics-interval"))
+    {}
+    ~StatisticsIntervalConfigTest() {
+        delete parser;
+    }
+    AuthConfigParser* parser;
+};
+
+TEST_F(StatisticsIntervalConfigTest, setInterval) {
+    // 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);
+    EXPECT_THROW(parser->build(Element::fromJSON("-1")), AuthConfigError);
+}
 }

src/lib/asiolink/asiolink.cc

@@ -386,6 +386,11 @@ public:
     void setupTimer(const IntervalTimer::Callback& cbfunc,
                     const uint32_t interval);
     void callback(const asio::error_code& error);
+    void cancel() {
+        timer_.cancel();
+        interval_ = 0;
+    }
+    uint32_t getInterval() const { return (interval_); }
 private:
     // a function to update timer_ when it expires
     void updateTimer();
@@ -398,7 +403,7 @@ private:
 };
 
 IntervalTimerImpl::IntervalTimerImpl(IOService& io_service) :
-    timer_(io_service.get_io_service())
+    interval_(0), timer_(io_service.get_io_service())
 {}
 
 IntervalTimerImpl::~IntervalTimerImpl()
@@ -427,6 +432,10 @@ IntervalTimerImpl::setupTimer(const IntervalTimer::Callback& cbfunc,
 
 void
 IntervalTimerImpl::updateTimer() {
+    if (interval_ == 0) {
+        // timer has been canceled.  Do nothing.
+        return;
+    }
     try {
         // Update expire time to (current time + interval_).
         timer_.expires_from_now(boost::posix_time::seconds(interval_));
@@ -461,4 +470,14 @@ IntervalTimer::setupTimer(const Callback& cbfunc, const uint32_t interval) {
     return (impl_->setupTimer(cbfunc, interval));
 }
 
+void
+IntervalTimer::cancel() {
+    impl_->cancel();
+}
+
+uint32_t
+IntervalTimer::getInterval() const {
+    return (impl_->getInterval());
+}
+
 }

src/lib/asiolink/asiolink.h

@@ -657,6 +657,29 @@ public:
     /// \throw isc::Unexpected ASIO library error
     ///
     void setupTimer(const Callback& cbfunc, const uint32_t interval);
+
+    /// Cancel the timer.
+    ///
+    /// If the timer has been set up, this method cancels any asynchronous
+    /// events waiting on the timer and stops the timer itself.
+    /// If the timer has already been canceled, this method effectively does
+    /// nothing.
+    ///
+    /// This method never throws an exception.
+    void cancel();
+
+    /// Return the timer interval.
+    ///
+    /// This method returns the timer interval in seconds if it's running;
+    /// if the timer has been canceled it returns 0.
+    ///
+    /// This method never throws an exception.
+    ///
+    /// Note: We may want to change the granularity of the timer to
+    /// milliseconds or even finer.  If and when this happens the semantics
+    /// of the return value of this method will be changed accordingly.
+    uint32_t getInterval() const;
+
 private:
     IntervalTimerImpl* impl_;
 };

src/lib/asiolink/tests/asiolink_unittest.cc

@@ -751,7 +751,7 @@ TEST_F(ASIOLinkTest, recursiveTimeout) {
 // or not.
 class IntervalTimerTest : public ::testing::Test {
 protected:
-    IntervalTimerTest() : io_service_() {};
+    IntervalTimerTest() : io_service_() {}
     ~IntervalTimerTest() {}
     class TimerCallBack : public std::unary_function<void, void> {
     public:
@@ -811,6 +811,19 @@ protected:
         int count_;
         int prev_counter_;
     };
+    class TimerCallBackCanceller {
+    public:
+        TimerCallBackCanceller(unsigned int& counter, IntervalTimer& itimer) :
+            counter_(counter), itimer_(itimer)
+        {}
+        void operator()() {
+            ++counter_;
+            itimer_.cancel();
+        }
+    private:
+        unsigned int& counter_;
+        IntervalTimer& itimer_;
+    };
     class TimerCallBackOverwriter : public std::unary_function<void, void> {
     public:
         TimerCallBackOverwriter(IntervalTimerTest* test_obj,
@@ -864,6 +877,7 @@ TEST_F(IntervalTimerTest, startIntervalTimer) {
     start = boost::posix_time::microsec_clock::universal_time();
     // setup timer
     itimer.setupTimer(TimerCallBack(this), 1);
+    EXPECT_EQ(1, itimer.getInterval());
     io_service_.run();
     // reaches here after timer expired
     // delta: difference between elapsed time and 1 second
@@ -926,6 +940,23 @@ TEST_F(IntervalTimerTest, destructIntervalTimer) {
     EXPECT_TRUE(timer_cancel_success_);
 }
 
+TEST_F(IntervalTimerTest, cancel) {
+    // Similar to destructIntervalTimer test, but the first timer explicitly
+    // cancels itself on first callback.
+    IntervalTimer itimer_counter(io_service_);
+    IntervalTimer itimer_watcher(io_service_);
+    unsigned int counter = 0;
+    itimer_counter.setupTimer(TimerCallBackCanceller(counter, itimer_counter),
+                              1);
+    itimer_watcher.setupTimer(TimerCallBack(this), 3);
+    io_service_.run();
+    EXPECT_EQ(1, counter);
+    EXPECT_EQ(0, itimer_counter.getInterval());
+
+    // canceling an already canceled timer shouldn't cause any surprise.
+    EXPECT_NO_THROW(itimer_counter.cancel());
+}
+
 TEST_F(IntervalTimerTest, overwriteIntervalTimer) {
     // Note: This test currently takes 4 seconds. The timer should have
     // finer granularity and timer periods in this test should be shorter