diff --git a/dbcon/joblist/resourcemanager.cpp b/dbcon/joblist/resourcemanager.cpp index e34221131..9a5d501b3 100644 --- a/dbcon/joblist/resourcemanager.cpp +++ b/dbcon/joblist/resourcemanager.cpp @@ -365,19 +365,16 @@ void ResourceManager::hbrPredicate() { } bool ResourceManager::getMysqldInfo( std::string& h, std::string& u, std::string& w, unsigned int& p) const { - h = getStringVal("CrossEngineSupport", "Host", "unassigned"); + static const std::string hostUserUnassignedValue("unassigned"); + // MCS will read username and pass from disk if the config changed. + bool reReadConfig = true; + u = getStringVal("CrossEngineSupport", "User", hostUserUnassignedValue, reReadConfig); + w = getStringVal("CrossEngineSupport", "Password", "", reReadConfig); + // MCS will not read username and pass from disk if the config changed. + h = getStringVal("CrossEngineSupport", "Host", hostUserUnassignedValue); p = getUintVal("CrossEngineSupport", "Port", 0); - u = getStringVal("CrossEngineSupport", "User", "unassigned"); - w = getStringVal("CrossEngineSupport", "Password", ""); - bool rc = true; - - if ((h.compare("unassigned") == 0) || - (u.compare("unassigned") == 0) || - (p == 0)) - rc = false; - - return rc; + return h != hostUserUnassignedValue && u != hostUserUnassignedValue && p; } bool ResourceManager::queryStatsEnabled() const diff --git a/dbcon/joblist/resourcemanager.h b/dbcon/joblist/resourcemanager.h index 898d5e61c..8fd1dc6b2 100644 --- a/dbcon/joblist/resourcemanager.h +++ b/dbcon/joblist/resourcemanager.h @@ -35,6 +35,7 @@ #include "calpontselectexecutionplan.h" #include "resourcedistributor.h" #include "installdir.h" +#include "branchpred.h" #include "atomicops.h" @@ -547,7 +548,10 @@ private: * @param name the param name whose value is to be returned * @param defVal the default value returned if the value is missing */ - std::string getStringVal(const std::string& section, const std::string& name, const std::string& defVal) const; + std::string getStringVal(const std::string& section, + const std::string& name, + const std::string& defVal, + const bool reReadConfigIfNeeded = false) const; template IntType getUintVal(const std::string& section, const std::string& name, IntType defval) const; @@ -603,13 +607,20 @@ private: }; -inline std::string ResourceManager::getStringVal(const std::string& section, const std::string& name, const std::string& defval) const +inline std::string ResourceManager::getStringVal(const std::string& section, + const std::string& name, + const std::string& defval, + const bool reReadConfigIfNeeded) const { - std::string val = fConfig->getConfig(section, name); + std::string val = UNLIKELY(reReadConfigIfNeeded) + ? fConfig->getFromActualConfig(section, name) + : fConfig->getConfig(section, name); #ifdef DEBUGRM std::cout << "RM getStringVal for " << section << " : " << name << " val: " << val << " default: " << defval << std::endl; #endif - return (0 == val.length() ? defval : val); + if (val.empty()) + val = defval; + return val; } template diff --git a/oam/oamcpp/oamcache.cpp b/oam/oamcpp/oamcache.cpp index ba049ed5e..4cda1577c 100644 --- a/oam/oamcpp/oamcache.cpp +++ b/oam/oamcpp/oamcache.cpp @@ -21,10 +21,10 @@ #include #include #include +#include using namespace std; #include -#include using namespace boost; #define OAMCACHE_DLLEXPORT @@ -38,19 +38,27 @@ using namespace boost; namespace { -oam::OamCache* oamCache = 0; -boost::mutex cacheLock; + oam::OamCache* oamCache = nullptr; + boost::mutex cacheLock; } namespace oam { +std::atomic_bool hasOAMCache{false}; OamCache* OamCache::makeOamCache() { - boost::mutex::scoped_lock lk(cacheLock); + if (!hasOAMCache.load(std::memory_order_relaxed)) + { + boost::mutex::scoped_lock lk(cacheLock); - if (oamCache == 0) - oamCache = new OamCache(); + if (oamCache == nullptr) + { + oamCache = new OamCache(); + oamCache->checkReload(); + hasOAMCache.store(true, std::memory_order_relaxed); + } + } return oamCache; } @@ -80,7 +88,7 @@ void OamCache::checkReload() dbRootPMMap.reset(new map()); - //cout << "reloading oamcache\n"; + //cerr << "reloading oamcache\n"; for (uint32_t i = 0; i < dbroots.size(); i++) { oam.getDbrootPmConfig(dbroots[i], temp); @@ -154,64 +162,41 @@ void OamCache::checkReload() OamCache::dbRootPMMap_t OamCache::getDBRootToPMMap() { - boost::mutex::scoped_lock lk(cacheLock); - - checkReload(); return dbRootPMMap; } OamCache::dbRootPMMap_t OamCache::getDBRootToConnectionMap() { - boost::mutex::scoped_lock lk(cacheLock); - - checkReload(); return dbRootConnectionMap; } OamCache::PMDbrootsMap_t OamCache::getPMToDbrootsMap() { - boost::mutex::scoped_lock lk(cacheLock); - - checkReload(); return pmDbrootsMap; } uint32_t OamCache::getDBRootCount() { - boost::mutex::scoped_lock lk(cacheLock); - - checkReload(); return numDBRoots; } DBRootConfigList& OamCache::getDBRootNums() { - boost::mutex::scoped_lock lk(cacheLock); - - checkReload(); return dbroots; } std::vector& OamCache::getModuleIds() { - boost::mutex::scoped_lock lk(cacheLock); - - checkReload(); return moduleIds; } std::string OamCache::getOAMParentModuleName() { - boost::mutex::scoped_lock lk(cacheLock); - - checkReload(); return OAMParentModuleName; } int OamCache::getLocalPMId() { - boost::mutex::scoped_lock lk(cacheLock); - // This comes from the file /var/lib/columnstore/local/module, not from the xml. // Thus, it's not refreshed during checkReload(). if (mLocalPMId > 0) @@ -252,16 +237,11 @@ int OamCache::getLocalPMId() string OamCache::getSystemName() { - boost::mutex::scoped_lock lk(cacheLock); - - checkReload(); return systemName; } string OamCache::getModuleName() { - boost::mutex::scoped_lock lk(cacheLock); - if (!moduleName.empty()) return moduleName; diff --git a/utils/configcpp/configcpp.cpp b/utils/configcpp/configcpp.cpp index 823e6b2a6..c1171910b 100644 --- a/utils/configcpp/configcpp.cpp +++ b/utils/configcpp/configcpp.cpp @@ -51,6 +51,7 @@ namespace fs = boost::filesystem; #include //#define NDEBUG #include +#include #include "configcpp.h" @@ -64,51 +65,52 @@ namespace fs = boost::filesystem; namespace { -const fs::path defaultCalpontConfigFile("Columnstore.xml"); + static const std::string configDefaultFileName("Columnstore.xml"); + const fs::path defaultConfigFilePath(configDefaultFileName); } namespace config { -Config::configMap_t Config::fInstanceMap; -boost::mutex Config::fInstanceMapMutex; -boost::mutex Config::fXmlLock; -boost::mutex Config::fWriteXmlLock; + Config* globConfigInstancePtr = nullptr; + Config::configMap_t Config::fInstanceMap; + boost::mutex Config::fInstanceMapMutex; + // 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; Config* Config::makeConfig(const string& cf) { - return makeConfig(cf.c_str()); + if (cf.empty() || cf == configDefaultFileName) + { + if (!globHasConfig.load(std::memory_order_relaxed)) + { + // To save against the moment zero race when multiple threads hits + // this scope. + boost::mutex::scoped_lock lk(fInstanceMapMutex); + if (globConfigInstancePtr) + 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); + if (fInstanceMap.find(cf) == fInstanceMap.end()) + { + fInstanceMap[cf] = new Config(cf); + } + return fInstanceMap[cf]; } Config* Config::makeConfig(const char* cf) { - boost::mutex::scoped_lock lk(fInstanceMapMutex); - - static string defaultFilePath; - - if (cf == 0 || *cf == 0) - { - fs::path configFilePath; - configFilePath = fs::path(MCSSYSCONFDIR) / fs::path("columnstore") / defaultCalpontConfigFile; - defaultFilePath = configFilePath.string(); - - if (fInstanceMap.find(defaultFilePath) == fInstanceMap.end()) - { - Config* instance = new Config(defaultFilePath); - fInstanceMap[defaultFilePath] = instance; - } - - return fInstanceMap[defaultFilePath]; - } - - string configFile(cf); - - if (fInstanceMap.find(configFile) == fInstanceMap.end()) - { - Config* instance = new Config(configFile); - fInstanceMap[configFile] = instance; - } - - return fInstanceMap[configFile]; + return cf ? makeConfig(std::string(cf)) : makeConfig(std::string("")); } Config::Config(const string& configFile) : @@ -216,8 +218,6 @@ void Config::closeConfig(void) const string Config::getConfig(const string& section, const string& name) { - boost::recursive_mutex::scoped_lock lk(fLock); - if (section.length() == 0 || name.length() == 0) throw invalid_argument("Config::getConfig: both section and name must have a length"); @@ -226,46 +226,57 @@ const string Config::getConfig(const string& section, const string& name) throw runtime_error("Config::getConfig: no XML document!"); } - struct stat statbuf; - - if (stat(fConfigFile.c_str(), &statbuf) == 0) - { - if (statbuf.st_mtime != fMtime) - { - closeConfig(); - fMtime = statbuf.st_mtime; - parseDoc(); - } - } - return fParser.getConfig(fDoc, section, name); } void Config::getConfig(const string& section, const string& name, vector& values) { - boost::recursive_mutex::scoped_lock lk(fLock); - if (section.length() == 0) throw invalid_argument("Config::getConfig: section must have a length"); if (fDoc == 0) throw runtime_error("Config::getConfig: no XML document!"); + fParser.getConfig(fDoc, section, name, values); +} + +const string Config::getFromActualConfig(const string& section, const string& name) +{ + if (section.length() == 0 || name.length() == 0) + throw invalid_argument("Config::getFromActualConfig: both section and name must have a length"); + + if (fDoc == 0) + { + throw runtime_error("Config::getFromActualConfig: no XML document!"); + } + struct stat statbuf; if (stat(fConfigFile.c_str(), &statbuf) == 0) { + // Config was changed on disk since last read. if (statbuf.st_mtime != fMtime) { - closeConfig(); - fMtime = statbuf.st_mtime; - parseDoc(); + 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) + { + closeConfig(); + fMtime = statbuf.st_mtime; + parseDoc(); + } } } - fParser.getConfig(fDoc, section, name, values); + return fParser.getConfig(fDoc, section, name); } + +// NB The only utility that uses setConfig is setConfig binary. +// !!!Don't ever ever use this in the engine code b/c it might result in a race +// b/w getConfig and setConfig methods.!!! void Config::setConfig(const string& section, const string& name, const string& value) { boost::recursive_mutex::scoped_lock lk(fLock); @@ -281,7 +292,6 @@ void Config::setConfig(const string& section, const string& name, const string& struct stat statbuf; memset(&statbuf, 0, sizeof(statbuf)); - if (stat(fConfigFile.c_str(), &statbuf) == 0) { if (statbuf.st_mtime != fMtime) @@ -356,14 +366,14 @@ void Config::writeConfig(const string& configFile) const fs::rename(outFilePth, configFilePth); #else - const fs::path defaultCalpontConfigFileTemp("Columnstore.xml.temp"); + const fs::path defaultConfigFilePathTemp("Columnstore.xml.temp"); const fs::path saveCalpontConfigFileTemp("Columnstore.xml.columnstoreSave"); const fs::path tmpCalpontConfigFileTemp("Columnstore.xml.temp1"); fs::path etcdir = fs::path(MCSSYSCONFDIR) / fs::path("columnstore"); - fs::path dcf = etcdir / fs::path(defaultCalpontConfigFile); - fs::path dcft = etcdir / fs::path(defaultCalpontConfigFileTemp); + fs::path dcf = etcdir / fs::path(defaultConfigFilePath); + fs::path dcft = etcdir / fs::path(defaultConfigFilePathTemp); fs::path scft = etcdir / fs::path(saveCalpontConfigFileTemp); fs::path tcft = etcdir / fs::path(tmpCalpontConfigFileTemp); @@ -548,6 +558,12 @@ void Config::deleteInstanceMap() } fInstanceMap.clear(); + + if (globConfigInstancePtr) + { + delete globConfigInstancePtr; + globConfigInstancePtr = nullptr; + } } /* static */ @@ -600,8 +616,6 @@ int64_t Config::fromText(const std::string& text) time_t Config::getCurrentMTime() { - boost::recursive_mutex::scoped_lock lk(fLock); - struct stat statbuf; if (stat(fConfigFile.c_str(), &statbuf) == 0) @@ -610,6 +624,7 @@ time_t Config::getCurrentMTime() return 0; } +// Utilized by getConfig only const vector Config::enumConfig() { boost::recursive_mutex::scoped_lock lk(fLock); @@ -634,6 +649,7 @@ const vector Config::enumConfig() return fParser.enumConfig(fDoc); } +// Utilized by getConfig only const vector Config::enumSection(const string& section) { boost::recursive_mutex::scoped_lock lk(fLock); diff --git a/utils/configcpp/configcpp.h b/utils/configcpp/configcpp.h index f33e222bd..2dfe8be5b 100644 --- a/utils/configcpp/configcpp.h +++ b/utils/configcpp/configcpp.h @@ -87,6 +87,15 @@ public: */ EXPORT const std::string getConfig(const std::string& section, const std::string& name); + /** @brief get name's value from section + * + * get name's value from section in the current config file re-reading the + * config file if it was updated. + * @param section the name of the config file section to search + * @param name the param name whose value is to be returned + */ + const std::string getFromActualConfig(const std::string& section, const std::string& name); + /** @brief get all name's values from a section * * get name's values from section in the current config file. @@ -104,6 +113,8 @@ public: * @param name the param name whose value is to be updated * @param value the param value */ + // !!!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); /** @brief delete name from section