diff --git a/dbcon/joblist/resourcemanager.cpp b/dbcon/joblist/resourcemanager.cpp index 76f9f5498..7bef4ae1e 100644 --- a/dbcon/joblist/resourcemanager.cpp +++ b/dbcon/joblist/resourcemanager.cpp @@ -385,16 +385,24 @@ bool ResourceManager::getMysqldInfo( { 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); - std::string encryptedPW = getStringVal("CrossEngineSupport", "Password", "", reReadConfig); + u = getStringVal("CrossEngineSupport", "User", hostUserUnassignedValue); + std::string encryptedPW = getStringVal("CrossEngineSupport", "Password", ""); //This will return back the plaintext password if there is no MCSDATADIR/.secrets file present w = decrypt_password(encryptedPW); // 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", ""); - return h != hostUserUnassignedValue && u != hostUserUnassignedValue && p; + bool rc = true; + + if ((h.compare("unassigned") == 0) || + (u.compare("unassigned") == 0) || + (p == 0)) + rc = false; + + return rc; } bool ResourceManager::queryStatsEnabled() const diff --git a/dbcon/joblist/resourcemanager.h b/dbcon/joblist/resourcemanager.h index 71c525cce..31106b3be 100644 --- a/dbcon/joblist/resourcemanager.h +++ b/dbcon/joblist/resourcemanager.h @@ -35,7 +35,6 @@ #include "calpontselectexecutionplan.h" #include "resourcedistributor.h" #include "installdir.h" -#include "branchpred.h" #include "atomicops.h" @@ -561,10 +560,7 @@ 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 bool reReadConfigIfNeeded = false) const; + std::string getStringVal(const std::string& section, const std::string& name, const std::string& defVal) const; template IntType getUintVal(const std::string& section, const std::string& name, IntType defval) const; @@ -625,20 +621,13 @@ private: }; -inline std::string ResourceManager::getStringVal(const std::string& section, - const std::string& name, - const std::string& defval, - const bool reReadConfigIfNeeded) const +inline std::string ResourceManager::getStringVal(const std::string& section, const std::string& name, const std::string& defval) const { - std::string val = UNLIKELY(reReadConfigIfNeeded) - ? fConfig->getFromActualConfig(section, name) - : fConfig->getConfig(section, name); + std::string val = fConfig->getConfig(section, name); #ifdef DEBUGRM std::cout << "RM getStringVal for " << section << " : " << name << " val: " << val << " default: " << defval << std::endl; #endif - if (val.empty()) - val = defval; - return val; + return (0 == val.length() ? defval : val); } template diff --git a/oam/oamcpp/oamcache.cpp b/oam/oamcpp/oamcache.cpp index 4cda1577c..ba049ed5e 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,27 +38,19 @@ using namespace boost; namespace { - oam::OamCache* oamCache = nullptr; - boost::mutex cacheLock; +oam::OamCache* oamCache = 0; +boost::mutex cacheLock; } namespace oam { -std::atomic_bool hasOAMCache{false}; OamCache* OamCache::makeOamCache() { - if (!hasOAMCache.load(std::memory_order_relaxed)) - { - boost::mutex::scoped_lock lk(cacheLock); + boost::mutex::scoped_lock lk(cacheLock); - if (oamCache == nullptr) - { - oamCache = new OamCache(); - oamCache->checkReload(); - hasOAMCache.store(true, std::memory_order_relaxed); - } - } + if (oamCache == 0) + oamCache = new OamCache(); return oamCache; } @@ -88,7 +80,7 @@ void OamCache::checkReload() dbRootPMMap.reset(new map()); - //cerr << "reloading oamcache\n"; + //cout << "reloading oamcache\n"; for (uint32_t i = 0; i < dbroots.size(); i++) { oam.getDbrootPmConfig(dbroots[i], temp); @@ -162,41 +154,64 @@ 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) @@ -237,11 +252,16 @@ 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 93a777dfe..99691e36f 100644 --- a/utils/configcpp/configcpp.cpp +++ b/utils/configcpp/configcpp.cpp @@ -51,7 +51,6 @@ namespace fs = boost::filesystem; #include //#define NDEBUG #include -#include #include "configcpp.h" @@ -68,52 +67,51 @@ namespace fs = boost::filesystem; namespace { - static const std::string configDefaultFileName("Columnstore.xml"); - const fs::path defaultConfigFilePath(configDefaultFileName); +const fs::path defaultCalpontConfigFile("Columnstore.xml"); } namespace config { - 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::configMap_t Config::fInstanceMap; +boost::mutex Config::fInstanceMapMutex; +boost::mutex Config::fXmlLock; +boost::mutex Config::fWriteXmlLock; Config* Config::makeConfig(const string& cf) { - 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]; + return makeConfig(cf.c_str()); } Config* Config::makeConfig(const char* cf) { - return cf ? makeConfig(std::string(cf)) : makeConfig(std::string("")); + 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]; } Config::Config(const string& configFile) : @@ -221,6 +219,8 @@ 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"); @@ -229,57 +229,46 @@ const string Config::getConfig(const string& section, const string& name) throw runtime_error("Config::getConfig: no XML document!"); } - return fParser.getConfig(fDoc, section, name); -} - -void Config::getConfig(const string& section, const string& name, vector& values) -{ - 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) { - 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(); - } + 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!"); + + struct stat statbuf; + + if (stat(fConfigFile.c_str(), &statbuf) == 0) + { + if (statbuf.st_mtime != fMtime) + { + closeConfig(); + fMtime = statbuf.st_mtime; + parseDoc(); + } + } + + fParser.getConfig(fDoc, section, name, values); +} -// 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); @@ -295,6 +284,7 @@ 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) @@ -369,14 +359,14 @@ void Config::writeConfig(const string& configFile) const fs::rename(outFilePth, configFilePth); #else - const fs::path defaultConfigFilePathTemp("Columnstore.xml.temp"); + const fs::path defaultCalpontConfigFileTemp("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(defaultConfigFilePath); - fs::path dcft = etcdir / fs::path(defaultConfigFilePathTemp); + fs::path dcf = etcdir / fs::path(defaultCalpontConfigFile); + fs::path dcft = etcdir / fs::path(defaultCalpontConfigFileTemp); fs::path scft = etcdir / fs::path(saveCalpontConfigFileTemp); fs::path tcft = etcdir / fs::path(tmpCalpontConfigFileTemp); @@ -561,12 +551,6 @@ void Config::deleteInstanceMap() } fInstanceMap.clear(); - - if (globConfigInstancePtr) - { - delete globConfigInstancePtr; - globConfigInstancePtr = nullptr; - } } /* static */ @@ -619,6 +603,8 @@ 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) @@ -627,7 +613,6 @@ time_t Config::getCurrentMTime() return 0; } -// Utilized by getConfig only const vector Config::enumConfig() { boost::recursive_mutex::scoped_lock lk(fLock); @@ -652,7 +637,6 @@ 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 57e90bc20..726287c3f 100644 --- a/utils/configcpp/configcpp.h +++ b/utils/configcpp/configcpp.h @@ -87,15 +87,6 @@ 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. @@ -113,8 +104,6 @@ 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