Use correct bound for getting CPU stats. There's an inconsistentcy here:

broadcastAccFunction does not include the handler, but does include the taskThread
This has to be made consistent. For the moment, keep the existing behaviour.

diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 9678bbeaecda05dc59c08a870cf96d9dc65727a3..9523d738b94a2dd8b08ac68a1cd740bbb9b8e5f7 100644 (file)
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -1118,7 +1118,9 @@ static StatsMap toCPUStatsMap(const string& name)
 {
   const string pbasename = getPrometheusName(name);
   StatsMap entries;
-  for (unsigned int n = 0; n < RecThreadInfo::numRecursorThreads(); ++n) {
+  // Only distr and worker threads, I think we should revisit this as we now not only have the handler thread but also
+  // taskThread(s).
+  for (unsigned int n = 0; n < RecThreadInfo::numDistributors() + RecThreadInfo::numWorkers(); ++n) {
     uint64_t tm = doGetThreadCPUMsec(n);
     std::string pname = pbasename + "{thread=\"" + std::to_string(n) + "\"}";
     entries.emplace(name + "-thread-" + std::to_string(n), StatsMapEntry{pname, std::to_string(tm)});

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 4a98bbc0dee9c67a4beabba76b4c006438db773a..aaad2df72e724245578aa9c0ffa50317ec09490f 100644 (file)
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -174,6 +174,8 @@ static void setCPUMap(const std::map<unsigned int, std::set<int>>& cpusMap, unsi
   }
 }
 
+static void recursorThread();
+
 void RecThreadInfo::start(unsigned int id, const string& name, const std::map<unsigned int, std::set<int>>& cpusMap)
 {
   thread = std::thread([id, name] {
@@ -1096,10 +1098,11 @@ static vector<pair<DNSName, uint16_t>>& operator+=(vector<pair<DNSName, uint16_t
   return a;
 }
 
-/*
-  This function should only be called by the handler to gather metrics, wipe the cache,
-  reload the Lua script (not the Lua config) or change the current trace regex,
-  and by the SNMP thread to gather metrics. */
+// This function should only be called by the handler to gather
+// metrics, wipe the cache, reload the Lua script (not the Lua config)
+// or change the current trace regex, and by the SNMP thread to gather
+// metrics.
+// Note that this currently skips the handler, but includes the taskThread(s).
 template <class T>
 T broadcastAccFunction(const boost::function<T*()>& func)
 {
@@ -1934,7 +1937,7 @@ static void houseKeeping(void*)
   }
 }
 
-void* recursorThread()
+static void recursorThread()
 {
   try {
     auto& threadInfo = RecThreadInfo::self();
@@ -1952,7 +1955,6 @@ void* recursorThread()
           threadInfo.setExitCode(EXIT_FAILURE);
           RecursorControlChannel::stop = 1;
           g_log << Logger::Critical << "Priming cache failed, stopping" << endl;
-          return nullptr;
         }
         g_log << Logger::Debug << "Done priming cache with root hints" << endl;
       }
@@ -2139,19 +2141,15 @@ void* recursorThread()
         }
       }
     }
-    return nullptr;
   }
   catch (PDNSException& ae) {
     g_log << Logger::Error << "Exception: " << ae.reason << endl;
-    return nullptr;
   }
   catch (std::exception& e) {
     g_log << Logger::Error << "STL Exception: " << e.what() << endl;
-    return nullptr;
   }
   catch (...) {
     g_log << Logger::Error << "any other exception in main: " << endl;
-    return nullptr;
   }
 }
 

diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh
index 650df429b58566e58f81d8e531d0f90fb140b6c2..d21c63e5835b6e180d7d1d3c279951196ff9f991 100644 (file)
--- a/pdns/recursordist/rec-main.hh
+++ b/pdns/recursordist/rec-main.hh
@@ -290,8 +290,6 @@ static bool sendResponseOverTCP(const std::unique_ptr<DNSComboWriter>& dc, const
   return hadError;
 }
 
-void* recursorThread();
-
 // For communicating with our threads effectively readonly after
 // startup.
 // First we have the handler thread, t_id == 0 (some other helper