From 3b026f44b41766c09aceac0a0447f63ece9edebc Mon Sep 17 00:00:00 2001 From: David Hill Date: Tue, 9 Oct 2018 15:34:35 -0500 Subject: [PATCH 1/4] MCOL-1762 - added joblist ThreadPoolSize --- tools/configMgt/autoConfigure.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/configMgt/autoConfigure.cpp b/tools/configMgt/autoConfigure.cpp index 1e7f98fdd..e87401127 100755 --- a/tools/configMgt/autoConfigure.cpp +++ b/tools/configMgt/autoConfigure.cpp @@ -1884,12 +1884,15 @@ int main(int argc, char *argv[]) string PrefetchThreshold; string MaxOutstandingRequests; string PmMaxMemorySmallSide; + string ThreadPoolSize; + try { ColScanReadAheadBlocks = sysConfigOld->getConfig("PrimitiveServers", "ColScanReadAheadBlocks"); PrefetchThreshold = sysConfigOld->getConfig("PrimitiveServers", "PrefetchThreshold"); MaxOutstandingRequests = sysConfigOld->getConfig("JobList", "MaxOutstandingRequests"); PmMaxMemorySmallSide = sysConfigOld->getConfig("HashJoin", "PmMaxMemorySmallSide"); + ThreadPoolSize = sysConfigOld->getConfig("JobList", "ThreadPoolSize"); } catch(...) {} @@ -1899,6 +1902,7 @@ int main(int argc, char *argv[]) sysConfigNew->setConfig("PrimitiveServers", "PrefetchThreshold", PrefetchThreshold); sysConfigNew->setConfig("JobList", "MaxOutstandingRequests", MaxOutstandingRequests); sysConfigNew->setConfig("HashJoin", "PmMaxMemorySmallSide", PmMaxMemorySmallSide); + sysConfigNew->setConfig("JobList", "ThreadPoolSize", ThreadPoolSize); } catch(...) {} From ccd9a414eb516e020fb74e042c90ed0128ffb890 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Tue, 16 Oct 2018 17:55:41 +0100 Subject: [PATCH 2/4] MCOL-1810 Fix hang on low core count Anything that links against joblist will spin up a threadpool upon startup. This includes the tools setConfig/getConfig. It is possible on a low core count machine or low CPU speed that the signal to the prune thread to shutdown is sent before the thread has completed startup when these quick-running tools are used. This fix adds a mutex so that spin up and shutdown can't happen at the same time as well as a stop watch in case we are shutting down when either the thread is running or we haven't fully started. --- utils/threadpool/threadpool.cpp | 4 ++++ utils/threadpool/threadpool.h | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/utils/threadpool/threadpool.cpp b/utils/threadpool/threadpool.cpp index 0b1546e98..602f868c4 100644 --- a/utils/threadpool/threadpool.cpp +++ b/utils/threadpool/threadpool.cpp @@ -55,6 +55,7 @@ ThreadPool::~ThreadPool() throw() { try { + boost::mutex::scoped_lock initLock(fInitMutex); stop(); } catch (...) @@ -64,6 +65,7 @@ ThreadPool::~ThreadPool() throw() void ThreadPool::init() { + boost::mutex::scoped_lock initLock(fInitMutex); fThreadCount = 0; fGeneralErrors = 0; fFunctorErrors = 0; @@ -89,6 +91,8 @@ void ThreadPool::pruneThread() while(true) { boost::system_time timeout = boost::get_system_time() + boost::posix_time::minutes(1); + if (fStop) + return; if (!fPruneThreadEnd.timed_wait(fPruneMutex, timeout)) { while(!fPruneThreads.empty()) diff --git a/utils/threadpool/threadpool.h b/utils/threadpool/threadpool.h index 1f0c6d0aa..4cb703c07 100644 --- a/utils/threadpool/threadpool.h +++ b/utils/threadpool/threadpool.h @@ -331,8 +331,9 @@ private: uint32_t waitingFunctorsSize; uint64_t fNextHandle; - std::string fName; // Optional to add a name to the pool for debugging. - bool fDebug; + std::string fName; // Optional to add a name to the pool for debugging. + bool fDebug; + boost::mutex fInitMutex; boost::mutex fPruneMutex; boost::condition fPruneThreadEnd; boost::thread* fPruneThread; From 65287a0613e11748e8bdb0ca5aac743acf1ac139 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 22 Oct 2018 17:56:49 +0100 Subject: [PATCH 3/4] MCOL-1826 Fix race in FLOAT/DOUBLE to string In the FLOAT/DOUBLE to string conversions a class global string was used to store the result. Unfortunately it is possible for an instance of this class to be used by multiple threads of PrimProc simultaneously. This would cause a race and data corruption or more likely a crash. This fix passes a string object from the caller to use instead. --- utils/funcexp/func_concat.cpp | 7 +++++-- utils/funcexp/func_concat_ws.cpp | 10 +++++++--- utils/funcexp/func_elt.cpp | 4 +++- utils/funcexp/func_insert.cpp | 8 +++++--- utils/funcexp/func_repeat.cpp | 4 +++- utils/funcexp/func_reverse.cpp | 3 ++- utils/funcexp/functor_str.h | 8 +++----- 7 files changed, 28 insertions(+), 16 deletions(-) diff --git a/utils/funcexp/func_concat.cpp b/utils/funcexp/func_concat.cpp index 744b1c8ea..dc07876cb 100644 --- a/utils/funcexp/func_concat.cpp +++ b/utils/funcexp/func_concat.cpp @@ -55,10 +55,13 @@ string Func_concat::getStrVal(Row& row, bool& isNull, CalpontSystemCatalog::ColType&) { - string ret = stringValue(parm[0], row, isNull); + string ret; + string tmp; + stringValue(parm[0], row, isNull, ret); for ( unsigned int id = 1 ; id < parm.size() ; id++) { - ret.append( stringValue(parm[id], row, isNull) ); + stringValue(parm[id], row, isNull, tmp); + ret.append(tmp); } return ret; diff --git a/utils/funcexp/func_concat_ws.cpp b/utils/funcexp/func_concat_ws.cpp index 7a236cfea..2718f20b3 100644 --- a/utils/funcexp/func_concat_ws.cpp +++ b/utils/funcexp/func_concat_ws.cpp @@ -49,7 +49,8 @@ string Func_concat_ws::getStrVal(Row& row, bool& isNull, CalpontSystemCatalog::ColType&) { - string delim = stringValue(parm[0], row, isNull); + string delim; + stringValue(parm[0], row, isNull, delim); if (isNull) return ""; @@ -62,7 +63,8 @@ string Func_concat_ws::getStrVal(Row& row, for ( unsigned int id = 1 ; id < parm.size() ; id++) { - string tstr = stringValue(parm[id], row, isNull); + string tstr; + stringValue(parm[id], row, isNull, tstr); if (isNull) { isNull = false; @@ -88,9 +90,11 @@ string Func_concat_ws::getStrVal(Row& row, #else string str; + string tmp; for ( uint32_t i = 1 ; i < parm.size() ; i++) { - str += string(stringValue(parm[i], row, isNull).c_str()); + string(stringValue(parm[i], row, isNull).c_str(), tmp); + str += tmp; if (isNull) { diff --git a/utils/funcexp/func_elt.cpp b/utils/funcexp/func_elt.cpp index 521fc5ea5..4bdda1e1f 100644 --- a/utils/funcexp/func_elt.cpp +++ b/utils/funcexp/func_elt.cpp @@ -95,7 +95,9 @@ string Func_elt::getStrVal(rowgroup::Row& row, return ""; } - return stringValue(parm[number], row, isNull); + std::string ret; + stringValue(parm[number], row, isNull, ret); + return ret; } diff --git a/utils/funcexp/func_insert.cpp b/utils/funcexp/func_insert.cpp index 519f860bc..1c6750e69 100644 --- a/utils/funcexp/func_insert.cpp +++ b/utils/funcexp/func_insert.cpp @@ -86,12 +86,14 @@ std::string Func_insert::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool& isNull, execplan::CalpontSystemCatalog::ColType&) -{ - const string& tstr = stringValue(fp[0], row, isNull); +{ + string tstr; + stringValue(fp[0], row, isNull, tstr); if (isNull) return ""; - const string& tnewstr = stringValue(fp[3], row, isNull); + string tnewstr; + stringValue(fp[3], row, isNull, tnewstr); if (isNull) return ""; diff --git a/utils/funcexp/func_repeat.cpp b/utils/funcexp/func_repeat.cpp index b962e6690..e08cb1fb4 100644 --- a/utils/funcexp/func_repeat.cpp +++ b/utils/funcexp/func_repeat.cpp @@ -61,7 +61,9 @@ std::string Func_repeat::getStrVal(rowgroup::Row& row, bool& isNull, execplan::CalpontSystemCatalog::ColType& op_ct) { - string str = stringValue(fp[0], row, isNull); + string str; + + stringValue(fp[0], row, isNull, str); if (str.empty() || str == "") return ""; diff --git a/utils/funcexp/func_reverse.cpp b/utils/funcexp/func_reverse.cpp index 41ffbb198..1ae8b9f68 100644 --- a/utils/funcexp/func_reverse.cpp +++ b/utils/funcexp/func_reverse.cpp @@ -65,7 +65,8 @@ std::string Func_reverse::getStrVal(rowgroup::Row& row, bool& isNull, execplan::CalpontSystemCatalog::ColType&) { - string str = stringValue(fp[0], row, isNull); + string str; + stringValue(fp[0], row, isNull, str); // We used to reverse in the string buffer, but that doesn't // work for all compilers as some re-use the buffer on simple diff --git a/utils/funcexp/functor_str.h b/utils/funcexp/functor_str.h index e3c97cca8..e54511b36 100644 --- a/utils/funcexp/functor_str.h +++ b/utils/funcexp/functor_str.h @@ -85,7 +85,7 @@ public: protected: - const std::string& stringValue(execplan::SPTP& fp, rowgroup::Row& row, bool& isNull) + const void stringValue(execplan::SPTP& fp, rowgroup::Row& row, bool& isNull, std::string& fFloatStr) { // Bug3788, use the shorter of fixed or scientific notation for floating point values. // [ the default format in treenode.h is fixed-point notation ] @@ -103,7 +103,8 @@ protected: break; default: - return fp->data()->getStrVal(row, isNull); + fFloatStr = fp->data()->getStrVal(row, isNull); + return; break; } exponent = (int)floor(log10( fabs(floatVal))); @@ -121,10 +122,7 @@ protected: fFloatStr += buf; } - return fFloatStr; } - - std::string fFloatStr; }; From a68caad9a20173656c8e941d9f32306b18f80eb4 Mon Sep 17 00:00:00 2001 From: Patrick LeBlanc Date: Mon, 5 Nov 2018 13:33:56 -0600 Subject: [PATCH 4/4] MCOL-1654. Querystats table creation broken. Escaped the ` chars to make the shell happy. --- dbcon/mysql/install_calpont_mysql.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbcon/mysql/install_calpont_mysql.sh b/dbcon/mysql/install_calpont_mysql.sh index e8eb5b2b0..f82ef3d2a 100755 --- a/dbcon/mysql/install_calpont_mysql.sh +++ b/dbcon/mysql/install_calpont_mysql.sh @@ -98,7 +98,7 @@ CREATE TABLE IF NOT EXISTS infinidb_querystats.querystats query varchar(8000), startTime timestamp NOT NULL, endTime timestamp NOT NULL, - `rows` bigint, + \`rows\` bigint, errno int, phyIO bigint, cacheIO bigint,