Commit a75ccad6 authored by Yoshitaka Aharen's avatar Yoshitaka Aharen
Browse files

Address review comments on ticket:347:12

- Documentation and comments were appended.
- Some classes and functions were renamed.
- Some test cases were appended.
- Removed unnecessary includes.
- Thrown exceptions were changed not to hide asio specific definitions.
- Query counter increment is moved from AuthSrv::processMessage() to
  AuthSrvImpl::processNormalQuery() and AuthSrvImpl::processAxfrQuery().



git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac347@3954 e5f2f494-b856-4b98-b285-d166d9295462
parent 17e32022
TBD. [func] y-aharen
src/bin/auth: Added a feature to count query and send counter
src/bin/auth: Added a feature to count queries and send counter
values to statistics periodically. To support it, added wrapping
class of asio::deadline_timer to use as interval timer.
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:
The counters can be seen using the "Stats show" command from
bindctl. The result would look like:
... "auth.queries.tcp": 1, "auth.queries.udp": 1 ...
Using the "Auth sendstats" command you can make b10-auth send the
counters to b10-stats immediately.
(Trac #347, svn rxxxx)
122. [func] stephen
......
......@@ -719,8 +719,12 @@ IntervalTimerImpl::setupTimer(const IntervalTimer::Callback& cbfunc,
void
IntervalTimerImpl::updateTimer() {
// Update expire time to (current time + interval_).
timer_.expires_from_now(boost::posix_time::seconds(interval_));
try {
// Update expire time to (current time + interval_).
timer_.expires_from_now(boost::posix_time::seconds(interval_));
} catch (asio::system_error& e) {
isc_throw(isc::Unexpected, "Failed to update timer");
}
// Reset timer.
timer_.async_wait(boost::bind(&IntervalTimerImpl::callback, this, _1));
}
......
......@@ -462,9 +462,6 @@ private:
/// and updates the timer to expire in (now + interval) seconds.
/// 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 an instance of this class while call back
/// is pending causes throwing an exception from \c IOService.
///
......@@ -516,22 +513,22 @@ public:
~IntervalTimer();
//@}
/// \brief Register timer callback function and interval
/// \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 \c void(void) to call back
/// when the timer is expired
/// when the timer is expired (should not be an empty functor)
/// \param interval Interval in seconds (greater than 0)
///
/// Note: IntervalTimer will not pass asio::error_code to
/// Note: IntervalTimer will not pass \c 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
/// \throw isc::Unexpected ASIO library error
///
void setupTimer(const Callback& cbfunc, const uint32_t interval);
private:
......
......@@ -102,7 +102,10 @@ public:
isc::datasrc::HotCache cache_;
/// Query counters for statistics
QueryCounters counters_;
AuthCounters counters_;
private:
/// Increment query counter
void incCounter(int protocol);
};
AuthSrvImpl::AuthSrvImpl(const bool use_cache,
......@@ -220,8 +223,8 @@ AuthSrv::setConfigSession(ModuleCCSession* config_session) {
}
void
AuthSrv::setStatsSession(AbstractSession* stats_session) {
impl_->counters_.setStatsSession(stats_session);
AuthSrv::setStatisticsSession(AbstractSession* statistics_session) {
impl_->counters_.setStatisticsSession(statistics_session);
}
ModuleCCSession*
......@@ -276,17 +279,6 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
cerr << "[b10-auth] received a message:\n" << message.toText() << endl;
}
// increment Query Counter
if (io_message.getSocket().getProtocol() == IPPROTO_UDP) {
impl_->counters_.inc(QueryCounters::COUNTER_UDP);
} else if (io_message.getSocket().getProtocol() == IPPROTO_TCP) {
impl_->counters_.inc(QueryCounters::COUNTER_TCP);
} else {
// unknown protocol
isc_throw(Unexpected, "Unknown protocol: " <<
io_message.getSocket().getProtocol());
}
// Perform further protocol-level validation.
if (message.getOpcode() == Opcode::NOTIFY()) {
......@@ -334,6 +326,9 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
message.setHeaderFlag(Message::HEADERFLAG_AA);
message.setRcode(Rcode::NOERROR());
// Increment query counter.
incCounter(io_message.getSocket().getProtocol());
if (remote_edns) {
EDNSPtr local_edns = EDNSPtr(new EDNS());
local_edns->setDNSSECAwareness(dnssec_ok);
......@@ -371,6 +366,9 @@ bool
AuthSrvImpl::processAxfrQuery(const IOMessage& io_message, Message& message,
MessageRenderer& response_renderer)
{
// Increment query counter.
incCounter(io_message.getSocket().getProtocol());
if (io_message.getSocket().getProtocol() == IPPROTO_UDP) {
if (verbose_mode_) {
cerr << "[b10-auth] AXFR query over UDP isn't allowed" << endl;
......@@ -498,6 +496,19 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, Message& message,
return (true);
}
void
AuthSrvImpl::incCounter(int protocol) {
// Increment query counter.
if (protocol == IPPROTO_UDP) {
counters_.inc(AuthCounters::COUNTER_UDP);
} else if (protocol == IPPROTO_TCP) {
counters_.inc(AuthCounters::COUNTER_TCP);
} else {
// unknown protocol
isc_throw(Unexpected, "Unknown protocol: " << protocol);
}
}
ConstElementPtr
AuthSrvImpl::setDbFile(ConstElementPtr config) {
ConstElementPtr answer = isc::config::createAnswer();
......@@ -565,11 +576,11 @@ AuthSrv::updateConfig(ConstElementPtr new_config) {
}
}
bool AuthSrv::submitStatistics() {
bool AuthSrv::submitStatistics() const {
return (impl_->counters_.submitStatistics());
}
const std::vector<uint64_t>&
AuthSrv::getCounters() const {
return (impl_->counters_.getCounters());
uint64_t
AuthSrv::getCounter(const AuthCounters::QueryType type) const {
return (impl_->counters_.getCounter(type));
}
......@@ -22,6 +22,8 @@
#include <cc/data.h>
#include <config/ccsession.h>
#include <auth/statistics.h>
namespace isc {
namespace dns {
class InputBuffer;
......@@ -62,6 +64,7 @@ class AuthSrvImpl;
///
/// The design of this class is still in flux. It's quite likely to change
/// in future versions.
///
class AuthSrv {
///
/// \name Constructors, Assignment Operator and Destructor.
......@@ -86,6 +89,8 @@ public:
//@}
/// \return \c true if the \a message contains a response to be returned;
/// otherwise \c false.
///
/// \throw isc::Unexpected Protocol type of \a message is unexpected
bool processMessage(const asio_link::IOMessage& io_message,
isc::dns::Message& message,
isc::dns::MessageRenderer& response_renderer);
......@@ -203,13 +208,13 @@ public:
/// \brief Set the communication session with Statistics.
///
/// This function never throws an exception as far as
/// QueryCounters::setStatsSession() doesn't throw.
/// AuthCounters::setStatisticsSession() 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
/// \param statistics_session A Session object over which statistics
/// information is exchanged with statistics module.
/// The session must be established before setting in the server
/// object.
......@@ -218,32 +223,33 @@ public:
/// disconnecting the session and destroying the object when the server
/// is shutdown.
///
void setStatsSession(isc::cc::AbstractSession* stats_session);
void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
/// \brief Submit statistics counters to statistics module.
///
/// This function can throw an exception from
/// QueryCounters::submitStatistics().
/// AuthCounters::submitStatistics().
///
/// \return true on success, false on failure (e.g. session timeout,
/// session error).
///
bool submitStatistics();
bool submitStatistics() const;
/// \brief Get counters in the QueryCounters.
/// \brief Get the value of counter in the AuthCounters.
///
/// This function calls QueryCounters::getCounters() and
/// returns its return velue, a reference to the counters.
/// This function calls AuthCounters::getCounter() and
/// returns its return value.
///
/// This function never throws an exception as far as
/// QueryCounters::getCounters() doesn't throw.
/// AuthCounters::getCounter() doesn't throw.
///
/// Note: Currently this function is for testing purpose only.
/// This function should not be called except from tests.
///
/// \return a reference to the counters.
/// \param type Type of a counter to get the value of
///
/// \return the value of the counter.
///
const std::vector<uint64_t>& getCounters() const;
uint64_t getCounter(const AuthCounters::QueryType type) const;
private:
AuthSrvImpl* impl_;
};
......
......@@ -61,7 +61,7 @@ const char* DNSPORT = "5300";
// Note: this value must be greater than 0.
// TODO: make it configurable via command channel.
const uint32_t STATS_SEND_INTERVAL_SEC = 60;
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
......@@ -170,10 +170,10 @@ main(int argc, char* argv[]) {
// XXX: we should eventually pass io_service here.
Session* cc_session = NULL;
Session* xfrin_session = NULL;
Session* stats_session = NULL;
Session* statistics_session = NULL;
asio_link::IntervalTimer* itimer = NULL;
bool xfrin_session_established = false; // XXX (see Trac #287)
bool stats_session_established = false; // XXX (see Trac #287)
bool statistics_session_established = false; // XXX (see Trac #287)
ModuleCCSession* config_session = NULL;
string xfrout_socket_path;
if (getenv("B10_FROM_BUILD") != NULL) {
......@@ -228,11 +228,11 @@ main(int argc, char* argv[]) {
xfrin_session_established = true;
cout << "[b10-auth] Xfrin session channel established." << endl;
stats_session = new Session(io_service->get_io_service());
cout << "[b10-auth] Stats session channel created." << endl;
stats_session->establish(NULL);
stats_session_established = true;
cout << "[b10-auth] Stats session channel established." << endl;
statistics_session = new Session(io_service->get_io_service());
cout << "[b10-auth] Statistics session channel created." << endl;
statistics_session->establish(NULL);
statistics_session_established = true;
cout << "[b10-auth] Statistics session channel established." << endl;
// XXX: with the current interface to asio_link we have to create
// auth_server before io_service while Session needs io_service.
......@@ -240,7 +240,7 @@ main(int argc, char* argv[]) {
// from auth_server, and create io_service, auth_server, and
// sessions in that order.
auth_server->setXfrinSession(xfrin_session);
auth_server->setStatsSession(stats_session);
auth_server->setStatisticsSession(statistics_session);
auth_server->setConfigSession(config_session);
auth_server->updateConfig(ElementPtr());
......@@ -249,8 +249,8 @@ main(int argc, char* argv[]) {
// set up interval timer
// register function to send statistics with interval
itimer->setupTimer(boost::bind(statisticsTimerCallback, auth_server),
STATS_SEND_INTERVAL_SEC);
cout << "[b10-auth] Interval timer set to send stats." << endl;
STATISTICS_SEND_INTERVAL_SEC);
cout << "[b10-auth] Interval timer to send statistics set." << endl;
cout << "[b10-auth] Server started." << endl;
io_service->run();
......@@ -259,8 +259,8 @@ main(int argc, char* argv[]) {
ret = 1;
}
if (stats_session_established) {
stats_session->disconnect();
if (statistics_session_established) {
statistics_session->disconnect();
}
if (xfrin_session_established) {
......@@ -268,7 +268,7 @@ main(int argc, char* argv[]) {
}
delete itimer;
delete stats_session;
delete statistics_session;
delete xfrin_session;
delete config_session;
delete cc_session;
......
......@@ -23,53 +23,49 @@
#include <iostream>
// TODO: We need a namespace ("auth_server"?) to hold
// AuthSrv and QueryCounters.
// AuthSrv and AuthCounters.
class QueryCountersImpl {
class AuthCountersImpl {
private:
// prohibit copy
QueryCountersImpl(const QueryCountersImpl& source);
QueryCountersImpl& operator=(const QueryCountersImpl& source);
AuthCountersImpl(const AuthCountersImpl& source);
AuthCountersImpl& operator=(const AuthCountersImpl& source);
public:
// References verbose_mode flag in AuthSrvImpl
// TODO: Fix this short term workaround for logging
// after we have logging framework
QueryCountersImpl(const bool& verbose_mode);
~QueryCountersImpl();
void inc(const QueryCounters::QueryCounterType type);
AuthCountersImpl(const bool& verbose_mode);
~AuthCountersImpl();
void inc(const AuthCounters::QueryType type);
bool submitStatistics() const;
void setStatsSession(isc::cc::AbstractSession* stats_session);
void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
// Currently for testing purpose only
const std::vector<uint64_t>& getCounters() const;
uint64_t getCounter(const AuthCounters::QueryType type) const;
private:
std::vector<uint64_t> counters_;
isc::cc::AbstractSession* stats_session_;
isc::cc::AbstractSession* statistics_session_;
const bool& verbose_mode_;
};
QueryCountersImpl::QueryCountersImpl(const bool& verbose_mode) :
AuthCountersImpl::AuthCountersImpl(const bool& verbose_mode) :
// initialize counter
// size: QueryCounters::COUNTER_TYPES, initial value: 0
counters_(QueryCounters::COUNTER_TYPES, 0),
stats_session_(NULL),
// size: AuthCounters::COUNTER_TYPES, initial value: 0
counters_(AuthCounters::COUNTER_TYPES, 0),
statistics_session_(NULL),
verbose_mode_(verbose_mode)
{}
QueryCountersImpl::~QueryCountersImpl()
AuthCountersImpl::~AuthCountersImpl()
{}
void
QueryCountersImpl::inc(const QueryCounters::QueryCounterType type) {
try {
++counters_.at(type);
} catch (std::out_of_range) {
isc_throw(isc::InvalidParameter, "Unknown counter type: " << type);
}
AuthCountersImpl::inc(const AuthCounters::QueryType type) {
++counters_.at(type);
}
bool
QueryCountersImpl::submitStatistics() const {
if (stats_session_ == NULL) {
AuthCountersImpl::submitStatistics() const {
if (statistics_session_ == NULL) {
if (verbose_mode_) {
std::cerr << "[b10-auth] "
<< "session interface for statistics"
......@@ -77,24 +73,25 @@ QueryCountersImpl::submitStatistics() const {
}
return (false);
}
std::stringstream strstats;
strstats << "{\"command\": [\"set\","
<< "{ \"stats_data\": "
<< "{ \"auth.queries.udp\": "
<< counters_.at(QueryCounters::COUNTER_UDP)
<< ", \"auth.queries.tcp\": "
<< counters_.at(QueryCounters::COUNTER_TCP)
<< " }"
<< "}"
<< "]}";
isc::data::ConstElementPtr set_stats =
isc::data::Element::fromJSON(strstats);
std::stringstream statistics_string;
statistics_string << "{\"command\": [\"set\","
<< "{ \"stats_data\": "
<< "{ \"auth.queries.udp\": "
<< counters_.at(AuthCounters::COUNTER_UDP)
<< ", \"auth.queries.tcp\": "
<< counters_.at(AuthCounters::COUNTER_TCP)
<< " }"
<< "}"
<< "]}";
isc::data::ConstElementPtr statistics_element =
isc::data::Element::fromJSON(statistics_string);
try {
// group_{send,recv}msg() can throw an exception when encountering
// an error, and group_recvmsg() will throw an exception on timeout.
// We don't want to kill the main server just due to this, so we
// handle them here.
const int seq = stats_session_->group_sendmsg(set_stats, "Stats");
const int seq =
statistics_session_->group_sendmsg(statistics_element, "Stats");
isc::data::ConstElementPtr env, answer;
if (verbose_mode_) {
std::cerr << "[b10-auth] "
......@@ -102,7 +99,7 @@ QueryCountersImpl::submitStatistics() const {
}
// TODO: parse and check response from statistics module
// currently it just returns empty message
stats_session_->group_recvmsg(env, answer, false, seq);
statistics_session_->group_recvmsg(env, answer, false, seq);
} catch (const isc::cc::SessionError& ex) {
if (verbose_mode_) {
std::cerr << "[b10-auth] "
......@@ -122,40 +119,46 @@ QueryCountersImpl::submitStatistics() const {
}
void
QueryCountersImpl::setStatsSession(isc::cc::AbstractSession* stats_session) {
stats_session_ = stats_session;
AuthCountersImpl::setStatisticsSession
(isc::cc::AbstractSession* statistics_session)
{
statistics_session_ = statistics_session;
}
// Currently for testing purpose only
const std::vector<uint64_t>&
QueryCountersImpl::getCounters() const {
return (counters_);
uint64_t
AuthCountersImpl::getCounter
(const AuthCounters::QueryType type) const
{
return (counters_.at(type));
}
QueryCounters::QueryCounters(const bool& verbose_mode) :
impl_(new QueryCountersImpl(verbose_mode))
AuthCounters::AuthCounters(const bool& verbose_mode) :
impl_(new AuthCountersImpl(verbose_mode))
{}
QueryCounters::~QueryCounters() {
AuthCounters::~AuthCounters() {
delete impl_;
}
void
QueryCounters::inc(const QueryCounters::QueryCounterType type) {
AuthCounters::inc(const AuthCounters::QueryType type) {
impl_->inc(type);
}
bool
QueryCounters::submitStatistics() const {
AuthCounters::submitStatistics() const {
return (impl_->submitStatistics());
}
void
QueryCounters::setStatsSession(isc::cc::AbstractSession* stats_session) {
impl_->setStatsSession(stats_session);
AuthCounters::setStatisticsSession
(isc::cc::AbstractSession* statistics_session)
{
impl_->setStatisticsSession(statistics_session);
}
const std::vector<uint64_t>&
QueryCounters::getCounters() const {
return (impl_->getCounters());
uint64_t
AuthCounters::getCounter(const AuthCounters::QueryType type) const {
return (impl_->getCounter(type));
}
......@@ -19,39 +19,43 @@
#include <cc/session.h>
class QueryCountersImpl;
class AuthCountersImpl;
/// \brief Set of query counters.
///
/// \c QueryCounters is set of query counters class. It holds query counters
/// and provides an interface to increment the counter of specified
/// type (e.g. UDP query, TCP query).
/// \c AuthCounters is set of query counters class. It holds query counters
/// and provides an interface to increment the counter of specified type
/// (e.g. UDP query, TCP query).
///
/// This class also provides a function to send statistics information to
/// statistics module.
///
/// This class is designed to be a part of \c AuthSrv.
/// Call \c setStatsSession() to set a session to communicate with statistics
/// module like Xfrin session.
/// Call \c setStatisticsSession() to set a session to communicate with
/// statistics module like Xfrin session.
/// Call \c inc() to increment a counter for specific type of query in
/// the query processing function. use \c enum \c QueryCounterType to specify
/// the query processing function. use \c enum \c QueryType to specify
/// the type of query.
/// Call \c submitStatistics() to submit statistics information to statistics
/// module with stats_session, periodically or at a time the command
/// module with statistics_session, periodically or at a time the command
/// \c sendstats is received.
///
/// We may eventually want to change the structure to hold values that is
/// not counters (such as concurrent TCP connections), or seperate generic
/// part to src/lib to share with the other modules.
///
/// This class uses pimpl idiom and hides detailed implementation.
/// This class is constructed on startup of the server, so
/// construction overhead of this approach should be acceptable.
///
/// \todo Hold counters for each query types (Notify, Axfr, Ixfr, Normal)
/// \todo Consider overhead of \c QueryCounters::inc()
class QueryCounters {
/// \todo Consider overhead of \c AuthCounters::inc()
class AuthCounters {
private:
QueryCountersImpl* impl_;
AuthCountersImpl* impl_;
public:
// Enum for the type of counter
enum QueryCounterType {
enum QueryType {
COUNTER_UDP = 0, ///< COUNTER_UDP: counter for UDP queries
COUNTER_TCP = 1, ///< COUNTER_TCP: counter for TCP queries
COUNTER_TYPES = 2 ///< The number of defined counters
......@@ -66,22 +70,22 @@ public:
/// \todo Fix this short term workaround for logging
/// after we have logging framework.
///
QueryCounters(const bool& verbose_mode);
AuthCounters(const bool& verbose_mode);
/// The destructor.
///
/// This method never throws an exception.
///
~QueryCounters();
~AuthCounters();
/// \brief Increment the counter specified by the parameter.
///
/// \param type Type of a counter to increment.
///
/// \throw isc::InvalidParameter the type is unknown.
/// \throw std::out_of_range \a type is unknown.
///
/// usage: counter.inc(QueryCounterType::COUNTER_UDP);
/// usage: counter.inc(QueryType::COUNTER_UDP);
///
void inc(const QueryCounterType type);
void inc(const QueryType type);
/// \brief Submit statistics counters to statistics module.
///
......@@ -90,7 +94,7 @@ public:
/// by the command 'sendstats'.
///
/// Note: Set the session to communicate with statistics module
/// by \c setStatsSession() before calling \c submitStatistics().
/// by \c setStatisticsSession() before calling \c submitStatistics().
///
/// This method is mostly exception free (error conditions are
/// represented via the return value). But it may still throw
......@@ -98,7 +102,8 @@ public:
///
/// \return true on success, false on error.
///
/// \todo Do not block query handling while submitting statistics data.
/// \todo Do not block message handling in auth_srv while submitting
/// statistics data.
///
bool submitStatistics() const;
......@@ -116,21 +121,22 @@ public:
/// disconnecting the session and destroying the object when the server
/// is shutdown.
///
/// \param stats_session A pointer to the session
/// \param statistics_session A pointer to the session
///
void setStatsSession(isc::cc::AbstractSession* stats_session);
void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
/// \brief Get a reference to the counters in the QueryCounters.
/// \brief Get a value of a counter in the AuthCounters.
///
/// This function returns a refetence to the counters.
/// This function returns a value of the counter specified by \a type.
/// This method never throws an exception.
///
/// Note: Currently this function is for testing purpose only.