From: bert hubert <bert.hubert@netherlabs.nl>
Date: Tue, 16 Dec 2014 11:53:00 +0000 (+0100)
Subject: implement ComboAddress native statistics ringbuffer based on boost::circular_buffer
X-Git-Tag: rec-3.7.0-rc1~109^2~9
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4eb26e0fdeca34c6fcb77c52a621c3fd8c1c9ae8;p=thirdparty%2Fpdns.git

implement ComboAddress native statistics ringbuffer based on boost::circular_buffer
Can use speedup by moving d_rings to a boost::multi_index_container with char* based lookup.
---

diff --git a/pdns/common_startup.cc b/pdns/common_startup.cc
index a2ab02f9d7..cd584fe916 100644
--- a/pdns/common_startup.cc
+++ b/pdns/common_startup.cc
@@ -270,9 +270,9 @@ void declareStats(void)
   S.declareRing("servfail-queries","Queries that could not be answered due to backend errors");
   S.declareRing("unauth-queries","Queries for domains that we are not authoritative for");
   S.declareRing("logmessages","Log Messages");
-  S.declareRing("remotes","Remote server IP addresses");
-  S.declareRing("remotes-unauth","Remote hosts querying domains for which we are not auth");
-  S.declareRing("remotes-corrupt","Remote hosts sending corrupt packets");
+  S.declareComboRing("remotes","Remote server IP addresses");
+  S.declareComboRing("remotes-unauth","Remote hosts querying domains for which we are not auth");
+  S.declareComboRing("remotes-corrupt","Remote hosts sending corrupt packets");
 }
 
 int isGuarded(char **argv)
@@ -352,7 +352,7 @@ void *qthread(void *number)
        continue;
 
     S.ringAccount("queries", P->qdomain+"/"+P->qtype.getName());
-    S.ringAccount("remotes",P->getRemote());
+    S.ringAccount("remotes",P->d_remote);
     if(logDNSQueries) {
       string remote;
       if(P->hasEDNSSubnet()) 
diff --git a/pdns/nameserver.cc b/pdns/nameserver.cc
index f4c20d60e0..4eaaddc524 100644
--- a/pdns/nameserver.cc
+++ b/pdns/nameserver.cc
@@ -302,7 +302,7 @@ void UDPNameserver::send(DNSPacket *p)
       S.ringAccount("nxdomain-queries",p->qdomain+"/"+p->qtype.getName());
   } else if (p->isEmpty()) {
     S.ringAccount("unauth-queries",p->qdomain+"/"+p->qtype.getName());
-    S.ringAccount("remotes-unauth",p->getRemote());
+    S.ringAccount("remotes-unauth",p->d_remote);
   }
 
   /* Count responses (total/v4/v6) and byte counts */
@@ -466,7 +466,7 @@ DNSPacket *UDPNameserver::receive(DNSPacket *prefilled)
 
   if(packet->parse(mesg, len)<0) {
     S.inc("corrupt-packets");
-    S.ringAccount("remotes-corrupt", packet->getRemote());
+    S.ringAccount("remotes-corrupt", packet->d_remote);
 
     if(!prefilled)
       delete packet;
diff --git a/pdns/statbag.cc b/pdns/statbag.cc
index 1c8f3bfada..7d352167ea 100644
--- a/pdns/statbag.cc
+++ b/pdns/statbag.cc
@@ -1,6 +1,6 @@
 /*
     PowerDNS Versatile Database Driven Nameserver
-    Copyright (C) 2002  PowerDNS.COM BV
+    Copyright (C) 2002 - 2014  PowerDNS.COM BV
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License version 2
@@ -28,6 +28,7 @@
 #include <algorithm>
 #include "arguments.hh"
 #include "lock.hh"
+#include "iputils.hh"
 #include <boost/foreach.hpp>
 
 #include "namespaces.hh"
@@ -160,75 +161,59 @@ StatBag::~StatBag()
   
 }
 
-StatRing::StatRing(unsigned int size)
+template<typename T, typename Comp>
+StatRing<T,Comp>::StatRing(unsigned int size)
 {
-  d_size=size;
-  d_items.resize(d_size);
-  d_lock=0;
-  d_pos=0;
-  d_lock=new pthread_mutex_t;
-  pthread_mutex_init(d_lock, 0);
+  d_items.set_capacity(size);
+  pthread_mutex_init(&d_lock, 0);
 }
 
-void StatRing::resize(unsigned int newsize)
+template<typename T, typename Comp>
+void StatRing<T,Comp>::account(const T& t)
 {
-  if(d_size==newsize)
-    return;
-  Lock l(d_lock);
-
-  // this is the hard part, shrink
-  if(newsize<d_size) {
-    unsigned int startpos=0;
-    if (d_pos>newsize)
-      startpos=d_pos-newsize;
-
-    vector<string>newring;
-    for(unsigned int i=startpos;i<d_pos;++i) {
-      newring.push_back(d_items[i%d_size]);
-    }
-
-    d_items=newring;
-    d_size=newring.size();
-    d_pos=min(d_pos,newsize);
-  }
+  Lock l(&d_lock);
+  d_items.push_back(t);
+}
 
-  if(newsize>d_size) {
-    d_size=newsize;
-    d_items.resize(d_size);
-  }
+template<typename T, typename Comp>
+unsigned int StatRing<T,Comp>::getSize()
+{
+  Lock l(&d_lock);
+  return d_items.capacity();
 }
 
-StatRing::~StatRing()
+template<typename T, typename Comp>
+void StatRing<T,Comp>::resize(unsigned int newsize)
 {
-  // do not clean up d_lock, it is shared
+  Lock l(&d_lock);
+  d_items.set_capacity(newsize);
 }
 
-void StatRing::setHelp(const string &str)
+
+template<typename T, typename Comp>
+void StatRing<T,Comp>::setHelp(const string &str)
 {
   d_help=str;
 }
 
-string StatRing::getHelp()
+template<typename T, typename Comp>
+string StatRing<T,Comp>::getHelp()
 {
   return d_help;
 }
 
-static bool popisort(const pair<string,int> &a, const pair<string,int> &b)
-{
-  return (a.second > b.second);
-}
 
-vector<pair<string,unsigned int> >StatRing::get() const
+template<typename T, typename Comp>
+vector<pair<T, unsigned int> >StatRing<T,Comp>::get() const
 {
-  Lock l(d_lock);
-  map<string,unsigned int> res;
-  for(vector<string>::const_iterator i=d_items.begin();i!=d_items.end();++i) {
-    if(!i->empty())
-      res[*i]++;
+  Lock l(&d_lock);
+  map<T,unsigned int, Comp> res;
+  for(typename boost::circular_buffer<T>::const_iterator i=d_items.begin();i!=d_items.end();++i) {
+    res[*i]++;
   }
   
-  vector<pair<string,unsigned int> > tmp;
-  for(map<string,unsigned int>::const_iterator i=res.begin();i!=res.end();++i) 
+  vector<pair<T ,unsigned int> > tmp;
+  for(typename map<T, unsigned int>::const_iterator i=res.begin();i!=res.end();++i) 
     tmp.push_back(*i);
 
   sort(tmp.begin(),tmp.end(),popisort);
@@ -238,55 +223,88 @@ vector<pair<string,unsigned int> >StatRing::get() const
 
 void StatBag::declareRing(const string &name, const string &help, unsigned int size)
 {
-  d_rings[name]=StatRing(size);
+  d_rings[name]=StatRing<string>(size);
   d_rings[name].setHelp(help);
 }
 
-vector<pair<string, unsigned int> > StatBag::getRing(const string &name)
+void StatBag::declareComboRing(const string &name, const string &help, unsigned int size)
 {
-  return d_rings[name].get();
+  d_comborings[name]=StatRing<ComboAddress, ComboAddress::addressOnlyLessThan >(size);
+  d_comborings[name].setHelp(help);
 }
 
-void StatRing::reset()
+
+vector<pair<string, unsigned int> > StatBag::getRing(const string &name)
 {
-  Lock l(d_lock);
-  for(vector<string>::iterator i=d_items.begin();i!=d_items.end();++i) {
-    if(!i->empty())
-      *i="";
+  if(d_rings.count(name))
+    return d_rings[name].get();
+  else {
+    typedef pair<ComboAddress, unsigned int> stor_t;
+    vector<stor_t> raw =d_comborings[name].get();
+    vector<pair<string, unsigned int> > ret;
+    BOOST_FOREACH(const stor_t& stor, raw) {
+      ret.push_back(make_pair(stor.first.toString(), stor.second));
+    }
+    return ret;
   }
+    
+}
+
+template<typename T, typename Comp>
+void StatRing<T,Comp>::reset()
+{
+  Lock l(&d_lock);
+  d_items.clear();
 }
 
 void StatBag::resetRing(const string &name)
 {
-  d_rings[name].reset();
+  if(d_rings.count(name))
+    d_rings[name].reset();
+  else
+    d_comborings[name].reset();
 }
 
 void StatBag::resizeRing(const string &name, unsigned int newsize)
 {
-  d_rings[name].resize(newsize);
+  if(d_rings.count(name))
+    d_rings[name].resize(newsize);
+  else
+    d_comborings[name].resize(newsize);
 }
 
 
 unsigned int StatBag::getRingSize(const string &name)
 {
-  return d_rings[name].getSize();
+  if(d_rings.count(name))
+    return d_rings[name].getSize();
+  else
+    return d_comborings[name].getSize();
 }
 
-
 string StatBag::getRingTitle(const string &name)
 {
-  return d_rings[name].getHelp();
+  if(d_rings.count(name))
+    return d_rings[name].getHelp();
+  else 
+    return d_comborings[name].getHelp();
 }
 
 vector<string>StatBag::listRings()
 {
   vector<string> ret;
-  for(map<string,StatRing>::const_iterator i=d_rings.begin();i!=d_rings.end();++i)
+  for(map<string,StatRing<string> >::const_iterator i=d_rings.begin();i!=d_rings.end();++i)
+    ret.push_back(i->first);
+  for(map<string,StatRing<ComboAddress, ComboAddress::addressOnlyLessThan> >::const_iterator i=d_comborings.begin();i!=d_comborings.end();++i)
     ret.push_back(i->first);
+
   return ret;
 }
 
 bool StatBag::ringExists(const string &name)
 {
-  return d_rings.count(name);
+  return d_rings.count(name) || d_comborings.count(name);
 }
+
+template class StatRing<std::string>;
+template class StatRing<ComboAddress, ComboAddress::addressOnlyLessThan >;
diff --git a/pdns/statbag.hh b/pdns/statbag.hh
index 3920c01a10..a175588a03 100644
--- a/pdns/statbag.hh
+++ b/pdns/statbag.hh
@@ -23,36 +23,36 @@
 #define STATBAG_HH
 #include <pthread.h>
 #include <map>
+#include <functional>
 #include <string>
 #include <vector>
 #include "lock.hh"
 #include "namespaces.hh"
+#include "iputils.hh"
+#include <boost/circular_buffer.hpp>
 
+template<typename T, typename Comp=std::less<T> >
 class StatRing
 {
 public:
   StatRing(unsigned int size=10000);
-  ~StatRing();
-  void account(const string &item)
-  {
-    Lock l(d_lock);
-    d_items[d_pos++ % d_size]=item;
-  }
+  void account(const T &item);
 
-  unsigned int getSize()
-  {
-    return d_size;
-  }
+  unsigned int getSize();
   void resize(unsigned int newsize);  
   void reset();
   void setHelp(const string &str);
   string getHelp();
-  vector<pair<string,unsigned int> >get() const;
+
+  vector<pair<T, unsigned int> > get() const;
 private:
-  unsigned int d_size;
-  unsigned int d_pos;
-  vector<string> d_items;
-  pthread_mutex_t *d_lock;
+  static bool popisort(const pair<T,int> &a, const pair<T,int> &b) 
+  {
+    return (a.second > b.second);
+  }
+
+  boost::circular_buffer<T> d_items;
+  mutable pthread_mutex_t d_lock;
   string d_help;
 };
 
@@ -62,7 +62,8 @@ class StatBag
 {
   map<string, AtomicCounter *> d_stats;
   map<string, string> d_keyDescrips;
-  map<string,StatRing>d_rings;
+  map<string,StatRing<string> >d_rings;
+  map<string,StatRing<ComboAddress, ComboAddress::addressOnlyLessThan> >d_comborings;
   typedef boost::function<uint64_t(const std::string&)> func_t;
   typedef map<string, func_t> funcstats_t;
   funcstats_t d_funcstats;
@@ -75,13 +76,27 @@ public:
   void declare(const string &key, const string &descrip, func_t func); //!< Before you can store or access a key, you need to declare it
 
   void declareRing(const string &name, const string &title, unsigned int size=10000);
+  void declareComboRing(const string &name, const string &help, unsigned int size=10000);
   vector<pair<string, unsigned int> >getRing(const string &name);
   string getRingTitle(const string &name);
-  void ringAccount(const string &name, const string &item)
+  void ringAccount(const char* name, const string &item)
   {
-    if(d_doRings)
+    if(d_doRings)  {
+      if(!d_rings.count(name))
+	throw runtime_error("Attempting to account to non-existent ring");
+
       d_rings[name].account(item);
+    }
   }
+  void ringAccount(const char* name, const ComboAddress &item)
+  {
+    if(d_doRings) {
+      if(!d_comborings.count(name))
+	throw runtime_error("Attempting to account to non-existent comboring");
+      d_comborings[name].account(item);
+    }
+  }
+
   void doRings()
   {
     d_doRings=true;
diff --git a/pdns/tcpreceiver.cc b/pdns/tcpreceiver.cc
index 95feba73b3..e6db0a28a2 100644
--- a/pdns/tcpreceiver.cc
+++ b/pdns/tcpreceiver.cc
@@ -183,7 +183,7 @@ void TCPNameserver::sendPacket(shared_ptr<DNSPacket> p, int outsock)
         S.ringAccount("nxdomain-queries",p->qdomain+"/"+p->qtype.getName());
     } else if(p->isEmpty()) {
       S.ringAccount("unauth-queries",p->qdomain+"/"+p->qtype.getName());
-      S.ringAccount("remotes-unauth",p->getRemote());
+      S.ringAccount("remotes-unauth",p->d_remote);
     }
   }