diff --git a/arangod/Agency/Agent.cpp b/arangod/Agency/Agent.cpp index d72304b2ae..f42eb98f06 100644 --- a/arangod/Agency/Agent.cpp +++ b/arangod/Agency/Agent.cpp @@ -58,7 +58,6 @@ std::string const NO_LEADER(""); /// Agent configuration Agent::Agent(ApplicationServer& server, config_t const& config) : Thread(server, "Agent"), - _server(server), _constituent(server), _supervision(server), _config(config), @@ -79,9 +78,6 @@ Agent::Agent(ApplicationServer& server, config_t const& config) } } -/// @brief the underlying application server -application_features::ApplicationServer& Agent::server() { return _server; } - /// This agent's id std::string Agent::id() const { return _config.id(); } diff --git a/arangod/Agency/Agent.h b/arangod/Agency/Agent.h index d85fa745c8..885d7929d2 100644 --- a/arangod/Agency/Agent.h +++ b/arangod/Agency/Agent.h @@ -51,9 +51,6 @@ class Agent final : public arangodb::Thread, public AgentInterface { /// @brief Clean up ~Agent(); - /// @brief the underlying application server - application_features::ApplicationServer& server(); - /// @brief bring down threads, can be called multiple times. void waitForThreadsStop(); @@ -332,9 +329,6 @@ class Agent final : public arangodb::Thread, public AgentInterface { /// @brief Find out, if we've had acknowledged RPCs recent enough bool challengeLeadership(); - /// @brief underlying application server - application_features::ApplicationServer& _server; - /// @brief Leader election delegate Constituent _constituent; diff --git a/arangod/Agency/Constituent.h b/arangod/Agency/Constituent.h index 248a6be905..883e3b7a56 100644 --- a/arangod/Agency/Constituent.h +++ b/arangod/Agency/Constituent.h @@ -52,7 +52,7 @@ class Agent; // RAFT leader election class Constituent : public Thread { public: - Constituent(application_features::ApplicationServer&); + explicit Constituent(application_features::ApplicationServer&); // clean up and exit election virtual ~Constituent(); diff --git a/arangod/Agency/Supervision.h b/arangod/Agency/Supervision.h index d9cbc9a69c..37e2babe7a 100644 --- a/arangod/Agency/Supervision.h +++ b/arangod/Agency/Supervision.h @@ -90,7 +90,7 @@ class Supervision : public arangodb::CriticalThread { }; /// @brief Construct sanity checking - Supervision(application_features::ApplicationServer& server); + explicit Supervision(application_features::ApplicationServer& server); /// @brief Default dtor ~Supervision(); diff --git a/arangod/Aql/BlocksWithClients.cpp b/arangod/Aql/BlocksWithClients.cpp index 944af42cd7..a2ae468ee3 100644 --- a/arangod/Aql/BlocksWithClients.cpp +++ b/arangod/Aql/BlocksWithClients.cpp @@ -77,7 +77,9 @@ BlocksWithClients::BlocksWithClients(ExecutionEngine* engine, ExecutionNode cons } std::pair BlocksWithClients::getBlock(size_t atMost) { - throwIfKilled(); // check if we were aborted + if (_engine->getQuery()->killed()) { + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } auto res = _dependencies[0]->getSome(atMost); if (res.first == ExecutionState::WAITING) { @@ -127,11 +129,6 @@ size_t BlocksWithClients::getClientId(std::string const& shardId) const { return it->second; } -void BlocksWithClients::throwIfKilled() { - if (_engine->getQuery()->killed()) { - THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); - } -} std::pair BlocksWithClients::getSome(size_t) { TRI_ASSERT(false); THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED); diff --git a/arangod/Aql/BlocksWithClients.h b/arangod/Aql/BlocksWithClients.h index 0a6fbc540f..8e5825775e 100644 --- a/arangod/Aql/BlocksWithClients.h +++ b/arangod/Aql/BlocksWithClients.h @@ -87,9 +87,6 @@ class BlocksWithClients : public ExecutionBlock { /// corresponding to size_t getClientId(std::string const& shardId) const; - /// @brief throw an exception if query was killed - void throwIfKilled(); - /// @brief _shardIdMap: map from shardIds to clientNrs std::unordered_map _shardIdMap; diff --git a/arangod/Aql/ExpressionContext.h b/arangod/Aql/ExpressionContext.h index e346df6aac..52e7c0f165 100644 --- a/arangod/Aql/ExpressionContext.h +++ b/arangod/Aql/ExpressionContext.h @@ -64,7 +64,6 @@ class ExpressionContext { transaction::Methods*, bool& isEmptyExpression) = 0; - virtual bool killed() const = 0; virtual TRI_vocbase_t& vocbase() const = 0; virtual Query* query() const = 0; }; diff --git a/arangod/Aql/Functions.cpp b/arangod/Aql/Functions.cpp index d7088b2d5b..b2589f021f 100644 --- a/arangod/Aql/Functions.cpp +++ b/arangod/Aql/Functions.cpp @@ -4186,19 +4186,21 @@ AqlValue Functions::Sleep(ExpressionContext* expressionContext, return AqlValue(AqlValueHintNull()); } - double now = TRI_microtime(); - double const until = now + value.toDouble(); auto& server = application_features::ApplicationServer::server(); - while (now < until) { - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + double const sleepValue = value.toDouble(); + auto now = std::chrono::steady_clock::now(); + auto const endTime = now + std::chrono::milliseconds(static_cast(sleepValue * 1000.0)); - if (expressionContext->killed()) { + while (now < endTime) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + + if (expressionContext->query()->killed()) { THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } else if (server.isStopping()) { THROW_ARANGO_EXCEPTION(TRI_ERROR_SHUTTING_DOWN); } - now = TRI_microtime(); + now = std::chrono::steady_clock::now(); } return AqlValue(AqlValueHintNull()); } diff --git a/arangod/Aql/QueryExpressionContext.cpp b/arangod/Aql/QueryExpressionContext.cpp index d26f065e39..57335b9cdc 100644 --- a/arangod/Aql/QueryExpressionContext.cpp +++ b/arangod/Aql/QueryExpressionContext.cpp @@ -54,8 +54,6 @@ icu::RegexMatcher* QueryExpressionContext::buildSplitMatcher(AqlValue splitExpre return _query->regexCache()->buildSplitMatcher(splitExpression, trx, isEmptyExpression); } -bool QueryExpressionContext::killed() const { return _query->killed(); } - TRI_vocbase_t& QueryExpressionContext::vocbase() const { return _query->vocbase(); } diff --git a/arangod/Aql/QueryExpressionContext.h b/arangod/Aql/QueryExpressionContext.h index 3c8f48c086..863f617b7c 100644 --- a/arangod/Aql/QueryExpressionContext.h +++ b/arangod/Aql/QueryExpressionContext.h @@ -45,7 +45,6 @@ class QueryExpressionContext : public ExpressionContext { icu::RegexMatcher* buildSplitMatcher(AqlValue splitExpression, transaction::Methods*, bool& isEmptyExpression) override; - bool killed() const override final; TRI_vocbase_t& vocbase() const override final; Query* query() const override final; diff --git a/arangod/Cluster/ClusterComm.h b/arangod/Cluster/ClusterComm.h index 4b90ee602a..0c535c06c7 100644 --- a/arangod/Cluster/ClusterComm.h +++ b/arangod/Cluster/ClusterComm.h @@ -466,7 +466,7 @@ class ClusterComm { ////////////////////////////////////////////////////////////////////////////// ClusterComm(application_features::ApplicationServer&); - ClusterComm(ClusterComm const&); // not implemented + explicit ClusterComm(ClusterComm const&); // not implemented void operator=(ClusterComm const&); // not implemented ////////////////////////////////////////////////////////////////////////////// @@ -717,7 +717,7 @@ class ClusterCommThread : public Thread { ClusterCommThread& operator=(ClusterCommThread const&); public: - ClusterCommThread(application_features::ApplicationServer&); + explicit ClusterCommThread(application_features::ApplicationServer&); ~ClusterCommThread(); public: void beginShutdown() override; diff --git a/arangod/Pregel/Recovery.h b/arangod/Pregel/Recovery.h index 7a48d3ae6a..82d08568be 100644 --- a/arangod/Pregel/Recovery.h +++ b/arangod/Pregel/Recovery.h @@ -52,7 +52,7 @@ class RecoveryManager { void _renewPrimaryServer(ShardID const& shard); public: - RecoveryManager(ClusterInfo&); + explicit RecoveryManager(ClusterInfo&); ~RecoveryManager(); void monitorCollections(DatabaseID const& database, diff --git a/arangod/Replication/ReplicationApplierConfiguration.h b/arangod/Replication/ReplicationApplierConfiguration.h index e2e99efc9c..60c1122415 100644 --- a/arangod/Replication/ReplicationApplierConfiguration.h +++ b/arangod/Replication/ReplicationApplierConfiguration.h @@ -81,7 +81,7 @@ class ReplicationApplierConfiguration { std::string _clientInfoString; public: - ReplicationApplierConfiguration(application_features::ApplicationServer&); + explicit ReplicationApplierConfiguration(application_features::ApplicationServer&); ~ReplicationApplierConfiguration() = default; ReplicationApplierConfiguration(ReplicationApplierConfiguration const&) = default; diff --git a/arangod/RestServer/DatabaseFeature.h b/arangod/RestServer/DatabaseFeature.h index 21b54b3b92..60bd8de153 100644 --- a/arangod/RestServer/DatabaseFeature.h +++ b/arangod/RestServer/DatabaseFeature.h @@ -57,7 +57,7 @@ class DatabaseManagerThread final : public Thread { DatabaseManagerThread(DatabaseManagerThread const&) = delete; DatabaseManagerThread& operator=(DatabaseManagerThread const&) = delete; - DatabaseManagerThread(application_features::ApplicationServer&); + explicit DatabaseManagerThread(application_features::ApplicationServer&); ~DatabaseManagerThread(); void run() override; diff --git a/arangod/RocksDBEngine/RocksDBEventListener.h b/arangod/RocksDBEngine/RocksDBEventListener.h index ab3f4046e0..8c5f755f68 100644 --- a/arangod/RocksDBEngine/RocksDBEventListener.h +++ b/arangod/RocksDBEngine/RocksDBEventListener.h @@ -79,7 +79,7 @@ protected: //////////////////////////////////////////////////////////////////////////////// class RocksDBEventListener : public rocksdb::EventListener { public: - RocksDBEventListener(application_features::ApplicationServer&); + explicit RocksDBEventListener(application_features::ApplicationServer&); virtual ~RocksDBEventListener(); void OnFlushCompleted(rocksdb::DB* db, const rocksdb::FlushJobInfo& flush_job_info) override; diff --git a/arangod/Statistics/StatisticsFeature.cpp b/arangod/Statistics/StatisticsFeature.cpp index 099f326e7d..81b6f44003 100644 --- a/arangod/Statistics/StatisticsFeature.cpp +++ b/arangod/Statistics/StatisticsFeature.cpp @@ -84,7 +84,8 @@ StatisticsDistribution TRI_TotalTimeDistributionStatistics(TRI_RequestTimeDistri class StatisticsThread final : public Thread { public: - StatisticsThread(ApplicationServer& server) : Thread(server, "Statistics") {} + explicit StatisticsThread(ApplicationServer& server) + : Thread(server, "Statistics") {} ~StatisticsThread() { shutdown(); } public: diff --git a/arangod/StorageEngine/HotBackup.h b/arangod/StorageEngine/HotBackup.h index f00dd17e5a..dd8a8360d8 100644 --- a/arangod/StorageEngine/HotBackup.h +++ b/arangod/StorageEngine/HotBackup.h @@ -44,7 +44,7 @@ enum BACKUP_ENGINE {ROCKSDB, MMFILES, CLUSTER}; class HotBackup { public: - HotBackup(application_features::ApplicationServer& server); + explicit HotBackup(application_features::ApplicationServer& server); virtual ~HotBackup() = default; /** diff --git a/lib/Basics/Thread.cpp b/lib/Basics/Thread.cpp index dfb835c2b2..7f669eb01a 100644 --- a/lib/Basics/Thread.cpp +++ b/lib/Basics/Thread.cpp @@ -158,13 +158,13 @@ std::string Thread::stringify(ThreadState state) { Thread::Thread(application_features::ApplicationServer& server, std::string const& name, bool deleteOnExit, std::uint32_t terminationTimeout) : _server(server), - _deleteOnExit(deleteOnExit), _threadStructInitialized(false), _refs(0), _name(name), _thread(), _threadNumber(0), _terminationTimeout(terminationTimeout), + _deleteOnExit(deleteOnExit), _finishedCondition(nullptr), _state(ThreadState::CREATED) { TRI_InitThread(&_thread); diff --git a/lib/Basics/Thread.h b/lib/Basics/Thread.h index b4fc569c14..37a58e49ca 100644 --- a/lib/Basics/Thread.h +++ b/lib/Basics/Thread.h @@ -89,13 +89,15 @@ class Thread { bool deleteOnExit = false, std::uint32_t terminationTimeout = INFINITE); virtual ~Thread(); - public: // whether or not the thread is allowed to start during prepare virtual bool isSystem() { return false; } /// @brief whether or not the thread is chatty on shutdown virtual bool isSilent() { return false; } + /// @brief the underlying application server + application_features::ApplicationServer& server() { return _server; } + /// @brief flags the thread as stopping /// Classes that override this function must ensure that they /// always call Thread::beginShutdown()! @@ -144,9 +146,6 @@ class Thread { /// be threadsafe! void shutdown(); - protected: - application_features::ApplicationServer& _server; - protected: /// @brief the thread program virtual void run() = 0; @@ -160,9 +159,11 @@ class Thread { void markAsStopped(); void runMe(); void releaseRef(); + + protected: + application_features::ApplicationServer& _server; private: - bool const _deleteOnExit; std::atomic _threadStructInitialized; std::atomic _refs; @@ -177,6 +178,8 @@ class Thread { // Failure to terminate within the specified time results in process abortion! // The default value is INFINITE, i.e., we want to wait forever instead of aborting the process. std::uint32_t _terminationTimeout; + + bool const _deleteOnExit; basics::ConditionVariable* _finishedCondition;