Simpler Config

utils/configcpp/configcpp.cpp

@@ -61,24 +61,8 @@ namespace fs = boost::filesystem;
 
 #include "bytestream.h"
 
-namespace
-{
-static const std::string configDefaultFileName("Columnstore.xml");
-const fs::path defaultConfigFilePath(configDefaultFileName);
-}  // namespace
-
 namespace config
 {
-boost::mutex Config::fInstanceMapMutex;
-Config::configMap_t Config::fInstanceMap;
-// duplicate to that in the Config class
-boost::mutex Config::fXmlLock;
-// duplicate to that in the Config class
-boost::mutex Config::fWriteXmlLock;
-std::atomic_bool globHasConfig;
-
-ConfigUniqPtr globConfigInstancePtr;
-
 
 void Config::checkAndReloadConfig()
 {
@@ -95,46 +79,35 @@ void Config::checkAndReloadConfig()
   }
 }
 
+Config& Config::globConfigInstance()
+{
+  std::string configFilePath =
+      std::string(MCSSYSCONFDIR) + std::string("/columnstore/") + configDefaultFileName();
+  static Config config(configFilePath);
+  return config;
+}
+
 Config* Config::makeConfig(const string& cf)
 {
-  if (cf.empty() || cf == configDefaultFileName)
+  if (cf.empty() || cf == configDefaultFileName())
   {
-    if (!globHasConfig.load(std::memory_order_relaxed))
+    boost::mutex::scoped_lock lk(instanceMapMutex());
-    {
+    globConfigInstance().checkAndReloadConfig();
-      // To save against the moment zero race when multiple threads hits
+    return &globConfigInstance();
-      // this scope.
-      boost::mutex::scoped_lock lk(fInstanceMapMutex);
-      if (globConfigInstancePtr)
-      {
-        globConfigInstancePtr->checkAndReloadConfig();
-        return globConfigInstancePtr.get();
   }
 
-      // Make this configurable at least at compile-time.
+  boost::mutex::scoped_lock lk(instanceMapMutex());
-      std::string configFilePath =
-          std::string(MCSSYSCONFDIR) + std::string("/columnstore/") + configDefaultFileName;
-      globConfigInstancePtr.reset(new Config(configFilePath));
-      globHasConfig.store(true, std::memory_order_relaxed);
-      return globConfigInstancePtr.get();
-    }
 
-    boost::mutex::scoped_lock lk(fInstanceMapMutex);
+  if (instanceMap().find(cf) == instanceMap().end())
-    globConfigInstancePtr->checkAndReloadConfig();
-    return globConfigInstancePtr.get();
-  }
-
-  boost::mutex::scoped_lock lk(fInstanceMapMutex);
-
-  if (fInstanceMap.find(cf) == fInstanceMap.end())
   {
-    fInstanceMap[cf] = new Config(cf);
+    instanceMap()[cf].reset(new Config(cf));
   }
   else
   {
-    fInstanceMap[cf]->checkAndReloadConfig();
+    instanceMap()[cf]->checkAndReloadConfig();
   }
 
-  return fInstanceMap[cf];
+  return instanceMap()[cf].get();
 }
 
 Config* Config::makeConfig(const char* cf)
@@ -191,9 +164,9 @@ void Config::parseDoc(void)
       cerr << oss.str() << endl;
     }
 
-    fXmlLock.lock();
+    xmlMutex().lock();
     fDoc = xmlParseFile(fConfigFile.c_str());
-    fXmlLock.unlock();
+    xmlMutex().unlock();
 
     fl.l_type = F_UNLCK;  // unlock
     fcntl(fd, F_SETLK, &fl);
@@ -346,10 +319,10 @@ void Config::writeConfig(const string& configFile) const
   if (fDoc == 0)
     throw runtime_error("Config::writeConfig: no XML document!");
 
-
+  static const fs::path defaultConfigFilePath("Columnstore.xml");
-  const fs::path defaultConfigFilePathTemp("Columnstore.xml.temp");
+  static const fs::path defaultConfigFilePathTemp("Columnstore.xml.temp");
-  const fs::path saveCalpontConfigFileTemp("Columnstore.xml.columnstoreSave");
+  static const fs::path saveCalpontConfigFileTemp("Columnstore.xml.columnstoreSave");
-  const fs::path tmpCalpontConfigFileTemp("Columnstore.xml.temp1");
+  static const fs::path tmpCalpontConfigFileTemp("Columnstore.xml.temp1");
 
   fs::path etcdir = fs::path(MCSSYSCONFDIR) / fs::path("columnstore");
 
@@ -439,7 +412,7 @@ void Config::writeConfig(const string& configFile) const
 
 void Config::write(void) const
 {
-  boost::mutex::scoped_lock lk(fWriteXmlLock);
+  boost::mutex::scoped_lock lk(writeXmlMutex());
   write(fConfigFile);
 }
 
@@ -630,18 +603,4 @@ std::string Config::getTempFileDir(Config::TempDirPurpose what)
   return {};
 }
 
-void Config::ConfigDeleter::operator()(Config* config)
-{
-  boost::mutex::scoped_lock lk(fInstanceMapMutex);
-
-  for (Config::configMap_t::iterator iter = fInstanceMap.begin(); iter != fInstanceMap.end(); ++iter)
-  {
-    Config* instance = iter->second;
-    delete instance;
-  }
-
-  fInstanceMap.clear();
-  delete config;
-}
-
 }  // namespace config

utils/configcpp/configcpp.h

@@ -42,7 +42,6 @@ class ByteStream;
 
 }
 
-#define EXPORT
 
 namespace config
 {
@@ -56,26 +55,22 @@ namespace config
 class Config
 {
  public:
-  struct ConfigDeleter
-  {
-    void operator()(Config* config);
-  };
 
   /** @brief Config factory method
    *
    * Creates a singleton Config object
    */
-  EXPORT static Config* makeConfig(const char* cf = 0);
+  static Config* makeConfig(const char* cf = 0);
 
   /** @brief Config factory method
    *
    * Creates a singleton Config object
    */
-  EXPORT static Config* makeConfig(const std::string& cf);
+  static Config* makeConfig(const std::string& cf);
 
   /** @brief dtor
    */
-  EXPORT virtual ~Config();
+  virtual ~Config();
 
   /** @brief get name's value from section
    *
@@ -83,7 +78,7 @@ class Config
    * @param section the name of the config file section to search
    * @param name the param name whose value is to be returned
    */
-  EXPORT const std::string getConfig(const std::string& section, const std::string& name);
+  const std::string getConfig(const std::string& section, const std::string& name);
 
   /** @brief get name's value from section
    *
@@ -101,7 +96,7 @@ class Config
    * @param name the param name whose value is to be returned
    * @param values the values in the section are returned in this vector
    */
-  EXPORT void getConfig(const std::string& section, const std::string& name,
+  void getConfig(const std::string& section, const std::string& name,
                         std::vector<std::string>& values);
 
   /** @brief set name's value in section
@@ -113,7 +108,7 @@ class Config
    */
   // !!!Don't ever ever use this in the engine code b/c it might result in a race
   // b/w getConfig and setConfig methods.!!!
-  EXPORT void setConfig(const std::string& section, const std::string& name, const std::string& value);
+  void setConfig(const std::string& section, const std::string& name, const std::string& value);
 
   /** @brief delete name from section
    *
@@ -122,33 +117,40 @@ class Config
    * @param name the param name whose entry is to be deleted
    * @note if you delete the last param from a section, the section will still remain
    */
-  EXPORT void delConfig(const std::string& section, const std::string& name);
+  void delConfig(const std::string& section, const std::string& name);
 
   /** @brief write the config file back out to disk
    *
    * write the config file back out to disk using the current filename.
    */
-  EXPORT void write(void) const;
+  void write(void) const;
 
   /** @brief write the config file back out to disk as fileName
    *
    * write the config file out to disk as a new file fileName. Does not affect the current
    * config filename.
    */
-  EXPORT void write(const std::string& fileName) const;
+  void write(const std::string& fileName) const;
 
   /** @brief write a stream copy of config file to disk
    *
    * write a stream copy of config file to disk. used to distributed mass updates to system nodes
    *
    */
-  EXPORT void writeConfigFile(messageqcpp::ByteStream msg) const;
+  void writeConfigFile(messageqcpp::ByteStream msg) const;
 
   /** @brief return the name of this config file
    *
    * return the name of this config file.
    */
-  EXPORT inline const std::string& configFile() const
+
+  static std::string configDefaultFileName()
+  {
+    static std::string defaultConfigName = "Columnstore.xml";
+    return defaultConfigName;
+  }
+
+  inline const std::string& configFile() const
   {
     return fConfigFile;
   }
@@ -157,7 +159,7 @@ class Config
    *
    * deletes \b all config file maps
    */
-  EXPORT static void deleteInstanceMap();
+  static void deleteInstanceMap();
 
   /** @brief parse config file numerics
    *
@@ -167,13 +169,13 @@ class Config
    * An empty string or an unparseable string returns 0.
    * Return a signed numeric value.
    */
-  EXPORT static int64_t fromText(const std::string& text);
+  static int64_t fromText(const std::string& text);
 
   /** @brief parse config file numerics
    *
    * Return an unsigned numeric value.
    */
-  EXPORT static inline uint64_t uFromText(const std::string& text)
+  static inline uint64_t uFromText(const std::string& text)
   {
     return static_cast<uint64_t>(fromText(text));
   }
@@ -189,17 +191,17 @@ class Config
   /** @brief Used externally to check whether there has been a config change without loading everything
    *
    */
-  EXPORT time_t getCurrentMTime();
+  time_t getCurrentMTime();
 
   /** @brief Enumerate all the sections in the config file
    *
    */
-  EXPORT const std::vector<std::string> enumConfig();
+  const std::vector<std::string> enumConfig();
 
   /** @brief Enumerate all the names in a section in the config file
    *
    */
-  EXPORT const std::vector<std::string> enumSection(const std::string& section);
+  const std::vector<std::string> enumSection(const std::string& section);
 
   enum class TempDirPurpose
   {
@@ -207,7 +209,7 @@ class Config
     Aggregates  ///< disk-based aggregation
   };
   /** @brief Return temporaru directory path for the specified purpose */
-  EXPORT std::string getTempFileDir(TempDirPurpose what);
+  std::string getTempFileDir(TempDirPurpose what);
 
  protected:
   /** @brief parse the XML file
@@ -218,7 +220,7 @@ class Config
   /** @brief write the XML tree to disk
    *
    */
-  EXPORT void writeConfig(const std::string& fileName) const;
+  void writeConfig(const std::string& fileName) const;
 
   /** @brief stop processing this XML file
    *
@@ -226,23 +228,39 @@ class Config
   void closeConfig(void);
 
  private:
-  typedef std::map<std::string, Config*> configMap_t;
+  typedef std::map<std::string, std::unique_ptr<Config>> configMap_t;
+
+  static configMap_t& instanceMap()
+  {
+    static configMap_t instanceMap;
+    return instanceMap;
+  }
+
+  static boost::mutex& instanceMapMutex()
+  {
+    static boost::mutex instanceMapMutex;
+    return instanceMapMutex;
+  }
+
+  static boost::mutex& xmlMutex()
+  {
+    static boost::mutex xmlMutex;
+    return xmlMutex;
+  }
+
+  static boost::mutex& writeXmlMutex()
+  {
+    static boost::mutex writeXmlMutex;
+    return writeXmlMutex;
+  }
+
+  static Config& globConfigInstance();
+
 
-  /*
-   */
   Config(const Config& rhs);
-  /*
-   */
   Config& operator=(const Config& rhs);
-
-  /** @brief ctor with config file specified
-   */
   Config(const std::string& configFile);
 
-  static configMap_t fInstanceMap;
-  static boost::mutex fInstanceMapMutex;
-  static boost::mutex fXmlLock;
-  static boost::mutex fWriteXmlLock;
 
   xmlDocPtr fDoc;
   const std::string fConfigFile;
@@ -258,10 +276,6 @@ class Config
 };
 
 
-
-using ConfigUniqPtr = std::unique_ptr<Config, Config::ConfigDeleter>;
-
-
 }  // namespace config
 
 #undef EXPORT