From 5c1ba3716a22c172efbe5bc9a75b97de137cd4b8 Mon Sep 17 00:00:00 2001 From: Ionatan Wiznia Date: Mon, 2 Dec 2024 12:52:44 -0300 Subject: [PATCH 1/2] Rollback transaction on DB plugin read query --- plugins/DB.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/DB.cpp b/plugins/DB.cpp index a14319dc7..bd1d92f0e 100644 --- a/plugins/DB.cpp +++ b/plugins/DB.cpp @@ -72,6 +72,10 @@ bool BedrockDBCommand::peek(SQLite& db) { return false; } + // We rollback here because if we are in a transaction and the querytakes long (which the queries in this command can) + // it prevents sqlite from checkpointing and if we accumulate a lot of things to checkpoint, things become slow + ((SQLite&) db).rollback(); + // Attempt the read-only query SQResult result; if (!db.read(query, result)) { From 450a2581ae2814827a529894a2939c4423e73225 Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Mon, 2 Dec 2024 15:13:01 -0300 Subject: [PATCH 2/2] reverting changes to use normal thread --- BedrockServer.cpp | 11 ++++----- libstuff/ResourceMonitorThread.cpp | 26 -------------------- libstuff/ResourceMonitorThread.h | 31 ------------------------ sqlitecluster/SQLiteClusterMessenger.cpp | 3 +-- sqlitecluster/SQLiteNode.cpp | 3 +-- 5 files changed, 7 insertions(+), 67 deletions(-) delete mode 100644 libstuff/ResourceMonitorThread.cpp delete mode 100644 libstuff/ResourceMonitorThread.h diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 776758dbb..ea610213e 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include @@ -118,9 +117,9 @@ void BedrockServer::sync() // our worker threads now. We don't wait until the node is `LEADING` or `FOLLOWING`, as it's state can change while // it's running, and our workers will have to maintain awareness of that state anyway. SINFO("Starting " << workerThreads << " worker threads."); - list workerThreadList; + list workerThreadList; for (int threadId = 0; threadId < workerThreads; threadId++) { - workerThreadList.emplace_back([this, threadId](){this->worker(threadId);}); + workerThreadList.emplace_back(&BedrockServer::worker, this, threadId); } // Now we jump into our main command processing loop. @@ -1339,7 +1338,7 @@ BedrockServer::BedrockServer(const SData& args_) // Start the sync thread, which will start the worker threads. SINFO("Launching sync thread '" << _syncThreadName << "'"); - _syncThread = ResourceMonitorThread(&BedrockServer::syncWrapper, this); + _syncThread = thread(&BedrockServer::syncWrapper, this); } BedrockServer::~BedrockServer() { @@ -1909,7 +1908,7 @@ void BedrockServer::_control(unique_ptr& command) { if (__quiesceThread) { response.methodLine = "400 Already Blocked"; } else { - __quiesceThread = new ResourceMonitorThread([&]() { + __quiesceThread = new thread([&]() { shared_ptr dbPoolCopy = _dbPool; if (dbPoolCopy) { SQLiteScopedHandle dbScope(*_dbPool, _dbPool->getIndex()); @@ -2139,7 +2138,7 @@ void BedrockServer::_acceptSockets() { bool threadStarted = false; while (!threadStarted) { try { - t = ResourceMonitorThread(&BedrockServer::handleSocket, this, move(socket), port == _controlPort, port == _commandPortPublic, port == _commandPortPrivate); + t = thread(&BedrockServer::handleSocket, this, move(socket), port == _controlPort, port == _commandPortPublic, port == _commandPortPrivate); threadStarted = true; } catch (const system_error& e) { // We don't care about this lock here from a performance perspective, it only happens when we diff --git a/libstuff/ResourceMonitorThread.cpp b/libstuff/ResourceMonitorThread.cpp deleted file mode 100644 index a9963892a..000000000 --- a/libstuff/ResourceMonitorThread.cpp +++ /dev/null @@ -1,26 +0,0 @@ -#include "ResourceMonitorThread.h" -#include "libstuff/libstuff.h" -#include -#include - -thread_local uint64_t ResourceMonitorThread::threadStartTime; -thread_local double ResourceMonitorThread::cpuStartTime; - -void ResourceMonitorThread::beforeProcessStart() { - threadStartTime = STimeNow(); - cpuStartTime = SGetCPUUserTime(); -} - -void ResourceMonitorThread::afterProcessFinished() { - const uint64_t threadUserTime = STimeNow() - threadStartTime; - const double cpuUserTime = SGetCPUUserTime() - cpuStartTime; - - // This shouldn't happen since the time to start/finish a thread should take more than a microsecond, but to be - // sure we're not dividing by 0 and causing crashes, let's add an if here and return if threadEndTime is 0. - if (threadUserTime == 0) { - return; - } - const double cpuUserPercentage = round((cpuUserTime / static_cast(threadUserTime)) * 100 * 1000) / 1000; - const pid_t tid = syscall(SYS_gettid); - SINFO(format("Thread finished. pID: '{}', CPUTime: '{}µs', CPUPercentage: '{}%'", tid, cpuUserTime, cpuUserPercentage)); -} diff --git a/libstuff/ResourceMonitorThread.h b/libstuff/ResourceMonitorThread.h deleted file mode 100644 index 5bb7e6896..000000000 --- a/libstuff/ResourceMonitorThread.h +++ /dev/null @@ -1,31 +0,0 @@ -#include "libstuff/libstuff.h" -#include - -using namespace std; - -// This class is a wrapper around the default thread. We use it to collect the thread CPU usage. That allows us -// to investigate if we have any threads using more resources than it should, which can cause CPU usage peaks in -// the cluster. -class ResourceMonitorThread : public thread -{ -public: - // When calling this constructor, if you're passing a class member function as the `f` parameter and that - // function receives parameters, you will need to wrap your function call in a lambda, doing something like: - // ResourceMonitorThread([=, this]{ this->memberFunction(param1, param2);}); - template - ResourceMonitorThread(F&& f, Args&&... args): - thread(ResourceMonitorThread::wrapper, forward(f), forward(args)...){}; -private: - thread_local static uint64_t threadStartTime; - thread_local static double cpuStartTime; - - static void beforeProcessStart(); - static void afterProcessFinished(); - - template - static void wrapper(F&& f, Args&&... args) { - beforeProcessStart(); - invoke(forward(f), forward(args)...); - afterProcessFinished(); - } -}; \ No newline at end of file diff --git a/sqlitecluster/SQLiteClusterMessenger.cpp b/sqlitecluster/SQLiteClusterMessenger.cpp index 80f4eb46b..502c05abe 100644 --- a/sqlitecluster/SQLiteClusterMessenger.cpp +++ b/sqlitecluster/SQLiteClusterMessenger.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include #include @@ -70,7 +69,7 @@ SQLiteClusterMessenger::WaitForReadyResult SQLiteClusterMessenger::waitForReady( } vector SQLiteClusterMessenger::runOnAll(const SData& cmd) { - list threads; + list threads; const list peerInfo = _node->getPeerInfo(); vector results(peerInfo.size()); atomic index = 0; diff --git a/sqlitecluster/SQLiteNode.cpp b/sqlitecluster/SQLiteNode.cpp index b55104160..79be57fed 100644 --- a/sqlitecluster/SQLiteNode.cpp +++ b/sqlitecluster/SQLiteNode.cpp @@ -4,7 +4,6 @@ #include #include -#include #include #include #include @@ -1507,7 +1506,7 @@ void SQLiteNode::_onMESSAGE(SQLitePeer* peer, const SData& message) { } else { _pendingSynchronizeResponses++; static atomic synchronizeCount(0); - ResourceMonitorThread([message, peer, currentSynchronizeCount = synchronizeCount++, this] () { + thread([message, peer, currentSynchronizeCount = synchronizeCount++, this] () { SInitialize("synchronize" + to_string(currentSynchronizeCount)); SData response("SYNCHRONIZE_RESPONSE"); SQLiteScopedHandle dbScope(*_dbPool, _dbPool->getIndex());