MCOL-5306 Re-read the config (Columnstore.xml) file if it was updated.

The existing implementation of Config::makeConfig() factory method
was returning a possibly stale config to the caller, without checking
if the config file was updated since the last read. This bug triggered
a scenario as described in MCOL-5306 where after a failover in an MCS
cluster, the controllernode coordinates changed in the config file
after failover and the existing mariadbd process was still using the
old controllernode coordinates. This lead to failed network connection
between mariadbd and the new controllernode.

The change in this fix, however, is more generic and not just limited
to this above scenario.

utils/configcpp/configcpp.cpp

@@ -83,6 +83,21 @@ boost::mutex Config::fXmlLock;
 boost::mutex Config::fWriteXmlLock;
 std::atomic_bool globHasConfig;
 
+void Config::checkAndReloadConfig()
+{
+  struct stat statbuf;
+
+  if (stat(fConfigFile.c_str(), &statbuf) == 0)
+  {
+    if (statbuf.st_mtime != fMtime)
+    {
+      closeConfig();
+      fMtime = statbuf.st_mtime;
+      parseDoc();
+    }
+  }
+}
+
 Config* Config::makeConfig(const string& cf)
 {
   if (cf.empty() || cf == configDefaultFileName)
@@ -93,22 +108,35 @@ Config* Config::makeConfig(const string& cf)
       // this scope.
       boost::mutex::scoped_lock lk(fInstanceMapMutex);
       if (globConfigInstancePtr)
+      {
+        globConfigInstancePtr->checkAndReloadConfig();
         return globConfigInstancePtr;
+      }
 
       // Make this configurable at least at compile-time.
       std::string configFilePath =
           std::string(MCSSYSCONFDIR) + std::string("/columnstore/") + configDefaultFileName;
       globConfigInstancePtr = new Config(configFilePath);
       globHasConfig.store(true, std::memory_order_relaxed);
+      return globConfigInstancePtr;
     }
+
+    boost::mutex::scoped_lock lk(fInstanceMapMutex);
+    globConfigInstancePtr->checkAndReloadConfig();
     return globConfigInstancePtr;
   }
 
   boost::mutex::scoped_lock lk(fInstanceMapMutex);
+
   if (fInstanceMap.find(cf) == fInstanceMap.end())
   {
     fInstanceMap[cf] = new Config(cf);
   }
+  else
+  {
+    fInstanceMap[cf]->checkAndReloadConfig();
+  }
+
   return fInstanceMap[cf];
 }
 
@@ -256,12 +284,7 @@ const string Config::getFromActualConfig(const string& section, const string& na
       boost::recursive_mutex::scoped_lock lk(fLock);
       // To protect the potential race that happens right after
       // the config was changed.
-      if (!stat(fConfigFile.c_str(), &statbuf) && statbuf.st_mtime != fMtime)
+      checkAndReloadConfig();
-      {
-        closeConfig();
-        fMtime = statbuf.st_mtime;
-        parseDoc();
-      }
     }
   }
 
@@ -312,17 +335,7 @@ void Config::delConfig(const string& section, const string& name)
     throw runtime_error("Config::delConfig: no XML document!");
   }
 
-  struct stat statbuf;
+  checkAndReloadConfig();
-
-  if (stat(fConfigFile.c_str(), &statbuf) == 0)
-  {
-    if (statbuf.st_mtime != fMtime)
-    {
-      closeConfig();
-      fMtime = statbuf.st_mtime;
-      parseDoc();
-    }
-  }
 
   fParser.delConfig(fDoc, section, name);
   return;
@@ -627,17 +640,7 @@ const vector<string> Config::enumConfig()
     throw runtime_error("Config::getConfig: no XML document!");
   }
 
-  struct stat statbuf;
+  checkAndReloadConfig();
-
-  if (stat(fConfigFile.c_str(), &statbuf) == 0)
-  {
-    if (statbuf.st_mtime != fMtime)
-    {
-      closeConfig();
-      fMtime = statbuf.st_mtime;
-      parseDoc();
-    }
-  }
 
   return fParser.enumConfig(fDoc);
 }
@@ -652,17 +655,7 @@ const vector<string> Config::enumSection(const string& section)
     throw runtime_error("Config::getConfig: no XML document!");
   }
 
-  struct stat statbuf;
+  checkAndReloadConfig();
-
-  if (stat(fConfigFile.c_str(), &statbuf) == 0)
-  {
-    if (statbuf.st_mtime != fMtime)
-    {
-      closeConfig();
-      fMtime = statbuf.st_mtime;
-      parseDoc();
-    }
-  }
 
   return fParser.enumSection(fDoc, section);
 }

utils/configcpp/configcpp.h

@@ -248,6 +248,11 @@ class Config
   time_t fMtime;
   mutable boost::recursive_mutex fLock;
   XMLParser fParser;
+
+  /** @brief Re-read the config file if it was updated
+   *
+   */
+  void checkAndReloadConfig();
 };
 
 }  // namespace config