[2157] some optimization of statistics in query processing

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<RRClass, boost::shared_ptr<ConfigurableClientList> >
         client_lists_;
 
+    /// Query / Response attributes
+    QRAttributes stats_attrs_;
+
     boost::shared_ptr<ConfigurableClientList> 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);
 }
 

src/bin/auth/statistics.cc

@@ -22,9 +22,6 @@
 #include <cc/data.h>
 #include <cc/session.h>
 
-#include <statistics/counter.h>
-#include <statistics/counter_dict.h>
-
 #include <algorithm>
 #include <cctype>
 #include <cassert>
@@ -32,8 +29,6 @@
 #include <sstream>
 #include <iostream>
 
-#include <boost/noncopyable.hpp>
-
 #include <unistd.h>
 #include <sys/types.h>
 #include <sys/socket.h>
@@ -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

src/bin/auth/statistics.h

@@ -20,17 +20,19 @@
 
 #include <dns/message.h>
 
+#include <statistics/counter.h>
+#include <statistics/counter_dict.h>
+
+#include <boost/noncopyable.hpp>
+
 #include <string>
 
 #include <stdint.h>
-#include <boost/scoped_ptr.hpp>
 
 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<CountersImpl> 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.
     ///        {