From: Yoshitaka Aharen Date: Tue, 2 Oct 2012 09:43:48 +0000 (+0900) Subject: [2157] some optimization of statistics in query processing X-Git-Tag: bind10-1.1.0beta1-release~85^2~5^2~145 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=415e9b6a2c8544882c71e3cb91d47bb768e41a9f;p=thirdparty%2Fkea.git [2157] some optimization of statistics in query processing --- diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc index e21d767335..dd0047a25b 100644 --- a/src/bin/auth/auth_srv.cc +++ b/src/bin/auth/auth_srv.cc @@ -83,6 +83,7 @@ using namespace isc::asiolink; using namespace isc::asiodns; using namespace isc::server_common::portconfig; using isc::auth::statistics::Counters; +using isc::auth::statistics::QRAttributes; namespace { // A helper class for cleaning up message renderer. @@ -270,6 +271,9 @@ public: std::map > client_lists_; + /// Query / Response attributes + QRAttributes stats_attrs_; + boost::shared_ptr getClientList(const RRClass& rrclass) { @@ -299,16 +303,10 @@ public: /// /// \param server The DNSServer as passed to processMessage() /// \param message The response as constructed by processMessage() - /// \param stats_attrs Query/response attributes for statistics which is - /// not in \p messsage. - /// Note: This parameter is modified inside this method - /// to store whether the answer has been sent and - /// the response is truncated. /// \param done If true, it indicates there is a response. /// this value will be passed to server->resume(bool) void resumeServer(isc::asiodns::DNSServer* server, isc::dns::Message& message, - statistics::QRAttributes& stats_attrs, const bool done); private: @@ -494,11 +492,11 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message, OutputBuffer& buffer, DNSServer* server) { InputBuffer request_buffer(io_message.getData(), io_message.getDataSize()); - statistics::QRAttributes stats_attrs; // statistics: check transport carrying the message (IP, transport) - stats_attrs.setQueryIPVersion(io_message.getRemoteEndpoint().getFamily()); - stats_attrs.setQueryTransportProtocol( + impl_->stats_attrs_.setQueryIPVersion( + io_message.getRemoteEndpoint().getFamily()); + impl_->stats_attrs_.setQueryTransportProtocol( io_message.getRemoteEndpoint().getProtocol()); // First, check the header part. If we fail even for the base header, @@ -509,13 +507,13 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message, // Ignore all responses. if (message.getHeaderFlag(Message::HEADERFLAG_QR)) { LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_RESPONSE_RECEIVED); - impl_->resumeServer(server, message, stats_attrs, false); + impl_->resumeServer(server, message, false); return; } } catch (const Exception& ex) { LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_HEADER_PARSE_FAIL) .arg(ex.what()); - impl_->resumeServer(server, message, stats_attrs, false); + impl_->resumeServer(server, message, false); return; } @@ -526,13 +524,13 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message, LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_PACKET_PROTOCOL_ERROR) .arg(error.getRcode().toText()).arg(error.what()); makeErrorMessage(impl_->renderer_, message, buffer, error.getRcode()); - impl_->resumeServer(server, message, stats_attrs, true); + impl_->resumeServer(server, message, true); return; } catch (const Exception& ex) { LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_PACKET_PARSE_ERROR) .arg(ex.what()); makeErrorMessage(impl_->renderer_, message, buffer, Rcode::SERVFAIL()); - impl_->resumeServer(server, message, stats_attrs, true); + impl_->resumeServer(server, message, true); return; } // other exceptions will be handled at a higher layer. @@ -557,14 +555,14 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message, io_message.getDataSize()); // statistics: check TSIG attributes // SIG(0) is currently not implemented in Auth - stats_attrs.setQuerySig(true, false, - tsig_error != TSIGError::NOERROR()); + impl_->stats_attrs_.setQuerySig(true, false, + tsig_error != TSIGError::NOERROR()); } if (tsig_error != TSIGError::NOERROR()) { makeErrorMessage(impl_->renderer_, message, buffer, tsig_error.toRcode(), tsig_context); - impl_->resumeServer(server, message, stats_attrs, true); + impl_->resumeServer(server, message, true); return; } @@ -576,14 +574,15 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message, { ConstEDNSPtr edns = message.getEDNS(); if (edns != NULL) { - stats_attrs.setQueryEDNS(true, edns->getVersion() != 0); - stats_attrs.setQueryDO(edns->getDNSSECAwareness()); + impl_->stats_attrs_.setQueryEDNS(true, + edns->getVersion() != 0); + impl_->stats_attrs_.setQueryDO(edns->getDNSSECAwareness()); } } // statistics: check OpCode // note: This can only be reliable after TSIG check succeeds. - stats_attrs.setQueryOpCode(opcode.getCode()); + impl_->stats_attrs_.setQueryOpCode(opcode.getCode()); if (opcode == Opcode::NOTIFY()) { send_answer = impl_->processNotify(io_message, message, buffer, @@ -625,7 +624,7 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message, LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_RESPONSE_FAILURE_UNKNOWN); makeErrorMessage(impl_->renderer_, message, buffer, Rcode::SERVFAIL()); } - impl_->resumeServer(server, message, stats_attrs, send_answer); + impl_->resumeServer(server, message, send_answer); } bool @@ -819,14 +818,14 @@ AuthSrvImpl::processUpdate(const IOMessage& io_message) { void AuthSrvImpl::resumeServer(DNSServer* server, Message& message, - statistics::QRAttributes& stats_attrs, const bool done) { if (done) { - stats_attrs.answerWasSent(); + stats_attrs_.answerWasSent(); // isTruncated from MessageRenderer - stats_attrs.setResponseTruncated(renderer_.isTruncated()); + stats_attrs_.setResponseTruncated(renderer_.isTruncated()); } - counters_.inc(stats_attrs, message); + counters_.inc(stats_attrs_, message); + stats_attrs_.reset(); server->resume(done); } diff --git a/src/bin/auth/statistics.cc b/src/bin/auth/statistics.cc index 5831eaf294..89c9d08cea 100644 --- a/src/bin/auth/statistics.cc +++ b/src/bin/auth/statistics.cc @@ -22,9 +22,6 @@ #include #include -#include -#include - #include #include #include @@ -32,8 +29,6 @@ #include #include -#include - #include #include #include @@ -66,24 +61,7 @@ namespace isc { namespace auth { namespace statistics { -class CountersImpl : boost::noncopyable { -public: - CountersImpl(); - ~CountersImpl(); - void inc(const QRAttributes& qrattrs, const Message& response); - Counters::ItemTreeType get() const; -private: - // counter for query/response - Counter server_qr_counter_; - // counter for socket - Counter socket_counter_; - // set of counters for zones - CounterDictionary zone_qr_counters_; - void incRequest(const QRAttributes& qrattrs, const Message& response); - void incResponse(const QRAttributes& qrattrs, const Message& response); -}; - -CountersImpl::CountersImpl() : +Counters::Counters() : // size of server_qr_counter_, zone_qr_counters_: QR_COUNTER_TYPES // size of server_socket_counter_: SOCKET_COUNTER_TYPES server_qr_counter_(QR_COUNTER_TYPES), @@ -91,13 +69,11 @@ CountersImpl::CountersImpl() : zone_qr_counters_(QR_COUNTER_TYPES) {} -CountersImpl::~CountersImpl() +Counters::~Counters() {} void -CountersImpl::incRequest(const QRAttributes& qrattrs, - const Message& response) -{ +Counters::incRequest(const QRAttributes& qrattrs, const Message& response) { // protocols carrying request if (qrattrs.req_ip_version_ == AF_INET) { server_qr_counter_.inc(QR_REQUEST_IPV4); @@ -167,9 +143,7 @@ CountersImpl::incRequest(const QRAttributes& qrattrs, } void -CountersImpl::incResponse(const QRAttributes& qrattrs, - const Message& response) -{ +Counters::incResponse(const QRAttributes& qrattrs, const Message& response) { // responded server_qr_counter_.inc(QR_RESPONSE); @@ -237,7 +211,7 @@ CountersImpl::incResponse(const QRAttributes& qrattrs, } void -CountersImpl::inc(const QRAttributes& qrattrs, const Message& response) { +Counters::inc(const QRAttributes& qrattrs, const Message& response) { // increment request counters incRequest(qrattrs, response); @@ -248,7 +222,7 @@ CountersImpl::inc(const QRAttributes& qrattrs, const Message& response) { } Counters::ItemTreeType -CountersImpl::get() const { +Counters::get() const { using namespace isc::data; Counters::ItemTreeType item_tree = Element::createMap(); @@ -264,21 +238,6 @@ CountersImpl::get() const { return (item_tree); } -Counters::Counters() : impl_(new CountersImpl()) -{} - -Counters::~Counters() {} - -void -Counters::inc(const QRAttributes& qrattrs, const Message& response) { - impl_->inc(qrattrs, response); -} - -Counters::ItemTreeType -Counters::get() const { - return (impl_->get()); -} - } // namespace statistics } // namespace auth } // namespace isc diff --git a/src/bin/auth/statistics.h b/src/bin/auth/statistics.h index ac11572cce..7de481366c 100644 --- a/src/bin/auth/statistics.h +++ b/src/bin/auth/statistics.h @@ -20,17 +20,19 @@ #include +#include +#include + +#include + #include #include -#include namespace isc { namespace auth { namespace statistics { -class CountersImpl; - class QRAttributes { /// \brief Query/Response attributes for statistics. /// @@ -38,8 +40,8 @@ class QRAttributes { /// for statistics data collection. /// /// This class does not have getter methods since it exposes private members -/// to \c CountersImpl directly. -friend class CountersImpl; +/// to \c Counters directly. +friend class Counters; private: // request attributes int req_ip_version_; // IP version @@ -200,9 +202,18 @@ QRAttributes::reset() { /// /// \todo Hold counters for each query types (Notify, Axfr, Ixfr, Normal) /// \todo Consider overhead of \c Counters::inc() -class Counters { +class Counters : boost::noncopyable { private: - boost::scoped_ptr impl_; + // counter for query/response + isc::statistics::Counter server_qr_counter_; + // counter for socket + isc::statistics::Counter socket_counter_; + // set of counters for zones + isc::statistics::CounterDictionary zone_qr_counters_; + void incRequest(const QRAttributes& qrattrs, + const isc::dns::Message& response); + void incResponse(const QRAttributes& qrattrs, + const isc::dns::Message& response); public: /// \brief A type of statistics item tree in isc::data::MapElement. /// {