Commit 468ed141 authored by Yoshitaka Aharen's avatar Yoshitaka Aharen
Browse files

Address some review comments [comment:ticket:347:5]

* Timer call back scheme of submitting statistics changed.
* Avoid dynamic allocations.
* Some files and classes were renamed.
  - stats.{cc,h} -> statistics.{cc,h}
  - class Counter -> class QueryCounters
* Remove unnecessary includes.
* Some test cases were appended.
  - class IntervalTimer
  - Submitted values from QueryCounters::submitStatistics()
  - Counter increment with handling query
* Remove namespace 'statistics'.
* Argument to constructor of IntervalTimer was changed
  to asio_link::IOService&.
* Error handling improved
  - Parameter check
  - Exception handling
* Documentation and comments were appended.



git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac347@3782 e5f2f494-b856-4b98-b285-d166d9295462
parent 928f3347
TBD. [func] y-aharen TBD. [func] y-aharen
src/bin/auth: Added a feature to count query and send counter src/bin/auth: Added a feature to count query and send counter
values to b10-stats periodically. To support it, added wrapping values to statistics periodically. To support it, added wrapping
class of asio::deadline_timer to use as interval timer. class of asio::deadline_timer to use as interval timer.
Added a command 'sendstats' to send counter values at once. Added a command 'sendstats' to send counter values at once.
The counter value can be seen with sending 'sendstats' command to
statistics module via bindctl like:
... "auth.queries.tcp": 1, "auth.queries.udp": 1 ...
(Trac #347, svn rxxxx) (Trac #347, svn rxxxx)
122. [func] stephen 122. [func] stephen
......
...@@ -58,7 +58,7 @@ pkglibexec_PROGRAMS = b10-auth ...@@ -58,7 +58,7 @@ pkglibexec_PROGRAMS = b10-auth
b10_auth_SOURCES = auth_srv.cc auth_srv.h b10_auth_SOURCES = auth_srv.cc auth_srv.h
b10_auth_SOURCES += change_user.cc change_user.h b10_auth_SOURCES += change_user.cc change_user.h
b10_auth_SOURCES += common.h b10_auth_SOURCES += common.h
b10_auth_SOURCES += stats.cc stats.h b10_auth_SOURCES += statistics.cc statistics.h
b10_auth_SOURCES += main.cc b10_auth_SOURCES += main.cc
b10_auth_LDADD = $(top_builddir)/src/lib/datasrc/libdatasrc.la b10_auth_LDADD = $(top_builddir)/src/lib/datasrc/libdatasrc.la
b10_auth_LDADD += $(top_builddir)/src/lib/dns/libdns++.la b10_auth_LDADD += $(top_builddir)/src/lib/dns/libdns++.la
......
...@@ -669,64 +669,74 @@ IOService::setCallBack(const IOCallBack callback) { ...@@ -669,64 +669,74 @@ IOService::setCallBack(const IOCallBack callback) {
class IntervalTimerImpl { class IntervalTimerImpl {
private: private:
// prohibit copy to avoid the function be called twice // prohibit copy
IntervalTimerImpl(const IntervalTimerImpl& source); IntervalTimerImpl(const IntervalTimerImpl& source);
IntervalTimerImpl& operator=(const IntervalTimerImpl& source); IntervalTimerImpl& operator=(const IntervalTimerImpl& source);
public: public:
IntervalTimerImpl(asio::io_service& io_service); IntervalTimerImpl(IOService& io_service);
~IntervalTimerImpl(); ~IntervalTimerImpl();
bool setupTimer(const IntervalTimer::Callback& cbfunc, void setupTimer(const IntervalTimer::Callback& cbfunc,
const uint32_t interval); const uint32_t interval);
void callback(const asio::error_code& error); void callback(const asio::error_code& error);
private: private:
// a function to update timer_ when it expires
void updateTimer(); void updateTimer();
// a function to call back when timer_ expires
IntervalTimer::Callback cbfunc_; IntervalTimer::Callback cbfunc_;
// interval in seconds // interval in seconds
uint32_t interval_; uint32_t interval_;
// asio timer
asio::deadline_timer timer_; asio::deadline_timer timer_;
}; };
IntervalTimerImpl::IntervalTimerImpl(asio::io_service& io_service) : IntervalTimerImpl::IntervalTimerImpl(IOService& io_service) :
timer_(io_service) timer_(io_service.get_io_service())
{} {}
IntervalTimerImpl::~IntervalTimerImpl() { IntervalTimerImpl::~IntervalTimerImpl()
timer_.cancel(); {}
}
bool void
IntervalTimerImpl::setupTimer(const IntervalTimer::Callback& cbfunc, IntervalTimerImpl::setupTimer(const IntervalTimer::Callback& cbfunc,
const uint32_t interval) const uint32_t interval)
{ {
// interval value must be positive // Interval should not be 0.
assert(interval > 0); if (interval == 0) {
isc_throw(isc::BadValue, "Interval should not be 0");
}
// Call back function should not be empty.
if (cbfunc.empty()) {
isc_throw(isc::InvalidParameter, "Callback function is empty");
}
cbfunc_ = cbfunc; cbfunc_ = cbfunc;
interval_ = interval; interval_ = interval;
// start timer // Set initial expire time.
// At this point the timer is not running yet and will not expire.
// After calling IOService::run(), the timer will expire.
updateTimer(); updateTimer();
return (true); return;
} }
void void
IntervalTimerImpl::updateTimer() { IntervalTimerImpl::updateTimer() {
// update expire time (current time + interval_) // Update expire time to (current time + interval_).
timer_.expires_from_now(boost::posix_time::seconds(interval_)); timer_.expires_from_now(boost::posix_time::seconds(interval_));
// restart timer // Reset timer.
timer_.async_wait(boost::bind(&IntervalTimerImpl::callback, this, _1)); timer_.async_wait(boost::bind(&IntervalTimerImpl::callback, this, _1));
} }
void void
IntervalTimerImpl::callback(const asio::error_code& cancelled) { IntervalTimerImpl::callback(const asio::error_code& cancelled) {
assert(!cbfunc_.empty()); // Do not call cbfunc_ in case the timer was cancelled.
// skip function call in case the timer was cancelled // The timer will be canelled in the destructor of asio::deadline_timer.
if (!cancelled) { if (!cancelled) {
cbfunc_(); cbfunc_();
// restart timer // Set next expire time.
updateTimer(); updateTimer();
} }
} }
IntervalTimer::IntervalTimer(asio::io_service& io_service) { IntervalTimer::IntervalTimer(IOService& io_service) {
impl_ = new IntervalTimerImpl(io_service); impl_ = new IntervalTimerImpl(io_service);
} }
...@@ -734,7 +744,7 @@ IntervalTimer::~IntervalTimer() { ...@@ -734,7 +744,7 @@ IntervalTimer::~IntervalTimer() {
delete impl_; delete impl_;
} }
bool void
IntervalTimer::setupTimer(const Callback& cbfunc, const uint32_t interval) { IntervalTimer::setupTimer(const Callback& cbfunc, const uint32_t interval) {
return (impl_->setupTimer(cbfunc, interval)); return (impl_->setupTimer(cbfunc, interval));
} }
......
...@@ -446,15 +446,45 @@ private: ...@@ -446,15 +446,45 @@ private:
IOServiceImpl* impl_; IOServiceImpl* impl_;
}; };
/// \brief The \c IntervalTimer class is a wrapper for the ASIO \c deadline_timer /// \brief The \c IntervalTimer class is a wrapper for the ASIO
/// class. /// \c asio::deadline_timer class.
///
/// This class is implemented to use \c asio::deadline_timer as
/// interval timer.
///
/// \c setupTimer() sets a timer to expire on (now + interval) and
/// a call back function.
///
/// \c IntervalTimerImpl::callback() is called by the timer when
/// it expires.
///
/// The function calls the call back function set by \c setupTimer()
/// and update the timer to expire on (now + interval).
/// The type of call back function is \c void(void).
///
/// This class is mainly designed to use for calling
/// \c QueryCounters::submitStatistics() periodically.
///
/// Note: Destruction of the instance of this class while call back
/// is pending causes throwing an exception from IOService.
///
/// Sample code:
/// \code
/// void function_to_call_back() {
/// // this function will called periodically
/// }
/// int interval_in_seconds = 1;
/// IOService io_service;
///
/// IntervalTimer intervalTimer(io_service);
/// intervalTimer.setupTimer(function_to_call_back, interval_in_seconds);
/// io_service.run();
/// \endcode
/// ///
/// This class is implemented to use boost::deadline_timer as interval timer.
/// Copy of this class is prohibited not to call the callback function twice.
class IntervalTimer { class IntervalTimer {
public: public:
/// \name The type of timer callback function /// \name The type of timer callback function
typedef boost::function<void(void)> Callback; typedef boost::function<void()> Callback;
/// ///
/// \name Constructors and Destructor /// \name Constructors and Destructor
...@@ -466,22 +496,43 @@ private: ...@@ -466,22 +496,43 @@ private:
IntervalTimer(const IntervalTimer& source); IntervalTimer(const IntervalTimer& source);
IntervalTimer& operator=(const IntervalTimer& source); IntervalTimer& operator=(const IntervalTimer& source);
public: public:
/// \brief The constructor with asio::io_service. /// \brief The constructor with \c IOService.
/// ///
/// \param io_service A reference to an instance of asio::io_service /// This constructor may throw a standard exception if
IntervalTimer(asio::io_service& io_service); /// memory allocation fails inside the method.
/// This constructor may also throw \c asio::system_error.
///
/// \param io_service A reference to an instance of IOService
///
IntervalTimer(IOService& io_service);
/// \brief The destructor. /// \brief The destructor.
///
/// This destructor never throws an exception.
///
/// On the destruction of this class the timer will be cancelled
/// inside \c asio::deadline_timer.
///
~IntervalTimer(); ~IntervalTimer();
//@} //@}
/// \brief Register timer callback function /// \brief Register timer callback function and interval
///
/// This function sets call back function and interval in seconds.
/// Timer will actually start after calling \c IOService::run().
/// ///
/// \param cbfunc A reference to a function to call back /// \param cbfunc A reference to a function \c void(void) to call back
/// when the timer is expired /// when the timer is expired
/// \param interval Interval in seconds /// \param interval Interval in seconds (greater than 0)
///
/// Note: IntervalTimer will not pass asio::error_code to
/// call back function. In case the timer is cancelled, the function
/// will not be called.
///
/// \throw isc::InvalidParameter cbfunc is empty
/// \throw isc::BadValue interval is 0
/// \throw asio::system_error ASIO library error
/// ///
/// \return \c true on success void setupTimer(const Callback& cbfunc, const uint32_t interval);
bool setupTimer(const Callback& cbfunc, const uint32_t interval);
private: private:
IntervalTimerImpl* impl_; IntervalTimerImpl* impl_;
}; };
......
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
}, },
{ {
"command_name": "sendstats", "command_name": "sendstats",
"command_description": "Send statistics data to b10-stats at once", "command_description": "Send data to a statistics module at once",
"command_args": [] "command_args": []
} }
] ]
......
...@@ -50,6 +50,7 @@ ...@@ -50,6 +50,7 @@
#include <auth/common.h> #include <auth/common.h>
#include <auth/auth_srv.h> #include <auth/auth_srv.h>
#include <auth/asio_link.h> #include <auth/asio_link.h>
#include <auth/statistics.h>
using namespace std; using namespace std;
...@@ -99,6 +100,9 @@ public: ...@@ -99,6 +100,9 @@ public:
/// Hot spot cache /// Hot spot cache
isc::datasrc::HotCache cache_; isc::datasrc::HotCache cache_;
/// Query counters for statistics
QueryCounters counters_;
}; };
AuthSrvImpl::AuthSrvImpl(const bool use_cache, AuthSrvImpl::AuthSrvImpl(const bool use_cache,
...@@ -106,7 +110,8 @@ AuthSrvImpl::AuthSrvImpl(const bool use_cache, ...@@ -106,7 +110,8 @@ AuthSrvImpl::AuthSrvImpl(const bool use_cache,
config_session_(NULL), verbose_mode_(false), config_session_(NULL), verbose_mode_(false),
xfrin_session_(NULL), xfrin_session_(NULL),
xfrout_connected_(false), xfrout_connected_(false),
xfrout_client_(xfrout_client) xfrout_client_(xfrout_client),
counters_(verbose_mode_)
{ {
// cur_datasrc_ is automatically initialized by the default constructor, // cur_datasrc_ is automatically initialized by the default constructor,
// effectively being an empty (sqlite) data source. once ccsession is up // effectively being an empty (sqlite) data source. once ccsession is up
...@@ -128,13 +133,10 @@ AuthSrvImpl::~AuthSrvImpl() { ...@@ -128,13 +133,10 @@ AuthSrvImpl::~AuthSrvImpl() {
AuthSrv::AuthSrv(const bool use_cache, AbstractXfroutClient& xfrout_client) : AuthSrv::AuthSrv(const bool use_cache, AbstractXfroutClient& xfrout_client) :
impl_(new AuthSrvImpl(use_cache, xfrout_client)) impl_(new AuthSrvImpl(use_cache, xfrout_client))
{ {}
counter = new statistics::Counter(impl_->verbose_mode_);
}
AuthSrv::~AuthSrv() { AuthSrv::~AuthSrv() {
delete impl_; delete impl_;
delete counter;
} }
namespace { namespace {
...@@ -219,7 +221,7 @@ AuthSrv::setConfigSession(ModuleCCSession* config_session) { ...@@ -219,7 +221,7 @@ AuthSrv::setConfigSession(ModuleCCSession* config_session) {
void void
AuthSrv::setStatsSession(AbstractSession* stats_session) { AuthSrv::setStatsSession(AbstractSession* stats_session) {
counter->setStatsSession(stats_session); impl_->counters_.setStatsSession(stats_session);
} }
ModuleCCSession* ModuleCCSession*
...@@ -276,15 +278,13 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message, ...@@ -276,15 +278,13 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
// increment Query Counter // increment Query Counter
if (io_message.getSocket().getProtocol() == IPPROTO_UDP) { if (io_message.getSocket().getProtocol() == IPPROTO_UDP) {
counter->inc(statistics::Counter::COUNTER_UDP); impl_->counters_.inc(QueryCounters::COUNTER_UDP);
} else if (io_message.getSocket().getProtocol() == IPPROTO_TCP) { } else if (io_message.getSocket().getProtocol() == IPPROTO_TCP) {
counter->inc(statistics::Counter::COUNTER_TCP); impl_->counters_.inc(QueryCounters::COUNTER_TCP);
} else { } else {
// unknown protocol // unknown protocol
if (impl_->verbose_mode_) { isc_throw(Unexpected, "Unknown protocol: " <<
cerr << "[b10-auth] Unknown protocol: " << io_message.getSocket().getProtocol());
io_message.getSocket().getProtocol() << endl;
}
} }
// Perform further protocol-level validation. // Perform further protocol-level validation.
...@@ -565,9 +565,11 @@ AuthSrv::updateConfig(ConstElementPtr new_config) { ...@@ -565,9 +565,11 @@ AuthSrv::updateConfig(ConstElementPtr new_config) {
} }
} }
asio_link::IntervalTimer::Callback bool AuthSrv::submitStatistics() {
AuthSrv::getStatsCallback() { return (impl_->counters_.submitStatistics());
// just invoke statistics::Counter::getStatsCallback() }
// and return its return value
return (counter->getCallback()); const std::vector<uint64_t>&
AuthSrv::getCounters() const {
return (impl_->counters_.getCounters());
} }
...@@ -22,12 +22,6 @@ ...@@ -22,12 +22,6 @@
#include <cc/data.h> #include <cc/data.h>
#include <config/ccsession.h> #include <config/ccsession.h>
/// for the typedef of asio_link::IntervalTimer::Callback
#include <auth/asio_link.h>
// for class statistics::Counter
#include <auth/stats.h>
namespace isc { namespace isc {
namespace dns { namespace dns {
class InputBuffer; class InputBuffer;
...@@ -208,25 +202,50 @@ public: ...@@ -208,25 +202,50 @@ public:
/// \brief Set the communication session with Statistics. /// \brief Set the communication session with Statistics.
/// ///
/// This function never throws an exception as far as
/// QueryCounters::setStatsSession() doesn't throw.
///
/// Note: this interface is tentative. We'll revisit the ASIO and
/// session frameworks, at which point the session will probably
/// be passed on construction of the server.
///
/// \param stats_session A Session object over which statistics
/// information is exchanged with statistics module.
/// The session must be established before setting in the server
/// object.
/// Ownership isn't transferred: the caller is responsible for keeping
/// this object to be valid while the server object is working and for
/// disconnecting the session and destroying the object when the server
/// is shutdown.
///
void setStatsSession(isc::cc::AbstractSession* stats_session); void setStatsSession(isc::cc::AbstractSession* stats_session);
/// \brief Return the function that sends statistics information /// \brief Submit statistics counters to statistics module.
/// to Statistics module. ///
/// This function can throw an exception from
/// QueryCounters::submitStatistics().
///
/// \return true on success, false on failure (e.g. session timeout,
/// session error).
///
bool submitStatistics();
/// \brief Get counters in the QueryCounters.
/// ///
/// This function returns the return value of /// This function calls QueryCounters::getCounters() and
/// statistics::Counter::getCallback(). /// returns its return velue, a reference to the counters.
/// ///
/// \return \c boost::function which contains the procedure /// This function never throws an exception as far as
/// to send statistics. /// QueryCounters::getCounters() doesn't throw.
asio_link::IntervalTimer::Callback getStatsCallback(); ///
/// Note: Currently this function is for testing purpose only.
/// This function should not be called except from tests.
///
/// \return a reference to the counters.
///
const std::vector<uint64_t>& getCounters() const;
private: private:
AuthSrvImpl* impl_; AuthSrvImpl* impl_;
// TODO: consider where to put the counter.
// Currently, count-up is in AuthSrv::processMessage.
// In the future, count-up will be in AuthSrvImpl::process*Query
// and this declaration will be moved into AuthSrvImpl.
statistics::Counter *counter;
}; };
#endif // __AUTH_SRV_H #endif // __AUTH_SRV_H
......
...@@ -9,7 +9,7 @@ CLEANFILES = *.gcno *.gcda ...@@ -9,7 +9,7 @@ CLEANFILES = *.gcno *.gcda
noinst_PROGRAMS = query_bench noinst_PROGRAMS = query_bench
query_bench_SOURCES = query_bench.cc query_bench_SOURCES = query_bench.cc
query_bench_SOURCES += ../auth_srv.h ../auth_srv.cc query_bench_SOURCES += ../auth_srv.h ../auth_srv.cc
query_bench_SOURCES += ../stats.h ../stats.cc query_bench_SOURCES += ../statistics.h ../statistics.cc
query_bench_LDADD = $(top_builddir)/src/lib/dns/libdns++.la query_bench_LDADD = $(top_builddir)/src/lib/dns/libdns++.la
query_bench_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la query_bench_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
......
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include <iostream> #include <iostream>
#include <boost/foreach.hpp> #include <boost/foreach.hpp>
#include <boost/bind.hpp>
#include <exceptions/exceptions.h> #include <exceptions/exceptions.h>
...@@ -89,9 +90,8 @@ my_command_handler(const string& command, ConstElementPtr args) { ...@@ -89,9 +90,8 @@ my_command_handler(const string& command, ConstElementPtr args) {
if (verbose_mode) { if (verbose_mode) {
cerr << "[b10-auth] command 'sendstats' received" << endl; cerr << "[b10-auth] command 'sendstats' received" << endl;
} }
if (auth_server != NULL) { assert(auth_server != NULL);
auth_server->getStatsCallback()(); auth_server->submitStatistics();
}
} }
return (answer); return (answer);
...@@ -104,6 +104,12 @@ usage() { ...@@ -104,6 +104,12 @@ usage() {
} }
} // end of anonymous namespace } // end of anonymous namespace
void
statisticsTimerCallback(AuthSrv* auth_server) {
assert(auth_server != NULL);
auth_server->submitStatistics();
}
int int
main(int argc, char* argv[]) { main(int argc, char* argv[]) {
int ch; int ch;
...@@ -240,10 +246,11 @@ main(int argc, char* argv[]) { ...@@ -240,10 +246,11 @@ main(int argc, char* argv[]) {
auth_server->updateConfig(ElementPtr()); auth_server->updateConfig(ElementPtr());
// create interval timer instance // create interval timer instance
itimer = new asio_link::IntervalTimer(io_service->get_io_service()); itimer = new asio_link::IntervalTimer(*io_service);
// set up interval timer // set up interval timer
// register function to send statistics with interval // register function to send statistics with interval
itimer->setupTimer(auth_server->getStatsCallback(), itimer->setupTimer(boost::bind(statisticsTimerCallback,
auth_server),
STATS_SEND_INTERVAL_SEC); STATS_SEND_INTERVAL_SEC);
cout << "[b10-auth] Interval timer set to send stats." << endl; cout << "[b10-auth] Interval timer set to send stats." << endl;
......
...@@ -14,131 +14,149 @@ ...@@ -14,131 +14,149 @@
// $Id$ // $Id$
#include <boost/bind.hpp> #include <auth/statistics.h>
#include <auth/stats.h>
#include <cc/data.h> #include <cc/data.h>
#include <cc/session.h>
#include <sstream> #include <sstream>
#include <iostream> #include <iostream>
namespace statistics { // TODO: We need a namespace ("auth_server"?) to hold
// AuthSrv and QueryCounters.
class CounterImpl {