From fc5113e1fcfc1744abfc81fe48d6ea50e4b50ba8 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Thu, 26 Aug 2021 22:25:44 +0200 Subject: [PATCH 1/2] Fix a rare shutdown race in RocksDBShaCalculatorThread --- CHANGELOG | 2 ++ arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp | 8 ++++++++ arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.h | 4 +++- arangod/RocksDBEngine/RocksDBEngine.cpp | 6 +++++- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 71f8f96efdb4..5772c46085a3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,8 @@ devel ----- +* Fix a rare shutdown race in RocksDBShaCalculatorThread. + * Updated ArangoDB Starter to 0.15.2-preview-1. * Reduce internal priority of AQL execution. This prevents possible deadlocks diff --git a/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp b/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp index 2c107c18f314..9650348a3861 100644 --- a/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp +++ b/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp @@ -269,11 +269,19 @@ RocksDBShaCalculator::RocksDBShaCalculator(application_features::ApplicationServ /// @brief Shutdown the background thread only if it was ever started RocksDBShaCalculator::~RocksDBShaCalculator() { + waitForShutdown(); +} + +void RocksDBShaCalculator::waitForShutdown() { + // send shutdown signal to SHA thread + _shaThread.beginShutdown(); + _shaThread.signalLoop(); CONDITION_LOCKER(locker, _threadDone); if (_shaThread.isRunning()) { _threadDone.wait(); } + // now we are sure the SHA thread is not running anymore } void RocksDBShaCalculator::OnFlushCompleted(rocksdb::DB* db, diff --git a/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.h b/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.h index f9ab7014f9fc..904bc2dac926 100644 --- a/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.h +++ b/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.h @@ -88,7 +88,9 @@ class RocksDBShaCalculator : public rocksdb::EventListener { void OnCompactionCompleted(rocksdb::DB* db, const rocksdb::CompactionJobInfo& ci) override; - void beginShutdown() { _shaThread.beginShutdown(); }; + void beginShutdown() { _shaThread.beginShutdown(); } + + void waitForShutdown(); protected: /// thread to perform sha256 and file deletes in background diff --git a/arangod/RocksDBEngine/RocksDBEngine.cpp b/arangod/RocksDBEngine/RocksDBEngine.cpp index 9e5cac0caac8..7ec438b0d314 100644 --- a/arangod/RocksDBEngine/RocksDBEngine.cpp +++ b/arangod/RocksDBEngine/RocksDBEngine.cpp @@ -934,7 +934,7 @@ void RocksDBEngine::beginShutdown() { // signal the event listener that we are going to shut down soon if (_shaListener != nullptr) { _shaListener->beginShutdown(); - } // if + } } void RocksDBEngine::stop() { @@ -943,6 +943,10 @@ void RocksDBEngine::stop() { // in case we missed the beginShutdown somehow, call it again replicationManager()->beginShutdown(); replicationManager()->dropAll(); + + if (_shaListener != nullptr) { + _shaListener->waitForShutdown(); + } if (_backgroundThread) { // stop the press From 9b54903007366bb7c4b0c404d140e114db9c12a8 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Mon, 30 Aug 2021 10:16:26 +0200 Subject: [PATCH 2/2] added assertion as suggested --- arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp b/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp index 9650348a3861..ee22adc09f19 100644 --- a/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp +++ b/arangod/RocksDBEngine/Listeners/RocksDBShaCalculator.cpp @@ -269,6 +269,9 @@ RocksDBShaCalculator::RocksDBShaCalculator(application_features::ApplicationServ /// @brief Shutdown the background thread only if it was ever started RocksDBShaCalculator::~RocksDBShaCalculator() { + // when we get here the background thread should have been stopped + // already. + TRI_ASSERT(!_shaThread.isRunning()); waitForShutdown(); }