Fix TSAN complaints: max stacksize and response stats size counters.

Start using new generic histogram code and while there, fix a bug
concerning the rcode stats (which was not passed in by the caller).

diff --git a/.not-formatted b/.not-formatted
index 452a9033052bea1eae8875ef98e72bf1ef24771e..9778db3d7cb6515b7c54ce56e34ef43f36b668a1 100644 (file)
--- a/.not-formatted
+++ b/.not-formatted
@@ -275,8 +275,6 @@
 ./pdns/resolver.cc
 ./pdns/resolver.hh
 ./pdns/responsestats-auth.cc
-./pdns/responsestats.cc
-./pdns/responsestats.hh
 ./pdns/rfc2136handler.cc
 ./pdns/root-addresses.hh
 ./pdns/root-dnssec.hh

diff --git a/pdns/histogram.hh b/pdns/histogram.hh
index 5abb3c39d54dbd738ab943a2f3feff4f81a111b3..a315b8813f36e8393522ea059938058aa65893f4 100644 (file)
--- a/pdns/histogram.hh
+++ b/pdns/histogram.hh
@@ -23,6 +23,7 @@
 
 #include <algorithm>
 #include <atomic>
+#include <string>
 #include <vector>
 
 namespace pdns {

diff --git a/pdns/mtasker.cc b/pdns/mtasker.cc
index a2c3a2fba2679a692aeefa4dee242e521c2b882e..96bf9e0125242e44238e44f4200ed50d49bc9685 100644 (file)
--- a/pdns/mtasker.cc
+++ b/pdns/mtasker.cc
@@ -399,7 +399,7 @@ template<class Key, class Val>int MTasker<Key,Val>::getTid() const
 }
 
 //! Returns the maximum stack usage so far of this MThread
-template<class Key, class Val>unsigned int MTasker<Key,Val>::getMaxStackUsage()
+template<class Key, class Val>uint64_t MTasker<Key,Val>::getMaxStackUsage()
 {
   return d_threads[d_tid].startOfStack - d_threads[d_tid].highestStackSeen;
 }

diff --git a/pdns/mtasker.hh b/pdns/mtasker.hh
index 607ca57745877455caea7061040ba44c7ef8e5e9..fa1ede41274b8770e45bf0b03861236e9263faa6 100644 (file)
--- a/pdns/mtasker.hh
+++ b/pdns/mtasker.hh
@@ -136,7 +136,7 @@ public:
   bool noProcesses() const;
   unsigned int numProcesses() const;
   int getTid() const;
-  unsigned int getMaxStackUsage();
+  uint64_t getMaxStackUsage();
   unsigned int getUsec();
 
 private:

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 5ff39d9fc8ecac7d1c1ae77c9382e55f0762bdb5..d33d3a6c25fc5f09c78d42c473458064fa537b36 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -2096,7 +2096,7 @@ static void startDoResolve(void *p)
       pw.commit();
     }
 
-    g_rs.submitResponse(dc->d_mdp.d_qtype, packet.size(), !dc->d_tcp);
+    g_rs.submitResponse(dc->d_mdp.d_qtype, packet.size(), pw.getHeader()->rcode, !dc->d_tcp);
     updateResponseStats(res, dc->d_source, packet.size(), &dc->d_mdp.d_qname, dc->d_mdp.d_qtype);
 #ifdef NOD_ENABLED
     bool nod = false;
@@ -2272,7 +2272,7 @@ static void startDoResolve(void *p)
 
   runTaskOnce(g_logCommonErrors);
 
-  g_stats.maxMThreadStackUsage = max(MT->getMaxStackUsage(), g_stats.maxMThreadStackUsage);
+  g_stats.maxMThreadStackUsage = max(MT->getMaxStackUsage(), g_stats.maxMThreadStackUsage.load());
 }
 
 static void makeControlChannelSocket(int processNum=-1)

diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 08ca90b8a91e958b3bf82d98e93c1d2c9e1f50b5..dca663d9950e96f070b03654e6e28dfdec2f105f 100644 (file)
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -1140,7 +1140,6 @@ static void registerAllStats1()
   addGetStat("over-capacity-drops", &g_stats.overCapacityDrops);
   addGetStat("policy-drops", &g_stats.policyDrops);
   addGetStat("no-packet-error", &g_stats.noPacketError);
-  addGetStat("dlg-only-drops", &SyncRes::s_nodelegated);
   addGetStat("ignored-packets", &g_stats.ignoredCount);
   addGetStat("empty-queries", &g_stats.emptyQueriesCount);
   addGetStat("max-mthread-stack", &g_stats.maxMThreadStackUsage);

diff --git a/pdns/responsestats.cc b/pdns/responsestats.cc
index fefa35c69e5a9f0d8767b8f3ce156fa7f017f413..bd5e34ca91b744880ce00a163479daf7253d098d 100644 (file)
--- a/pdns/responsestats.cc
+++ b/pdns/responsestats.cc
@@ -8,54 +8,56 @@
 
 #include "dnsparser.hh"
 
-ResponseStats::ResponseStats() :   d_qtypecounters(new std::atomic<unsigned long>[65536]), d_rcodecounters(new std::atomic<unsigned long>[256])
+static auto sizeBounds()
 {
-  d_sizecounters.push_back(make_pair(20,0));
-  d_sizecounters.push_back(make_pair(40,0));
-  d_sizecounters.push_back(make_pair(60,0));
-  d_sizecounters.push_back(make_pair(80,0));
-  d_sizecounters.push_back(make_pair(100,0));
-  d_sizecounters.push_back(make_pair(150,0));
-  for(int n=200; n < 65000 ; n+=200)
-    d_sizecounters.push_back(make_pair(n,0));
-  d_sizecounters.push_back(make_pair(std::numeric_limits<uint16_t>::max(),0));
-  for(unsigned int n =0 ; n < 65535; ++n)
+  std::vector<uint64_t> bounds;
+
+  bounds.push_back(20);
+  bounds.push_back(40);
+  bounds.push_back(60);
+  bounds.push_back(80);
+  bounds.push_back(100);
+  bounds.push_back(150);
+  for (uint64_t n = 200; n < 65000; n += 200) {
+    bounds.push_back(n);
+  }
+  return bounds;
+}
+
+ResponseStats::ResponseStats() :
+  d_sizecounters("SizeCounters", sizeBounds())
+{
+  for (unsigned int n = 0; n < 65535; ++n) {
     d_qtypecounters[n] = 0;
-  for(unsigned int n =0 ; n < 256; ++n)
+  }
+  for (unsigned int n = 0; n < 256; ++n) {
     d_rcodecounters[n] = 0;
+  }
 }
 
 ResponseStats g_rs;
 
-static bool pcomp(const pair<uint16_t, uint64_t>&a , const pair<uint16_t, uint64_t>&b)
-{
-  return a.first < b.first;
-}
-
-void ResponseStats::submitResponse(uint16_t qtype,uint16_t respsize, uint8_t rcode, bool udpOrTCP)
+void ResponseStats::submitResponse(uint16_t qtype, uint16_t respsize, uint8_t rcode, bool udpOrTCP)
 {
-    d_rcodecounters[rcode]++;
-    submitResponse(qtype, respsize, udpOrTCP);
+  d_rcodecounters[rcode]++;
+  submitResponse(qtype, respsize, udpOrTCP);
 }
 
-void ResponseStats::submitResponse(uint16_t qtype,uint16_t respsize, bool udpOrTCP)
+void ResponseStats::submitResponse(uint16_t qtype, uint16_t respsize, bool udpOrTCP)
 {
   d_qtypecounters[qtype]++;
-  pair<uint16_t, uint64_t> s(respsize, 0);
-  sizecounters_t::iterator iter = std::upper_bound(d_sizecounters.begin(), d_sizecounters.end(), s, pcomp);
-  if(iter!= d_sizecounters.begin())
-    --iter;
-  iter->second++;
+  d_sizecounters(respsize);
 }
 
 map<uint16_t, uint64_t> ResponseStats::getQTypeResponseCounts()
 {
   map<uint16_t, uint64_t> ret;
   uint64_t count;
-  for(unsigned int i = 0 ; i < 65535 ; ++i) {
-    count= d_qtypecounters[i];
-    if(count)
-      ret[i]=count;
+  for (unsigned int i = 0; i < 65535; ++i) {
+    count = d_qtypecounters[i];
+    if (count) {
+      ret[i] = count;
+    }
   }
   return ret;
 }
@@ -63,8 +65,10 @@ map<uint16_t, uint64_t> ResponseStats::getQTypeResponseCounts()
 map<uint16_t, uint64_t> ResponseStats::getSizeResponseCounts()
 {
   map<uint16_t, uint64_t> ret;
-  for(const auto & sizecounter : d_sizecounters) {
-    ret[sizecounter.first]=sizecounter.second;
+  for (const auto& sizecounter : d_sizecounters.getRawData()) {
+    if (sizecounter.d_count) {
+      ret[sizecounter.d_boundary] = sizecounter.d_count;
+    }
   }
   return ret;
 }
@@ -73,22 +77,22 @@ map<uint8_t, uint64_t> ResponseStats::getRCodeResponseCounts()
 {
   map<uint8_t, uint64_t> ret;
   uint64_t count;
-  for(unsigned int i = 0 ; i < 256 ; ++i) {
-    count= d_rcodecounters[i];
-    if(count)
-      ret[i]=count;
+  for (unsigned int i = 0; i < 256; ++i) {
+    count = d_rcodecounters[i];
+    if (count) {
+      ret[i] = count;
+    }
   }
   return ret;
 }
 
 string ResponseStats::getQTypeReport()
 {
-  typedef map<uint16_t, uint64_t> qtypenums_t;
-  qtypenums_t qtypenums = getQTypeResponseCounts();
+  auto qtypenums = getQTypeResponseCounts();
   ostringstream os;
   boost::format fmt("%s\t%d\n");
-  for(const qtypenums_t::value_type& val :  qtypenums) {
-    os << (fmt %DNSRecordContent::NumberToType( val.first) % val.second).str();
+  for (const auto& val : qtypenums) {
+    os << (fmt % DNSRecordContent::NumberToType(val.first) % val.second).str();
   }
   return os.str();
 }

diff --git a/pdns/responsestats.hh b/pdns/responsestats.hh
index d068a1c3d537f8d3ad1992dedb2af4b7afdd7807..259a545c94a3cb041bf5a7200e93cad8bebf2511 100644 (file)
--- a/pdns/responsestats.hh
+++ b/pdns/responsestats.hh
@@ -21,7 +21,8 @@
  */
 #pragma once
 
-#include <boost/scoped_array.hpp>
+#include <array>
+#include "histogram.hh"
 
 #include "dnspacket.hh"
 
@@ -30,7 +31,7 @@ class ResponseStats
 public:
   ResponseStats();
 
-  void submitResponse(DNSPacket &p, bool udpOrTCP, bool last=true);
+  void submitResponse(DNSPacket& p, bool udpOrTCP, bool last = true);
   void submitResponse(uint16_t qtype, uint16_t respsize, bool udpOrTCP);
   void submitResponse(uint16_t qtype, uint16_t respsize, uint8_t rcode, bool udpOrTCP);
   map<uint16_t, uint64_t> getQTypeResponseCounts();
@@ -39,10 +40,9 @@ public:
   string getQTypeReport();
 
 private:
-  boost::scoped_array<std::atomic<unsigned long>> d_qtypecounters;
-  boost::scoped_array<std::atomic<unsigned long>> d_rcodecounters;
-  typedef vector<pair<uint16_t, uint64_t> > sizecounters_t;
-  sizecounters_t d_sizecounters;
+  std::array<std::atomic<uint64_t>, 65535> d_qtypecounters;
+  std::array<std::atomic<uint64_t>, 256> d_rcodecounters;
+  pdns::AtomicHistogram<uint64_t> d_sizecounters;
 };
 
 extern ResponseStats g_rs;

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 1a32e196f847b95cde287a7a5e495717be71f113..68aff8f259910d147a8ec105774230c1eaf69457 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -1033,7 +1033,7 @@ struct RecursorStats
   std::atomic<uint64_t> dnssecAuthenticDataQueries;
   std::atomic<uint64_t> dnssecCheckDisabledQueries;
   std::atomic<uint64_t> variableResponses;
-  unsigned int maxMThreadStackUsage;
+  std::atomic<uint64_t> maxMThreadStackUsage;
   std::atomic<uint64_t> dnssecValidations; // should be the sum of all dnssecResult* stats
   std::map<vState, std::atomic<uint64_t> > dnssecResults;
   std::map<vState, std::atomic<uint64_t> > xdnssecResults;

diff --git a/pdns/tcpreceiver.cc b/pdns/tcpreceiver.cc
index 61dcc2fca45ea70f19d6a609ff15d6d1d4e9871d..1fbc4b2bb195a9b06e9d96dbcedc7acc9581bd82 100644 (file)
--- a/pdns/tcpreceiver.cc
+++ b/pdns/tcpreceiver.cc
@@ -23,6 +23,7 @@
 #include "config.h"
 #endif
 #include <boost/algorithm/string.hpp>
+#include <boost/scoped_array.hpp>
 #include "auth-packetcache.hh"
 #include "utility.hh"
 #include "threadname.hh"