Merge pull request #11879 from fredmorcos/backport-11850-to-rec-4.7.x

Backport #11850 (Fix recursor not responsive after Lua config reload) to rec 4.7.x
PowerDNS · Aug 24, 2022 · 355a13e · 355a13e
2 parents abf4b68 + 850237f
commit 355a13e
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 15 deletions.
diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
@@ -924,7 +924,7 @@ void startDoResolve(void* p)
     sr.setInitialRequestId(dc->d_uuid);
     sr.setOutgoingProtobufServers(t_outgoingProtobufServers);
 #ifdef HAVE_FSTRM
-    sr.setFrameStreamServers(t_frameStreamServers);
+    sr.setFrameStreamServers(t_frameStreamServersInfo.servers);
 #endif
 
     bool useMapped = true;

diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc
@@ -43,6 +43,27 @@ LuaConfigItems::LuaConfigItems()
 
 /* DID YOU READ THE STORY ABOVE? */
 
+bool operator==(const FrameStreamExportConfig& configA, const FrameStreamExportConfig& configB)
+{
+  // clang-format off
+  return configA.enabled              == configB.enabled              &&
+         configA.logQueries           == configB.logQueries           &&
+         configA.logResponses         == configB.logResponses         &&
+         configA.bufferHint           == configB.bufferHint           &&
+         configA.flushTimeout         == configB.flushTimeout         &&
+         configA.inputQueueSize       == configB.inputQueueSize       &&
+         configA.outputQueueSize      == configB.outputQueueSize      &&
+         configA.queueNotifyThreshold == configB.queueNotifyThreshold &&
+         configA.reopenInterval       == configB.reopenInterval       &&
+         configA.servers              == configB.servers;
+  // clang-format on
+}
+
+bool operator!=(const FrameStreamExportConfig& configA, const FrameStreamExportConfig& configB)
+{
+  return !(configA == configB);
+}
+
 template <typename C>
 typename C::value_type::second_type constGet(const C& c, const std::string& name)
 {

diff --git a/pdns/rec-lua-conf.hh b/pdns/rec-lua-conf.hh
@@ -27,6 +27,8 @@
 #include "filterpo.hh"
 #include "validate.hh"
 #include "rec-zonetocache.hh"
+#include "logging.hh"
+#include "fstrm_logger.hh"
 
 struct ProtobufExportConfig
 {
@@ -57,6 +59,9 @@ struct FrameStreamExportConfig
   unsigned reopenInterval{0};
 };
 
+bool operator==(const FrameStreamExportConfig& configA, const FrameStreamExportConfig& configB);
+bool operator!=(const FrameStreamExportConfig& configA, const FrameStreamExportConfig& configB);
+
 struct TrustAnchorFileInfo
 {
   uint32_t interval{24};

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
@@ -55,8 +55,7 @@ static thread_local uint64_t t_protobufServersGeneration;
 static thread_local uint64_t t_outgoingProtobufServersGeneration;
 
 #ifdef HAVE_FSTRM
-thread_local std::shared_ptr<std::vector<std::unique_ptr<FrameStreamLogger>>> t_frameStreamServers{nullptr};
-thread_local uint64_t t_frameStreamServersGeneration;
+thread_local FrameStreamServersInfo t_frameStreamServersInfo;
 #endif /* HAVE_FSTRM */
 
 string g_programname = "pdns_recursor";
@@ -591,28 +590,38 @@ static std::shared_ptr<std::vector<std::unique_ptr<FrameStreamLogger>>> startFra
   return result;
 }
 
+static void asyncFrameStreamLoggersCleanup(std::shared_ptr<std::vector<std::unique_ptr<FrameStreamLogger>>>&& servers)
+{
+  auto thread = std::thread([&] {
+    servers.reset();
+  });
+  thread.detach();
+}
+
 bool checkFrameStreamExport(LocalStateHolder<LuaConfigItems>& luaconfsLocal)
 {
   if (!luaconfsLocal->frameStreamExportConfig.enabled) {
-    if (t_frameStreamServers) {
+    if (t_frameStreamServersInfo.servers) {
       // dt's take care of cleanup
-      t_frameStreamServers.reset();
+      asyncFrameStreamLoggersCleanup(std::move(t_frameStreamServersInfo.servers));
+      t_frameStreamServersInfo.config = luaconfsLocal->frameStreamExportConfig;
     }
 
     return false;
   }
 
-  /* if the server was not running, or if it was running according to a
-     previous configuration */
-  if (!t_frameStreamServers || t_frameStreamServersGeneration < luaconfsLocal->generation) {
-
-    if (t_frameStreamServers) {
+  /* if the server was not running, or if it was running according to a previous
+   * configuration
+   */
+  if (t_frameStreamServersInfo.generation < luaconfsLocal->generation && t_frameStreamServersInfo.config != luaconfsLocal->frameStreamExportConfig) {
+    if (t_frameStreamServersInfo.servers) {
       // dt's take care of cleanup
-      t_frameStreamServers.reset();
+      asyncFrameStreamLoggersCleanup(std::move(t_frameStreamServersInfo.servers));
     }
 
-    t_frameStreamServers = startFrameStreamServers(luaconfsLocal->frameStreamExportConfig);
-    t_frameStreamServersGeneration = luaconfsLocal->generation;
+    t_frameStreamServersInfo.servers = startFrameStreamServers(luaconfsLocal->frameStreamExportConfig);
+    t_frameStreamServersInfo.config = luaconfsLocal->frameStreamExportConfig;
+    t_frameStreamServersInfo.generation = luaconfsLocal->generation;
   }
 
   return true;

diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh
@@ -245,8 +245,14 @@ extern thread_local std::shared_ptr<nod::UniqueResponseDB> t_udrDBp;
 #endif
 
 #ifdef HAVE_FSTRM
-extern thread_local std::shared_ptr<std::vector<std::unique_ptr<FrameStreamLogger>>> t_frameStreamServers;
-extern thread_local uint64_t t_frameStreamServersGeneration;
+struct FrameStreamServersInfo
+{
+  std::shared_ptr<std::vector<std::unique_ptr<FrameStreamLogger>>> servers;
+  uint64_t generation;
+  FrameStreamExportConfig config;
+};
+
+extern thread_local FrameStreamServersInfo t_frameStreamServersInfo;
 #endif /* HAVE_FSTRM */
 
 #ifdef HAVE_BOOST_CONTAINER_FLAT_SET_HPP