From af3c206d89297db32d74eb9ba17b72d7b27d560a Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Mon, 30 Jan 2017 22:32:05 +0100 Subject: [PATCH 01/22] Try to solve sporadic shutdown blockage in heartbeat thread. --- arangod/Cluster/HeartbeatThread.cpp | 71 ++++++++++++++--------------- arangod/Cluster/HeartbeatThread.h | 8 +--- 2 files changed, 37 insertions(+), 42 deletions(-) diff --git a/arangod/Cluster/HeartbeatThread.cpp b/arangod/Cluster/HeartbeatThread.cpp index 3e70fb53da..589c660ef0 100644 --- a/arangod/Cluster/HeartbeatThread.cpp +++ b/arangod/Cluster/HeartbeatThread.cpp @@ -98,35 +98,44 @@ HeartbeatThread::~HeartbeatThread() { shutdown(); } /// watching the command key, it will wake up and apply the change locally. //////////////////////////////////////////////////////////////////////////////// +class HeartbeatBackgroundJob { + std::shared_ptr _heartbeatThread; + public: + HeartbeatBackgroundJob(std::shared_ptr hbt) + : _heartbeatThread(hbt) {} + + void operator()() { + _heartbeatThread->runBackgroundJob(); + } +}; + +void HeartbeatThread::runBackgroundJob() { + uint64_t jobNr = ++_backgroundJobsLaunched; + LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "sync callback started " << jobNr; + { + DBServerAgencySync job(this); + job.work(); + } + LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "sync callback ended " << jobNr; + + { + MUTEX_LOCKER(mutexLocker, *_statusLock); + if (_launchAnotherBackgroundJob) { + LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "dispatching sync tail " + << ++_backgroundJobsPosted; + _launchAnotherBackgroundJob = false; + _ioService->post(HeartbeatBackgroundJob(shared_from_this())); + } else { + _backgroundJobScheduledOrRunning = false; + _launchAnotherBackgroundJob = false; + } + } +} + void HeartbeatThread::run() { if (ServerState::instance()->isCoordinator()) { runCoordinator(); } else { - // Set the member variable that holds a closure to run background - // jobs in JS: - auto self = shared_from_this(); - _backgroundJob = [self, this]() { - uint64_t jobNr = ++_backgroundJobsLaunched; - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "sync callback started " << jobNr; - { - DBServerAgencySync job(this); - job.work(); - } - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "sync callback ended " << jobNr; - - { - MUTEX_LOCKER(mutexLocker, *_statusLock); - if (_launchAnotherBackgroundJob) { - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "dispatching sync tail " - << ++_backgroundJobsPosted; - _launchAnotherBackgroundJob = false; - _ioService->post(_backgroundJob); - } else { - _backgroundJobScheduledOrRunning = false; - _launchAnotherBackgroundJob = false; - } - } - }; runDBServer(); } } @@ -338,16 +347,6 @@ void HeartbeatThread::runDBServer() { } _agencyCallbackRegistry->unregisterCallback(planAgencyCallback); - int count = 0; - while (++count < 3000) { - { - MUTEX_LOCKER(mutexLocker, *_statusLock); - if (!_backgroundJobScheduledOrRunning) { - break; - } - } - usleep(100000); - } LOG_TOPIC(TRACE, Logger::HEARTBEAT) << "stopped heartbeat thread (DBServer version)"; } @@ -764,7 +763,7 @@ void HeartbeatThread::syncDBServerStatusQuo() { LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "dispatching sync " << ++_backgroundJobsPosted; _backgroundJobScheduledOrRunning = true; - _ioService->post(_backgroundJob); + _ioService->post(HeartbeatBackgroundJob(shared_from_this())); } //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Cluster/HeartbeatThread.h b/arangod/Cluster/HeartbeatThread.h index 9e17cf8c4c..e02c008bd5 100644 --- a/arangod/Cluster/HeartbeatThread.h +++ b/arangod/Cluster/HeartbeatThread.h @@ -74,6 +74,8 @@ class HeartbeatThread : public Thread, void setReady() { _ready.store(true); } + void runBackgroundJob(); + void dispatchedJobResult(DBServerAgencySyncResult); ////////////////////////////////////////////////////////////////////////////// @@ -253,12 +255,6 @@ class HeartbeatThread : public Thread, ////////////////////////////////////////////////////////////////////////////// bool _launchAnotherBackgroundJob; - - ////////////////////////////////////////////////////////////////////////////// - /// @brief _backgroundJob, the closure that does the work - ////////////////////////////////////////////////////////////////////////////// - - std::function _backgroundJob; }; } From 4172b3e0e46f3cc0bf793a444846b4cd6aa76057 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Mon, 30 Jan 2017 22:54:00 +0100 Subject: [PATCH 02/22] turn down loglevel a bit --- arangod/Wal/LogfileManager.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/arangod/Wal/LogfileManager.cpp b/arangod/Wal/LogfileManager.cpp index 865eed1ca0..b4ec42e78c 100644 --- a/arangod/Wal/LogfileManager.cpp +++ b/arangod/Wal/LogfileManager.cpp @@ -876,7 +876,7 @@ int LogfileManager::flush(bool waitForSync, bool waitForCollector, res = this->waitForCollector(lastOpenLogfileId, maxWaitTime); if (res == TRI_ERROR_LOCK_TIMEOUT) { - LOG(ERR) << "got lock timeout when waiting for WAL flush. lastOpenLogfileId: " << lastOpenLogfileId; + LOG(DEBUG) << "got lock timeout when waiting for WAL flush. lastOpenLogfileId: " << lastOpenLogfileId; } } else if (res == TRI_ERROR_ARANGO_DATAFILE_EMPTY) { // current logfile is empty and cannot be collected @@ -887,7 +887,7 @@ int LogfileManager::flush(bool waitForSync, bool waitForCollector, res = this->waitForCollector(lastSealedLogfileId, maxWaitTime); if (res == TRI_ERROR_LOCK_TIMEOUT) { - LOG(ERR) << "got lock timeout when waiting for WAL flush. lastSealedLogfileId: " << lastSealedLogfileId; + LOG(DEBUG) << "got lock timeout when waiting for WAL flush. lastSealedLogfileId: " << lastSealedLogfileId; } } } @@ -1731,8 +1731,7 @@ int LogfileManager::waitForCollector(Logfile::IdType logfileId, // try again } - // TODO: remove debug info here - LOG(ERR) << "going into lock timeout. having waited for logfile: " << logfileId << ", maxWaitTime: " << maxWaitTime; + LOG(DEBUG) << "going into lock timeout. having waited for logfile: " << logfileId << ", maxWaitTime: " << maxWaitTime; logStatus(); // waited for too long @@ -1740,12 +1739,11 @@ int LogfileManager::waitForCollector(Logfile::IdType logfileId, } void LogfileManager::logStatus() { - // TODO: remove debug info here - LOG(ERR) << "logfile manager status report: lastCollectedId: " << _lastCollectedId.load() << ", lastSealedId: " << _lastSealedId.load(); + LOG(DEBUG) << "logfile manager status report: lastCollectedId: " << _lastCollectedId.load() << ", lastSealedId: " << _lastSealedId.load(); READ_LOCKER(locker, _logfilesLock); for (auto logfile : _logfiles) { - LOG(ERR) << "- logfile " << logfile.second->id() << ", filename '" << logfile.second->filename() - << "', status " << logfile.second->statusText(); + LOG(DEBUG) << "- logfile " << logfile.second->id() << ", filename '" << logfile.second->filename() + << "', status " << logfile.second->statusText(); } } From 16c19ad927dc274f2d22754a0f36bae233cfd889 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Mon, 30 Jan 2017 23:02:05 +0100 Subject: [PATCH 03/22] cppcheck --- arangod/Cluster/HeartbeatThread.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/Cluster/HeartbeatThread.cpp b/arangod/Cluster/HeartbeatThread.cpp index 589c660ef0..d0e05a7650 100644 --- a/arangod/Cluster/HeartbeatThread.cpp +++ b/arangod/Cluster/HeartbeatThread.cpp @@ -101,7 +101,7 @@ HeartbeatThread::~HeartbeatThread() { shutdown(); } class HeartbeatBackgroundJob { std::shared_ptr _heartbeatThread; public: - HeartbeatBackgroundJob(std::shared_ptr hbt) + explicit HeartbeatBackgroundJob(std::shared_ptr hbt) : _heartbeatThread(hbt) {} void operator()() { From 9525f46b2b0514881e2b361e79219530c6a1d993 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Mon, 30 Jan 2017 23:22:58 +0100 Subject: [PATCH 04/22] Some cleanup of comments for better understandability. --- arangod/Cluster/HeartbeatThread.cpp | 39 +++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/arangod/Cluster/HeartbeatThread.cpp b/arangod/Cluster/HeartbeatThread.cpp index d0e05a7650..17c2c873cb 100644 --- a/arangod/Cluster/HeartbeatThread.cpp +++ b/arangod/Cluster/HeartbeatThread.cpp @@ -86,16 +86,18 @@ HeartbeatThread::HeartbeatThread(AgencyCallbackRegistry* agencyCallbackRegistry, HeartbeatThread::~HeartbeatThread() { shutdown(); } //////////////////////////////////////////////////////////////////////////////// -/// @brief heartbeat main loop -/// the heartbeat thread constantly reports the current server status to the -/// agency. it does so by sending the current state string to the key -/// "Sync/ServerStates/" + my-id. -/// after transferring the current state to the agency, the heartbeat thread -/// will wait for changes on the "Sync/Commands/" + my-id key. If no changes -/// occur, -/// then the request it aborted and the heartbeat thread will go on with -/// reporting its state to the agency again. If it notices a change when -/// watching the command key, it will wake up and apply the change locally. +/// @brief running of heartbeat background jobs (in JavaScript), we run +/// these by instantiating an object in class HeartbeatBackgroundJob, +/// which is a std::function and holds a shared_ptr to the +/// HeartbeatThread singleton itself. This instance is then posted to +/// the io_service for execution in the thread pool. Should the heartbeat +/// thread itself terminate during shutdown, then the HeartbeatThread +/// singleton itself is still kept alive by the shared_ptr in the instance +/// of HeartbeatBackgroundJob. The operator() method simply calls the +/// runBackgroundJob() method of the heartbeat thread. Should this have +/// to schedule another background job, then it can simply create a new +/// HeartbeatBackgroundJob instance, again using shared_from_this() to +/// create a new shared_ptr keeping the HeartbeatThread object alive. //////////////////////////////////////////////////////////////////////////////// class HeartbeatBackgroundJob { @@ -109,6 +111,10 @@ class HeartbeatBackgroundJob { } }; +//////////////////////////////////////////////////////////////////////////////// +/// @brief method runBackgroundJob() +//////////////////////////////////////////////////////////////////////////////// + void HeartbeatThread::runBackgroundJob() { uint64_t jobNr = ++_backgroundJobsLaunched; LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "sync callback started " << jobNr; @@ -132,6 +138,19 @@ void HeartbeatThread::runBackgroundJob() { } } +//////////////////////////////////////////////////////////////////////////////// +/// @brief heartbeat main loop +/// the heartbeat thread constantly reports the current server status to the +/// agency. it does so by sending the current state string to the key +/// "Sync/ServerStates/" + my-id. +/// after transferring the current state to the agency, the heartbeat thread +/// will wait for changes on the "Sync/Commands/" + my-id key. If no changes +/// occur, +/// then the request it aborted and the heartbeat thread will go on with +/// reporting its state to the agency again. If it notices a change when +/// watching the command key, it will wake up and apply the change locally. +//////////////////////////////////////////////////////////////////////////////// + void HeartbeatThread::run() { if (ServerState::instance()->isCoordinator()) { runCoordinator(); From 91c97d301a91a8d4f4d1529c1dba1e66ce2d8332 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Mon, 30 Jan 2017 23:59:46 +0100 Subject: [PATCH 05/22] Lower timeout in query registry to 10min from 1h. --- arangod/Aql/ExecutionEngine.cpp | 2 +- arangod/Aql/QueryRegistry.h | 2 +- arangod/Aql/RestAqlHandler.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arangod/Aql/ExecutionEngine.cpp b/arangod/Aql/ExecutionEngine.cpp index 4507235bc5..58f79e8536 100644 --- a/arangod/Aql/ExecutionEngine.cpp +++ b/arangod/Aql/ExecutionEngine.cpp @@ -1042,7 +1042,7 @@ struct CoordinatorInstanciator : public WalkerWorker { id = TRI_NewTickServer(); try { - queryRegistry->insert(id, engine->getQuery(), 3600.0); + queryRegistry->insert(id, engine->getQuery(), 600.0); } catch (...) { delete engine->getQuery(); // This deletes the new query as well as the engine diff --git a/arangod/Aql/QueryRegistry.h b/arangod/Aql/QueryRegistry.h index e67bf9eaa2..071a985900 100644 --- a/arangod/Aql/QueryRegistry.h +++ b/arangod/Aql/QueryRegistry.h @@ -45,7 +45,7 @@ class QueryRegistry { /// a query for this and combination and an exception will /// be thrown in that case. The time to live is in seconds and the /// query will be deleted if it is not opened for that amount of time. - void insert(QueryId id, Query* query, double ttl = 3600.0); + void insert(QueryId id, Query* query, double ttl = 600.0); /// @brief open, find a query in the registry, if none is found, a nullptr /// is returned, otherwise, ownership of the query is transferred to the diff --git a/arangod/Aql/RestAqlHandler.cpp b/arangod/Aql/RestAqlHandler.cpp index 22c5ee0928..8e19b2cb5c 100644 --- a/arangod/Aql/RestAqlHandler.cpp +++ b/arangod/Aql/RestAqlHandler.cpp @@ -105,7 +105,7 @@ void RestAqlHandler::createQueryFromVelocyPack() { } // Now the query is ready to go, store it in the registry and return: - double ttl = 3600.0; + double ttl = 600.0; bool found; std::string const& ttlstring = _request->header("ttl", found); @@ -317,7 +317,7 @@ void RestAqlHandler::createQueryFromString() { } // Now the query is ready to go, store it in the registry and return: - double ttl = 3600.0; + double ttl = 600.0; bool found; std::string const& ttlstring = _request->header("ttl", found); From f4a521eebfde476decceb0beef25f89599aa4199 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 31 Jan 2017 08:36:36 +0100 Subject: [PATCH 06/22] updated CHANGELOG --- CHANGELOG | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index c99b3244ad..c0f3e96983 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,12 @@ devel * changed index filling to make it more parallel, dispatch tasks to boost::asio +* updated versions of bundled node modules: + - joi: from 8.4.2 to 9.2.0 + - joi-to-json-schema: from 2.2.0 to 2.3.0 + - sinon: from 1.17.4 to 1.17.6 + - lodash: from 4.13.1 to 4.16.6 + * added shortcut for AQL ternary operator instead of `condition ? true-part : false-part` it is now possible to also use a shortcut variant `condition ? : false-part`, e.g. From 8c1441740a41134c12833ba92c2c741fc51e6cff Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 31 Jan 2017 08:36:52 +0100 Subject: [PATCH 07/22] removed libev --- LICENSES-OTHER-COMPONENTS.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/LICENSES-OTHER-COMPONENTS.md b/LICENSES-OTHER-COMPONENTS.md index 4fb9f1f10c..adaabcff75 100644 --- a/LICENSES-OTHER-COMPONENTS.md +++ b/LICENSES-OTHER-COMPONENTS.md @@ -51,11 +51,6 @@ * Project Home: http://site.icu-project.org/ * License: [ICU License](http://source.icu-project.org/repos/icu/icu/trunk/license.html) -### libev 4.11 - -* Project Home: http://software.schmorp.de/pkg/libev.html -* License: Dual-License [BSD-style 2-Clause License](http://cvs.schmorp.de/libev/LICENSE?revision=1.11&view=markup) - ### linenoise-ng * GitHub: https://github.com/arangodb/linenoise-ng From 5d267037f46924df65892116e3cbfbd86f034257 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 31 Jan 2017 08:47:25 +0100 Subject: [PATCH 08/22] updated CHANGELOG --- CHANGELOG | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index c0f3e96983..b2dd39ddc5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -80,6 +80,13 @@ edge attribute `label`. * added option -D to define a configuration file environment key=value +* changed encoding behavior for URLs encoded in the C++ code of ArangoDB: + previously the special characters `-`, `_`, `~` and `.` were returned as-is + after URL-encoding, now `.` will be encoded to be `%2e`. + This also changes the behavior of how incoming URIs are processed: previously + occurrences of `..` in incoming request URIs were collapsed (e.g. `a/../b/` was + collapsed to a plain `b/`). Now `..` in incoming request URIs are not collapsed. + * Foxx request URL suffix is no longer unescaped * @arangodb/request option json now defaults to `true` if the response body is not empty and encoding is not explicitly set to `null` (binary). From fe130289090b95e8cb765d716e4150592f8f57fd Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 31 Jan 2017 08:50:09 +0100 Subject: [PATCH 09/22] removed unused file --- Documentation/DocuBlocks/schedulerBackend.md | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 Documentation/DocuBlocks/schedulerBackend.md diff --git a/Documentation/DocuBlocks/schedulerBackend.md b/Documentation/DocuBlocks/schedulerBackend.md deleted file mode 100644 index 541ae1d21c..0000000000 --- a/Documentation/DocuBlocks/schedulerBackend.md +++ /dev/null @@ -1,10 +0,0 @@ - - -@brief scheduler backend -`--scheduler.backend arg` - -The I/O method used by the event handler. The default (if this option is -not specified) is to try all recommended backends. This is platform -specific. See libev for further details and the meaning of select, poll -and epoll. - From d8d8ef9755493732dd8eb5e9f8aa00cab12936b0 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Tue, 31 Jan 2017 08:56:19 +0100 Subject: [PATCH 10/22] Add an assertion. --- arangod/Cluster/HeartbeatThread.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/arangod/Cluster/HeartbeatThread.cpp b/arangod/Cluster/HeartbeatThread.cpp index 17c2c873cb..d38b72c200 100644 --- a/arangod/Cluster/HeartbeatThread.cpp +++ b/arangod/Cluster/HeartbeatThread.cpp @@ -126,9 +126,10 @@ void HeartbeatThread::runBackgroundJob() { { MUTEX_LOCKER(mutexLocker, *_statusLock); + TRI_assert(_backgroundJobScheduledOrRunning); if (_launchAnotherBackgroundJob) { - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "dispatching sync tail " - << ++_backgroundJobsPosted; + jobNr = ++_backgroundJobsPosted; + LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "dispatching sync tail " << jobNr; _launchAnotherBackgroundJob = false; _ioService->post(HeartbeatBackgroundJob(shared_from_this())); } else { @@ -779,8 +780,8 @@ void HeartbeatThread::syncDBServerStatusQuo() { } // schedule a job for the change: - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "dispatching sync " - << ++_backgroundJobsPosted; + uint64_t jobNr = ++_backgroundJobsPosted; + LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "dispatching sync " << jobNr; _backgroundJobScheduledOrRunning = true; _ioService->post(HeartbeatBackgroundJob(shared_from_this())); } From d8171651bf5979620099469c3a0b452ef808c0d8 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Tue, 31 Jan 2017 08:59:20 +0100 Subject: [PATCH 11/22] Fix assert. --- arangod/Cluster/HeartbeatThread.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arangod/Cluster/HeartbeatThread.cpp b/arangod/Cluster/HeartbeatThread.cpp index d38b72c200..972829eec0 100644 --- a/arangod/Cluster/HeartbeatThread.cpp +++ b/arangod/Cluster/HeartbeatThread.cpp @@ -117,19 +117,19 @@ class HeartbeatBackgroundJob { void HeartbeatThread::runBackgroundJob() { uint64_t jobNr = ++_backgroundJobsLaunched; - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "sync callback started " << jobNr; + LOG_TOPIC(INFO, Logger::HEARTBEAT) << "sync callback started " << jobNr; { DBServerAgencySync job(this); job.work(); } - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "sync callback ended " << jobNr; + LOG_TOPIC(INFO, Logger::HEARTBEAT) << "sync callback ended " << jobNr; { MUTEX_LOCKER(mutexLocker, *_statusLock); - TRI_assert(_backgroundJobScheduledOrRunning); + TRI_ASSERT(_backgroundJobScheduledOrRunning); if (_launchAnotherBackgroundJob) { jobNr = ++_backgroundJobsPosted; - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "dispatching sync tail " << jobNr; + LOG_TOPIC(INFO, Logger::HEARTBEAT) << "dispatching sync tail " << jobNr; _launchAnotherBackgroundJob = false; _ioService->post(HeartbeatBackgroundJob(shared_from_this())); } else { @@ -596,7 +596,7 @@ bool HeartbeatThread::init() { //////////////////////////////////////////////////////////////////////////////// void HeartbeatThread::dispatchedJobResult(DBServerAgencySyncResult result) { - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "Dispatched job returned!"; + LOG_TOPIC(INFO, Logger::HEARTBEAT) << "Dispatched job returned!"; bool doSleep = false; { MUTEX_LOCKER(mutexLocker, *_statusLock); @@ -781,7 +781,7 @@ void HeartbeatThread::syncDBServerStatusQuo() { // schedule a job for the change: uint64_t jobNr = ++_backgroundJobsPosted; - LOG_TOPIC(DEBUG, Logger::HEARTBEAT) << "dispatching sync " << jobNr; + LOG_TOPIC(INFO, Logger::HEARTBEAT) << "dispatching sync " << jobNr; _backgroundJobScheduledOrRunning = true; _ioService->post(HeartbeatBackgroundJob(shared_from_this())); } From 66fbc966486f05e682e6c0840af1277409ad16ba Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 31 Jan 2017 09:07:22 +0100 Subject: [PATCH 12/22] fix CHANGELOG for devel --- CHANGELOG | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index b2dd39ddc5..158c97fd2f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,12 @@ devel * changed index filling to make it more parallel, dispatch tasks to boost::asio +* more detailed stacktraces in Foxx apps + + +v3.1.10 (2017-XX-XX) +-------------------- + * updated versions of bundled node modules: - joi: from 8.4.2 to 9.2.0 - joi-to-json-schema: from 2.2.0 to 2.3.0 From d7f2ee7e2f90f90a238b9376dd28420460d195be Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 31 Jan 2017 09:07:34 +0100 Subject: [PATCH 13/22] fix comment --- arangod/Indexes/PathBasedIndex.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/Indexes/PathBasedIndex.cpp b/arangod/Indexes/PathBasedIndex.cpp index db8969e475..0028d59316 100644 --- a/arangod/Indexes/PathBasedIndex.cpp +++ b/arangod/Indexes/PathBasedIndex.cpp @@ -124,7 +124,7 @@ int PathBasedIndex::fillElement(std::vector& elements, auto slices = buildIndexValue(doc); if (slices.size() == n) { - // if shapes.size() != n, then the value is not inserted into the index + // if slices.size() != n, then the value is not inserted into the index // because of index sparsity! T* element = static_cast(_allocator->allocate()); TRI_ASSERT(element != nullptr); From a5b4eb3c3daef77382af201a17808924b448a250 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Tue, 31 Jan 2017 09:31:52 +0100 Subject: [PATCH 14/22] Lower log level for local shard operations to debug. --- js/server/modules/@arangodb/cluster.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/js/server/modules/@arangodb/cluster.js b/js/server/modules/@arangodb/cluster.js index a5d542cdb9..0b203f2fc8 100644 --- a/js/server/modules/@arangodb/cluster.js +++ b/js/server/modules/@arangodb/cluster.js @@ -745,7 +745,7 @@ function executePlanForCollections(plannedCollections) { let collection; if (!localCollections.hasOwnProperty(shardName)) { // must create this shard - console.info("creating local shard '%s/%s' for central '%s/%s'", + console.debug("creating local shard '%s/%s' for central '%s/%s'", database, shardName, database, @@ -813,7 +813,7 @@ function executePlanForCollections(plannedCollections) { }, {}); if (Object.keys(properties).length > 0) { - console.info("updating properties for local shard '%s/%s'", + console.debug("updating properties for local shard '%s/%s'", database, shardName); @@ -831,17 +831,17 @@ function executePlanForCollections(plannedCollections) { // Now check whether the status is OK: if (collectionStatus !== collectionInfo.status) { - console.info("detected status change for local shard '%s/%s'", + console.debug("detected status change for local shard '%s/%s'", database, shardName); if (collectionInfo.status === ArangoCollection.STATUS_UNLOADED) { - console.info("unloading local shard '%s/%s'", + console.debug("unloading local shard '%s/%s'", database, shardName); collection.unload(); } else if (collectionInfo.status === ArangoCollection.STATUS_LOADED) { - console.info("loading local shard '%s/%s'", + console.debug("loading local shard '%s/%s'", database, shardName); collection.load(); @@ -1264,7 +1264,7 @@ function executePlanForDatabases(plannedDatabases) { if (!plannedDatabases.hasOwnProperty(name) && name.substr(0, 1) !== '_') { // must drop database - console.info("dropping local database '%s'", name); + console.debug("dropping local database '%s'", name); // Do we have to stop a replication applier first? if (ArangoServerState.role() === 'SECONDARY') { @@ -1273,7 +1273,7 @@ function executePlanForDatabases(plannedDatabases) { var rep = require('@arangodb/replication'); var state = rep.applier.state(); if (state.state.running === true) { - console.info('stopping replication applier first'); + console.debug('stopping replication applier first'); rep.applier.stop(); } } From b7b8a6cf88ca2b6247bd77b87f7b9f22827bef87 Mon Sep 17 00:00:00 2001 From: Kaveh Vahedipour Date: Tue, 31 Jan 2017 09:37:47 +0100 Subject: [PATCH 15/22] lowering log output in agencycomm --- arangod/Agency/AgencyComm.cpp | 44 ++++++++++++------- .../tests/resilience/moving-shards-cluster.js | 2 +- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/arangod/Agency/AgencyComm.cpp b/arangod/Agency/AgencyComm.cpp index ba0d79e811..a51d53c90c 100644 --- a/arangod/Agency/AgencyComm.cpp +++ b/arangod/Agency/AgencyComm.cpp @@ -612,7 +612,7 @@ std::string AgencyCommManager::redirect( << specification << ", url = " << rest; if (endpoint == specification) { - LOG_TOPIC(WARN, Logger::AGENCYCOMM) + LOG_TOPIC(DEBUG, Logger::AGENCYCOMM) << "got an agency redirect back to the old agency '" << endpoint << "'"; failedNonLocking(std::move(connection), endpoint); return ""; @@ -632,7 +632,7 @@ std::string AgencyCommManager::redirect( std::remove(_endpoints.begin(), _endpoints.end(), specification), _endpoints.end()); - LOG_TOPIC(WARN, Logger::AGENCYCOMM) + LOG_TOPIC(DEBUG, Logger::AGENCYCOMM) << "Got an agency redirect from '" << endpoint << "' to '" << specification << "'"; @@ -1359,17 +1359,11 @@ AgencyCommResult AgencyComm::sendWithFailover( LOG_TOPIC(ERR, Logger::AGENCYCOMM) << result._message; return result; } - - if (1 < tries) { - LOG_TOPIC(WARN, Logger::AGENCYCOMM) - << "Retrying agency communication at '" << endpoint - << "', tries: " << tries << " (" - << 1.e-2 * ( - std::round( - 1.e+2 * std::chrono::duration( - std::chrono::steady_clock::now() - started).count())) << "s)"; - } - + + double elapsed = 1.e-2 * ( + std::round(1.e+2 * std::chrono::duration( + std::chrono::steady_clock::now() - started).count())); + // try to send; if we fail completely, do not retry try { result = send(connection.get(), method, conTimeout, url, body, clientId); @@ -1379,7 +1373,7 @@ AgencyCommResult AgencyComm::sendWithFailover( connection = AgencyCommManager::MANAGER->acquire(endpoint); continue; } - + // got a result, we are done if (result.successful()) { AgencyCommManager::MANAGER->release(std::move(connection), endpoint); @@ -1389,7 +1383,7 @@ AgencyCommResult AgencyComm::sendWithFailover( // break on a watch timeout (drop connection) if (!clientId.empty() && result._sent && (result._statusCode == 0 || result._statusCode == 503)) { - + VPackBuilder b; { VPackArrayBuilder ab(&b); @@ -1470,6 +1464,26 @@ AgencyCommResult AgencyComm::sendWithFailover( break; } + if (tries%50 == 0) { + LOG_TOPIC(WARN, Logger::AGENCYCOMM) + << "Bad agency communiction! Unsuccessful consecutive tries:" + << tries << " (" << elapsed << "s). Network checks needed!"; + } else if (tries%15 == 0) { + LOG_TOPIC(INFO, Logger::AGENCYCOMM) + << "Flaky agency communication. Unsuccessful consecutive tries: " + << tries << " (" << elapsed << "s). Network checks advised."; + } + + if (1 < tries) { + LOG_TOPIC(DEBUG, Logger::AGENCYCOMM) + << "Retrying agency communication at '" << endpoint + << "', tries: " << tries << " (" + << 1.e-2 * ( + std::round( + 1.e+2 * std::chrono::duration( + std::chrono::steady_clock::now() - started).count())) << "s)"; + } + // here we have failed and want to try next endpoint AgencyCommManager::MANAGER->failed(std::move(connection), endpoint); endpoint.clear(); diff --git a/js/server/tests/resilience/moving-shards-cluster.js b/js/server/tests/resilience/moving-shards-cluster.js index 97f3e81537..2c6dc62f63 100644 --- a/js/server/tests/resilience/moving-shards-cluster.js +++ b/js/server/tests/resilience/moving-shards-cluster.js @@ -163,7 +163,7 @@ function MovingShardsSuite () { } if (!ok) { - console.info( + console.error( "Failed: Server " + id + " was not cleaned out. List of cleaned servers: [" + obj.cleanedServers + "]"); } From 3b2f0bac6aa0a3021d047cb840727fc55eb4051c Mon Sep 17 00:00:00 2001 From: Jan Christoph Uhde Date: Tue, 31 Jan 2017 10:06:36 +0100 Subject: [PATCH 16/22] remove "junk" from test code --- js/server/tests/aql/aql-optimizer-geoindex.js | 40 +------------------ 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/js/server/tests/aql/aql-optimizer-geoindex.js b/js/server/tests/aql/aql-optimizer-geoindex.js index 76ae0fbb93..ec37685c5d 100644 --- a/js/server/tests/aql/aql-optimizer-geoindex.js +++ b/js/server/tests/aql/aql-optimizer-geoindex.js @@ -53,23 +53,8 @@ function optimizerRuleTestSuite() { sorted : true }; - var ruleName = "use-geoindex"; - var secondRuleName = "use-geoindexes"; - var removeCalculationNodes = "remove-unnecessary-calculations-2"; + var ruleName = "geoindex"; var colName = "UnitTestsAqlOptimizer" + ruleName.replace(/-/g, "_"); - var colNameOther = colName + "_XX"; - - // various choices to control the optimizer: - var paramNone = { optimizer: { rules: [ "-all" ] } }; - var paramIndexFromSort = { optimizer: { rules: [ "-all", "+" + ruleName ] } }; - var paramIndexRange = { optimizer: { rules: [ "-all", "+" + secondRuleName ] } }; - var paramIndexFromSort_IndexRange = { optimizer: { rules: [ "-all", "+" + ruleName, "+" + secondRuleName ] } }; - var paramIndexFromSort_IndexRange_RemoveCalculations = { - optimizer: { rules: [ "-all", "+" + ruleName, "+" + secondRuleName, "+" + removeCalculationNodes ] } - }; - var paramIndexFromSort_RemoveCalculations = { - optimizer: { rules: [ "-all", "+" + ruleName, "+" + removeCalculationNodes ] } - }; var geocol; var sortArray = function (l, r) { @@ -113,19 +98,6 @@ function optimizerRuleTestSuite() { }; var geodistance = function(latitude1, longitude1, latitude2, longitude2) { - //if (TYPEWEIGHT(latitude1) !== TYPEWEIGHT_NUMBER || - // TYPEWEIGHT(longitude1) !== TYPEWEIGHT_NUMBER || - // TYPEWEIGHT(latitude2) !== TYPEWEIGHT_NUMBER || - // TYPEWEIGHT(longitude2) !== TYPEWEIGHT_NUMBER) { - // WARN('DISTANCE', INTERNAL.errors.ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - // return null; - //} - - //var p1 = AQL_TO_NUMBER(latitude1) * (Math.PI / 180.0); - //var p2 = AQL_TO_NUMBER(latitude2) * (Math.PI / 180.0); - //var d1 = AQL_TO_NUMBER(latitude2 - latitude1) * (Math.PI / 180.0); - //var d2 = AQL_TO_NUMBER(longitude2 - longitude1) * (Math.PI / 180.0); - var p1 = (latitude1) * (Math.PI / 180.0); var p2 = (latitude2) * (Math.PI / 180.0); var d1 = (latitude2 - latitude1) * (Math.PI / 180.0); @@ -165,7 +137,6 @@ function optimizerRuleTestSuite() { tearDown : function () { internal.db._drop(colName); - internal.db._drop(colNameOther); geocol = null; }, @@ -215,14 +186,6 @@ function optimizerRuleTestSuite() { queries.forEach(function(query) { var result = AQL_EXPLAIN(query.string); - // //optimized on cluster - // if (query[1]) { - // assertNotEqual(-1, removeAlwaysOnClusterRules(result.plan.rules).indexOf(ruleName), query[0]); - // } - // else { - // assertEqual(-1, removeAlwaysOnClusterRules(result.plan.rules).indexOf(ruleName), query[0]); - // } - //sort nodes if (query.sort) { hasSortNode(result,query); @@ -268,7 +231,6 @@ function optimizerRuleTestSuite() { var pairs = result.json.map(function(res){ return [res.lat,res.lon]; }); - //internal.print(pairs) assertEqual(expected[qindex].sort(),pairs.sort()); //expect(expected[qindex].sort()).to.be.equal(result.json.sort()) }); From 27b01aaebb0ec81f212e1fdb9f4a60f60e08cee9 Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Tue, 31 Jan 2017 10:55:16 +0100 Subject: [PATCH 17/22] add ARMv6 detection; we won't run on cpus of that family --- lib/Basics/ArangoGlobalContext.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/Basics/ArangoGlobalContext.cpp b/lib/Basics/ArangoGlobalContext.cpp index 874435b4d9..046c10921c 100644 --- a/lib/Basics/ArangoGlobalContext.cpp +++ b/lib/Basics/ArangoGlobalContext.cpp @@ -289,6 +289,23 @@ void ArangoGlobalContext::runStartupChecks() { "necessary to set the value in '" << filename << "' to 2"; } + std::string const proc_cpuinfo_filename("/proc/cpuinfo"); + try { + std::string const cpuInfo = + arangodb::basics::FileUtils::slurp(proc_cpuinfo_filename); + auto start = cpuInfo.find("ARMv6"); + + if (start != std::string::npos) { + LOG(FATAL) + << "possibly incompatible ARMv6 CPU detected."; + FATAL_ERROR_EXIT(); + } + } catch (...) { + // ignore that we cannot detect the alignment + LOG(TRACE) + << "unable to detect CPU type '" + << filename << "'"; + } } #endif } From 3f8aba5f0ea91f19de1c29a94281064efa9af698 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Tue, 31 Jan 2017 11:58:59 +0100 Subject: [PATCH 18/22] Increase log level for tests. --- etc/testing/arangod-common.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/etc/testing/arangod-common.conf b/etc/testing/arangod-common.conf index 8cb85c7d25..cc6179e80d 100644 --- a/etc/testing/arangod-common.conf +++ b/etc/testing/arangod-common.conf @@ -2,6 +2,7 @@ force-direct = true level = info level = replication=warn +level = threads=debug [database] force-sync-properties = false From da467d1fb824b9015d706af61589d2ffc9f55d09 Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Tue, 31 Jan 2017 12:08:47 +0100 Subject: [PATCH 19/22] according to Georgios Kafataridis we need to choose larger timeouts. --- Installation/systemd/arangodb3.service.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Installation/systemd/arangodb3.service.in b/Installation/systemd/arangodb3.service.in index 658a11aaf5..32e95c10e6 100644 --- a/Installation/systemd/arangodb3.service.in +++ b/Installation/systemd/arangodb3.service.in @@ -30,7 +30,8 @@ ExecStartPre=@CHOWN_EXECUTABLE@ -R arangodb:arangodb /var/lib/arangodb3-apps ExecStartPre=@CHMOD_EXECUTABLE@ 700 /var/lib/arangodb3-apps ExecStartPre=/usr/sbin/arangod --uid arangodb --gid arangodb --pid-file /var/run/arangodb3/arangod.pid --server.rest-server false --database.auto-upgrade true ExecStart=/usr/sbin/arangod --uid arangodb --gid arangodb --pid-file /var/run/arangodb3/arangod.pid --temp.path /var/tmp/arangodb3 --supervisor --log.foreground-tty false -TimeoutStopSec=120 +TimeoutStopSec=3600 +TimeoutSec=3600 Restart=on-failure [Install] From 3c2a466c488d9ef20fd7f1ef6df85370662f53e3 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 31 Jan 2017 12:16:53 +0100 Subject: [PATCH 20/22] use block collector for IndexBlock --- arangod/Aql/AqlItemBlock.cpp | 36 ++++++++++++++- arangod/Aql/AqlItemBlock.h | 4 ++ arangod/Aql/BasicBlocks.cpp | 11 +++++ arangod/Aql/BasicBlocks.h | 9 +--- arangod/Aql/BlockCollector.cpp | 84 ++++++++++++++++++++++++++++++++++ arangod/Aql/BlockCollector.h | 61 ++++++++++++++++++++++++ arangod/Aql/ClusterBlocks.cpp | 1 + arangod/Aql/ExecutionBlock.cpp | 5 +- arangod/Aql/ExecutionBlock.h | 2 +- arangod/Aql/IndexBlock.cpp | 19 ++++++-- arangod/Aql/IndexBlock.h | 3 ++ arangod/CMakeLists.txt | 1 + 12 files changed, 219 insertions(+), 17 deletions(-) create mode 100644 arangod/Aql/BlockCollector.cpp create mode 100644 arangod/Aql/BlockCollector.h diff --git a/arangod/Aql/AqlItemBlock.cpp b/arangod/Aql/AqlItemBlock.cpp index 3f3f04849c..52022bb243 100644 --- a/arangod/Aql/AqlItemBlock.cpp +++ b/arangod/Aql/AqlItemBlock.cpp @@ -22,6 +22,8 @@ //////////////////////////////////////////////////////////////////////////////// #include "AqlItemBlock.h" +#include "Aql/BlockCollector.h" +#include "Aql/ExecutionBlock.h" #include "Aql/ExecutionNode.h" #include "Basics/VelocyPackHelper.h" @@ -430,6 +432,37 @@ AqlItemBlock* AqlItemBlock::steal(std::vector const& chosen, size_t from return res.release(); } +/// @brief concatenate multiple blocks +AqlItemBlock* AqlItemBlock::concatenate(ResourceMonitor* resourceMonitor, + BlockCollector* collector) { + + size_t totalSize = collector->totalSize(); + RegisterId nrRegs = collector->nrRegs(); + + TRI_ASSERT(totalSize > 0); + TRI_ASSERT(nrRegs > 0); + + auto res = std::make_unique(resourceMonitor, totalSize, nrRegs); + + size_t pos = 0; + for (auto& it : collector->_blocks) { + size_t const n = it->size(); + for (size_t row = 0; row < n; ++row) { + for (RegisterId col = 0; col < nrRegs; ++col) { + // copy over value + AqlValue const& a = it->getValueReference(row, col); + if (!a.isEmpty()) { + res->setValue(pos + row, col, a); + } + } + } + it->eraseAll(); + pos += n; + } + + return res.release(); +} + /// @brief concatenate multiple blocks, note that the new block now owns all /// AqlValue pointers in the old blocks, therefore, the latter are all /// set to nullptr, just to be sure. @@ -455,7 +488,6 @@ AqlItemBlock* AqlItemBlock::concatenate(ResourceMonitor* resourceMonitor, size_t pos = 0; for (auto& it : blocks) { - TRI_ASSERT(it != res.get()); size_t const n = it->size(); for (size_t row = 0; row < n; ++row) { for (RegisterId col = 0; col < nrRegs; ++col) { @@ -467,7 +499,7 @@ AqlItemBlock* AqlItemBlock::concatenate(ResourceMonitor* resourceMonitor, } } it->eraseAll(); - pos += it->size(); + pos += n; } return res.release(); diff --git a/arangod/Aql/AqlItemBlock.h b/arangod/Aql/AqlItemBlock.h index 825030f4af..5baf89b091 100644 --- a/arangod/Aql/AqlItemBlock.h +++ b/arangod/Aql/AqlItemBlock.h @@ -32,6 +32,7 @@ namespace arangodb { namespace aql { +class BlockCollector; // an is a x vector of s (not // pointers). The size of an is the number of items. @@ -254,6 +255,9 @@ class AqlItemBlock { /// after this operation, because it is unclear, when the values /// to which our AqlValues point will vanish. AqlItemBlock* steal(std::vector const& chosen, size_t from, size_t to); + + /// @brief concatenate multiple blocks from a collector + static AqlItemBlock* concatenate(ResourceMonitor*, BlockCollector* collector); /// @brief concatenate multiple blocks, note that the new block now owns all /// AqlValue pointers in the old blocks, therefore, the latter are all diff --git a/arangod/Aql/BasicBlocks.cpp b/arangod/Aql/BasicBlocks.cpp index 6f0ff5e62e..d32c220b9e 100644 --- a/arangod/Aql/BasicBlocks.cpp +++ b/arangod/Aql/BasicBlocks.cpp @@ -22,11 +22,17 @@ //////////////////////////////////////////////////////////////////////////////// #include "BasicBlocks.h" +#include "Aql/AqlItemBlock.h" #include "Aql/ExecutionEngine.h" #include "Basics/Exceptions.h" #include "VocBase/vocbase.h" using namespace arangodb::aql; + +void SingletonBlock::deleteInputVariables() { + delete _inputRegisterValues; + _inputRegisterValues = nullptr; +} void SingletonBlock::buildWhitelist() { if (!_whitelistBuilt) { @@ -154,6 +160,11 @@ FilterBlock::FilterBlock(ExecutionEngine* engine, FilterNode const* en) } FilterBlock::~FilterBlock() {} + +/// @brief internal function to actually decide if the document should be used +bool FilterBlock::takeItem(AqlItemBlock* items, size_t index) const { + return items->getValueReference(index, _inReg).toBoolean(); +} /// @brief internal function to get another block bool FilterBlock::getBlock(size_t atLeast, size_t atMost) { diff --git a/arangod/Aql/BasicBlocks.h b/arangod/Aql/BasicBlocks.h index ddcb5cf398..4a86ee2597 100644 --- a/arangod/Aql/BasicBlocks.h +++ b/arangod/Aql/BasicBlocks.h @@ -59,10 +59,7 @@ class SingletonBlock : public ExecutionBlock { int64_t remaining() override final { return _done ? 0 : 1; } private: - void deleteInputVariables() { - delete _inputRegisterValues; - _inputRegisterValues = nullptr; - } + void deleteInputVariables(); void buildWhitelist(); @@ -85,9 +82,7 @@ class FilterBlock : public ExecutionBlock { private: /// @brief internal function to actually decide if the document should be used - inline bool takeItem(AqlItemBlock* items, size_t index) const { - return items->getValueReference(index, _inReg).toBoolean(); - } + bool takeItem(AqlItemBlock* items, size_t index) const; /// @brief internal function to get another block bool getBlock(size_t atLeast, size_t atMost); diff --git a/arangod/Aql/BlockCollector.cpp b/arangod/Aql/BlockCollector.cpp new file mode 100644 index 0000000000..e6f65f75ba --- /dev/null +++ b/arangod/Aql/BlockCollector.cpp @@ -0,0 +1,84 @@ +//////////////////////////////////////////////////////////////////////////////// +/// @brief Infrastructure for ExecutionPlans +/// +/// DISCLAIMER +/// +/// Copyright 2010-2014 triagens GmbH, Cologne, Germany +/// +/// Licensed under the Apache License, Version 2.0 (the "License"); +/// you may not use this file except in compliance with the License. +/// You may obtain a copy of the License at +/// +/// http://www.apache.org/licenses/LICENSE-2.0 +/// +/// Unless required by applicable law or agreed to in writing, software +/// distributed under the License is distributed on an "AS IS" BASIS, +/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +/// See the License for the specific language governing permissions and +/// limitations under the License. +/// +/// Copyright holder is triAGENS GmbH, Cologne, Germany +/// +/// @author Jan Steemann +/// @author Copyright 2014, triagens GmbH, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +#include "BlockCollector.h" +#include "Aql/AqlItemBlock.h" +#include "Aql/ResourceUsage.h" + +using namespace arangodb::aql; + +BlockCollector::BlockCollector() : _totalSize(0) {} + +BlockCollector::~BlockCollector() { clear(); } + +size_t BlockCollector::totalSize() const { return _totalSize; } + +size_t BlockCollector::nrRegs() const { + TRI_ASSERT(_totalSize > 0); + TRI_ASSERT(!_blocks.empty()); + return _blocks[0]->getNrRegs(); +} + +void BlockCollector::clear() { + for (auto& it : _blocks) { + it->eraseAll(); + delete it; + } + _blocks.clear(); + _totalSize = 0; +} + +void BlockCollector::add(std::unique_ptr block) { + TRI_ASSERT(block->size() > 0); + + _blocks.push_back(block.get()); + _totalSize += block->size(); + block.release(); +} + +AqlItemBlock* BlockCollector::steal(ResourceMonitor* resourceMonitor) { + if (_blocks.empty()) { + return nullptr; + } + + TRI_ASSERT(_totalSize > 0); + AqlItemBlock* result = nullptr; + + if (_blocks.size() == 1) { + // only got a single result. return it as it is + result = _blocks[0]; + } else { + result = AqlItemBlock::concatenate(resourceMonitor, this); + for (auto& it : _blocks) { + delete it; + } + } + + // ownership is now passed to result + _totalSize = 0; + _blocks.clear(); + return result; +} + diff --git a/arangod/Aql/BlockCollector.h b/arangod/Aql/BlockCollector.h new file mode 100644 index 0000000000..1898f33da4 --- /dev/null +++ b/arangod/Aql/BlockCollector.h @@ -0,0 +1,61 @@ +//////////////////////////////////////////////////////////////////////////////// +/// DISCLAIMER +/// +/// Copyright 2014-2016 ArangoDB GmbH, Cologne, Germany +/// Copyright 2004-2014 triAGENS GmbH, Cologne, Germany +/// +/// Licensed under the Apache License, Version 2.0 (the "License"); +/// you may not use this file except in compliance with the License. +/// You may obtain a copy of the License at +/// +/// http://www.apache.org/licenses/LICENSE-2.0 +/// +/// Unless required by applicable law or agreed to in writing, software +/// distributed under the License is distributed on an "AS IS" BASIS, +/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +/// See the License for the specific language governing permissions and +/// limitations under the License. +/// +/// Copyright holder is ArangoDB GmbH, Cologne, Germany +/// +/// @author Jan Steemann +//////////////////////////////////////////////////////////////////////////////// + +#ifndef ARANGOD_AQL_BLOCK_COLLECTOR_H +#define ARANGOD_AQL_BLOCK_COLLECTOR_H 1 + +#include "Basics/Common.h" + +namespace arangodb { +namespace aql { +class AqlItemBlock; +class ResourceMonitor; + +class BlockCollector { + friend class AqlItemBlock; + + public: + BlockCollector(BlockCollector const&) = delete; + BlockCollector& operator=(BlockCollector const&) = delete; + + BlockCollector(); + ~BlockCollector(); + + size_t totalSize() const; + size_t nrRegs() const; + + void clear(); + + void add(std::unique_ptr block); + + AqlItemBlock* steal(ResourceMonitor*); + + private: + std::vector _blocks; + size_t _totalSize; +}; + +} // namespace arangodb::aql +} // namespace arangodb + +#endif diff --git a/arangod/Aql/ClusterBlocks.cpp b/arangod/Aql/ClusterBlocks.cpp index 00780f7727..9496cca883 100644 --- a/arangod/Aql/ClusterBlocks.cpp +++ b/arangod/Aql/ClusterBlocks.cpp @@ -29,6 +29,7 @@ #include #include +#include "Aql/AqlItemBlock.h" #include "Aql/AqlValue.h" #include "Aql/ExecutionEngine.h" #include "Basics/Exceptions.h" diff --git a/arangod/Aql/ExecutionBlock.cpp b/arangod/Aql/ExecutionBlock.cpp index 2b20b949ad..eb878bc028 100644 --- a/arangod/Aql/ExecutionBlock.cpp +++ b/arangod/Aql/ExecutionBlock.cpp @@ -23,11 +23,12 @@ //////////////////////////////////////////////////////////////////////////////// #include "ExecutionBlock.h" -#include "Aql/ExecutionEngine.h" +#include "Aql/AqlItemBlock.h" #include "Aql/Ast.h" +#include "Aql/ExecutionEngine.h" using namespace arangodb::aql; - + ExecutionBlock::ExecutionBlock(ExecutionEngine* engine, ExecutionNode const* ep) : _engine(engine), _trx(engine->getQuery()->trx()), diff --git a/arangod/Aql/ExecutionBlock.h b/arangod/Aql/ExecutionBlock.h index d39d8d8be6..ebbdefc4a0 100644 --- a/arangod/Aql/ExecutionBlock.h +++ b/arangod/Aql/ExecutionBlock.h @@ -24,7 +24,6 @@ #ifndef ARANGOD_AQL_EXECUTION_BLOCK_H #define ARANGOD_AQL_EXECUTION_BLOCK_H 1 -#include "AqlItemBlock.h" #include "Aql/ExecutionNode.h" #include "Aql/Variable.h" @@ -61,6 +60,7 @@ namespace arangodb { class Transaction; namespace aql { +class AqlItemBlock; /// @brief sort element for block, consisting of register, sort direction, /// and a possible attribute path to dig into the document diff --git a/arangod/Aql/IndexBlock.cpp b/arangod/Aql/IndexBlock.cpp index 392b247778..974266d9f0 100644 --- a/arangod/Aql/IndexBlock.cpp +++ b/arangod/Aql/IndexBlock.cpp @@ -470,6 +470,7 @@ bool IndexBlock::readIndex(size_t atMost) { int IndexBlock::initializeCursor(AqlItemBlock* items, size_t pos) { DEBUG_BEGIN_BLOCK(); + _collector.clear(); int res = ExecutionBlock::initializeCursor(items, pos); if (res != TRI_ERROR_NO_ERROR) { @@ -492,9 +493,9 @@ AqlItemBlock* IndexBlock::getSome(size_t atLeast, size_t atMost) { traceGetSomeBegin(); if (_done) { traceGetSomeEnd(nullptr); - return nullptr; + return _collector.steal(_engine->getQuery()->resourceMonitor()); } - + std::unique_ptr res; do { @@ -508,7 +509,7 @@ AqlItemBlock* IndexBlock::getSome(size_t atLeast, size_t atMost) { if (!ExecutionBlock::getBlock(toFetch, toFetch) || (!initIndexes())) { _done = true; traceGetSomeEnd(nullptr); - return nullptr; + return _collector.steal(_engine->getQuery()->resourceMonitor()); } _pos = 0; // this is in the first block @@ -530,7 +531,7 @@ AqlItemBlock* IndexBlock::getSome(size_t atLeast, size_t atMost) { if (!ExecutionBlock::getBlock(DefaultBatchSize(), DefaultBatchSize())) { _done = true; traceGetSomeEnd(nullptr); - return nullptr; + return _collector.steal(_engine->getQuery()->resourceMonitor()); } _pos = 0; // this is in the first block } @@ -538,7 +539,7 @@ AqlItemBlock* IndexBlock::getSome(size_t atLeast, size_t atMost) { if (!initIndexes()) { _done = true; traceGetSomeEnd(nullptr); - return nullptr; + return _collector.steal(_engine->getQuery()->resourceMonitor()); } readIndex(atMost); } @@ -581,6 +582,13 @@ AqlItemBlock* IndexBlock::getSome(size_t atLeast, size_t atMost) { res->copyValuesFromFirstRow(j, static_cast(curRegs)); } } + + _collector.add(std::move(res)); + TRI_ASSERT(res.get() == nullptr); + + if (_collector.totalSize() >= atMost) { + res.reset(_collector.steal(_engine->getQuery()->resourceMonitor())); + } } } while (res.get() == nullptr); @@ -588,6 +596,7 @@ AqlItemBlock* IndexBlock::getSome(size_t atLeast, size_t atMost) { // Clear out registers no longer needed later: clearRegisters(res.get()); traceGetSomeEnd(res.get()); + return res.release(); // cppcheck-suppress style diff --git a/arangod/Aql/IndexBlock.h b/arangod/Aql/IndexBlock.h index 88c6d4a247..663fdc83ca 100644 --- a/arangod/Aql/IndexBlock.h +++ b/arangod/Aql/IndexBlock.h @@ -25,6 +25,7 @@ #ifndef ARANGOD_AQL_INDEX_BLOCK_H #define ARANGOD_AQL_INDEX_BLOCK_H 1 +#include "Aql/BlockCollector.h" #include "Aql/ExecutionBlock.h" #include "Aql/ExecutionNode.h" #include "Aql/IndexNode.h" @@ -155,6 +156,8 @@ class IndexBlock : public ExecutionBlock { bool _hasV8Expression; std::unique_ptr _mmdr; + + BlockCollector _collector; }; } // namespace arangodb::aql diff --git a/arangod/CMakeLists.txt b/arangod/CMakeLists.txt index d996b9600d..5a49d43ec2 100644 --- a/arangod/CMakeLists.txt +++ b/arangod/CMakeLists.txt @@ -115,6 +115,7 @@ SET(ARANGOD_SOURCES Aql/BaseExpressionContext.cpp Aql/BasicBlocks.cpp Aql/BindParameters.cpp + Aql/BlockCollector.cpp Aql/CalculationBlock.cpp Aql/ClusterBlocks.cpp Aql/ClusterNodes.cpp From eec173c4eecf95a780fc26e123e68b48a6440408 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 31 Jan 2017 12:17:35 +0100 Subject: [PATCH 21/22] optimize `IS_NULL(x)` to `x == null` --- arangod/Aql/Ast.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arangod/Aql/Ast.cpp b/arangod/Aql/Ast.cpp index d1fd12c2df..8850530149 100644 --- a/arangod/Aql/Ast.cpp +++ b/arangod/Aql/Ast.cpp @@ -2749,6 +2749,12 @@ AstNode* Ast::optimizeFunctionCall(AstNode* node) { return createNodeFunctionCall("COLLECTION_COUNT", countArgs); } } + } else if (func->externalName == "IS_NULL") { + auto args = node->getMember(0); + if (args->numMembers() == 1) { + // replace IS_NULL(x) function call with `x == null` + return createNodeBinaryOperator(NODE_TYPE_OPERATOR_BINARY_EQ, args->getMemberUnchecked(0), createNodeValueNull()); + } } if (!func->isDeterministic) { From 659699a11e58ff56102f899e5ae679487692cc51 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 31 Jan 2017 12:17:55 +0100 Subject: [PATCH 22/22] allow usage of loglevel `err` (not only `error`) --- lib/Logger/Logger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Logger/Logger.cpp b/lib/Logger/Logger.cpp index 57d95c3472..bd365ce709 100644 --- a/lib/Logger/Logger.cpp +++ b/lib/Logger/Logger.cpp @@ -84,7 +84,7 @@ void Logger::setLogLevel(std::string const& levelName) { if (l == "fatal") { level = LogLevel::FATAL; - } else if (l == "error") { + } else if (l == "error" || l == "err") { level = LogLevel::ERR; } else if (l == "warning" || l == "warn") { level = LogLevel::WARN;