From 93d6b7f05869080c988e3e0344b56541acac9f27 Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Fri, 21 Apr 2017 11:12:14 +0200 Subject: [PATCH 1/4] add ArangoDBStarter article, move detail articles into submenu layers --- Documentation/Books/Manual/Deployment/README.mdpp | 10 ++++++---- Documentation/Books/Manual/SUMMARY.md | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Documentation/Books/Manual/Deployment/README.mdpp b/Documentation/Books/Manual/Deployment/README.mdpp index 5a6ade17e1..e22cec39f7 100644 --- a/Documentation/Books/Manual/Deployment/README.mdpp +++ b/Documentation/Books/Manual/Deployment/README.mdpp @@ -10,8 +10,10 @@ or using Docker containers. - [Single instance](Single.md) - [Cluster: DC/OS, Apache Mesos and Marathon](Mesos.md) -- [Cluster: Test setup on a local machine](Local.md) -- [Cluster: Starting processes on different machines](Distributed.md) -- [Cluster: Launching an ArangoDB cluster using Docker containers](Docker.md) -- [Agency](Agency.md) +- [Cluster: Generic & Docker](ArangoDBStarter.md) +- [Advanced Topics](Advanced.md) + - [Cluster: Test setup on a local machine](Local.md) + - [Cluster: Starting processes on different machines](Distributed.md) + - [Cluster: Launching an ArangoDB cluster using Docker containers](Docker.md) + - [Agency](Agency.md) diff --git a/Documentation/Books/Manual/SUMMARY.md b/Documentation/Books/Manual/SUMMARY.md index 8158fbf516..b41b290947 100644 --- a/Documentation/Books/Manual/SUMMARY.md +++ b/Documentation/Books/Manual/SUMMARY.md @@ -123,10 +123,12 @@ * [Deployment](Deployment/README.md) * [Single instance](Deployment/Single.md) * [Cluster: Mesos, DC/OS](Deployment/Mesos.md) - * [Cluster: Local test](Deployment/Local.md) - * [Cluster: Processes](Deployment/Distributed.md) - * [Cluster: Docker](Deployment/Docker.md) - * [Agency](Deployment/Agency.md) + * [Cluster: Generic & Docker](Deployment/ArangoDBStarter.md) + * [Advanced Topics](Deployment/Advanced.md) + * [Standalone Agency](Deployment/Agency.md) + * [Cluster: Local test setups](Deployment/Local.md) + * [Cluster: Processes](Deployment/Distributed.md) + * [Cluster: Docker](Deployment/Docker.md) # * [Administration](Administration/README.md) * [Web Interface](Administration/WebInterface/README.md) From e3beaaa8fce29612e6172a1ed10f90e445baeda7 Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Fri, 21 Apr 2017 11:31:30 +0200 Subject: [PATCH 2/4] Added a test + fix for a Stackoverflow issue in Traversals and ShortestPath in AQL. This occured in the case where there are a lot of starting vertices in a row that do not have any paths attached to them. Fixes: #2445 --- CHANGELOG | 6 + arangod/Aql/ShortestPathBlock.cpp | 137 +++++++++-------- arangod/Aql/TraversalBlock.cpp | 162 ++++++++++----------- js/server/tests/aql/aql-graph-traverser.js | 16 ++ 4 files changed, 171 insertions(+), 150 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 147bbce1e6..2d188361fa 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -184,6 +184,12 @@ v3.2.alpha1 (2017-02-05) * generated Foxx services now use swagger tags +v3.1.19 (XXXX-XX-XX) +-------------------- + + +* Fixed a StackOverflow issue in Traversal and ShortestPath. Occured if many (>1000) input + values in a row do not return any result. Fixes issue: #2445 v3.1.18 (2017-04-18) -------------------- diff --git a/arangod/Aql/ShortestPathBlock.cpp b/arangod/Aql/ShortestPathBlock.cpp index dfbe71d4fd..20c4002282 100644 --- a/arangod/Aql/ShortestPathBlock.cpp +++ b/arangod/Aql/ShortestPathBlock.cpp @@ -532,87 +532,86 @@ bool ShortestPathBlock::nextPath(AqlItemBlock const* items) { AqlItemBlock* ShortestPathBlock::getSome(size_t, size_t atMost) { DEBUG_BEGIN_BLOCK(); traceGetSomeBegin(); - if (_done) { - traceGetSomeEnd(nullptr); - return nullptr; - } - - if (_buffer.empty()) { - size_t toFetch = (std::min)(DefaultBatchSize(), atMost); - if (!ExecutionBlock::getBlock(toFetch, toFetch)) { - _done = true; + while (true) { + if (_done) { traceGetSomeEnd(nullptr); return nullptr; } - _pos = 0; // this is in the first block - } - // If we get here, we do have _buffer.front() - AqlItemBlock* cur = _buffer.front(); - size_t const curRegs = cur->getNrRegs(); + if (_buffer.empty()) { + size_t toFetch = (std::min)(DefaultBatchSize(), atMost); + if (!ExecutionBlock::getBlock(toFetch, toFetch)) { + _done = true; + traceGetSomeEnd(nullptr); + return nullptr; + } + _pos = 0; // this is in the first block + } - // Collect the next path: - if (_posInPath >= _pathLength) { - if (!nextPath(cur)) { - // This input does not have any path. maybe the next one has. - // we can only return nullptr iff the buffer is empty. + // If we get here, we do have _buffer.front() + AqlItemBlock* cur = _buffer.front(); + size_t const curRegs = cur->getNrRegs(); + + // Collect the next path: + if (_posInPath >= _pathLength) { + if (!nextPath(cur)) { + // This input does not have any path. maybe the next one has. + // we can only return nullptr iff the buffer is empty. + if (++_pos >= cur->size()) { + _buffer.pop_front(); // does not throw + returnBlock(cur); + _pos = 0; + } + continue; + } + } + + size_t available = _pathLength - _posInPath; + size_t toSend = (std::min)(atMost, available); + + RegisterId nrRegs = + getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()]; + std::unique_ptr res(requestBlock(toSend, nrRegs)); + // automatically freed if we throw + TRI_ASSERT(curRegs <= res->getNrRegs()); + + // only copy 1st row of registers inherited from previous frame(s) + inheritRegisters(cur, res.get(), _pos); + + // TODO: lease builder? + VPackBuilder resultBuilder; + for (size_t j = 0; j < toSend; j++) { + if (usesVertexOutput()) { + resultBuilder.clear(); + _path->vertexToVelocyPack(_trx, _mmdr.get(), _posInPath, resultBuilder); + res->setValue(j, _vertexReg, AqlValue(resultBuilder.slice())); + } + if (usesEdgeOutput()) { + resultBuilder.clear(); + _path->edgeToVelocyPack(_trx, _mmdr.get(), _posInPath, resultBuilder); + res->setValue(j, _edgeReg, AqlValue(resultBuilder.slice())); + } + if (j > 0) { + // re-use already copied aqlvalues + res->copyValuesFromFirstRow(j, static_cast(curRegs)); + } + ++_posInPath; + } + + if (_posInPath >= _pathLength) { + // Advance read position for next call if (++_pos >= cur->size()) { _buffer.pop_front(); // does not throw returnBlock(cur); _pos = 0; } - auto r = getSome(atMost, atMost); - traceGetSomeEnd(r); - return r; } + + // Clear out registers no longer needed later: + clearRegisters(res.get()); + traceGetSomeEnd(res.get()); + return res.release(); } - - size_t available = _pathLength - _posInPath; - size_t toSend = (std::min)(atMost, available); - - RegisterId nrRegs = - getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()]; - std::unique_ptr res(requestBlock(toSend, nrRegs)); - // automatically freed if we throw - TRI_ASSERT(curRegs <= res->getNrRegs()); - - // only copy 1st row of registers inherited from previous frame(s) - inheritRegisters(cur, res.get(), _pos); - - // TODO: lease builder? - VPackBuilder resultBuilder; - for (size_t j = 0; j < toSend; j++) { - if (usesVertexOutput()) { - resultBuilder.clear(); - _path->vertexToVelocyPack(_trx, _mmdr.get(), _posInPath, resultBuilder); - res->setValue(j, _vertexReg, AqlValue(resultBuilder.slice())); - } - if (usesEdgeOutput()) { - resultBuilder.clear(); - _path->edgeToVelocyPack(_trx, _mmdr.get(), _posInPath, resultBuilder); - res->setValue(j, _edgeReg, AqlValue(resultBuilder.slice())); - } - if (j > 0) { - // re-use already copied aqlvalues - res->copyValuesFromFirstRow(j, static_cast(curRegs)); - } - ++_posInPath; - } - - if (_posInPath >= _pathLength) { - // Advance read position for next call - if (++_pos >= cur->size()) { - _buffer.pop_front(); // does not throw - returnBlock(cur); - _pos = 0; - } - } - - // Clear out registers no longer needed later: - clearRegisters(res.get()); - traceGetSomeEnd(res.get()); - return res.release(); - // cppcheck-suppress style DEBUG_END_BLOCK(); } diff --git a/arangod/Aql/TraversalBlock.cpp b/arangod/Aql/TraversalBlock.cpp index efac58964c..aed15d9b24 100644 --- a/arangod/Aql/TraversalBlock.cpp +++ b/arangod/Aql/TraversalBlock.cpp @@ -352,100 +352,100 @@ AqlItemBlock* TraversalBlock::getSome(size_t, // atLeast, size_t atMost) { DEBUG_BEGIN_BLOCK(); traceGetSomeBegin(); - if (_done) { - traceGetSomeEnd(nullptr); - return nullptr; - } - - if (_buffer.empty()) { - size_t toFetch = (std::min)(DefaultBatchSize(), atMost); - if (!ExecutionBlock::getBlock(toFetch, toFetch)) { - _done = true; + while (true) { + if (_done) { traceGetSomeEnd(nullptr); return nullptr; } - _pos = 0; // this is in the first block - } - // If we get here, we do have _buffer.front() - AqlItemBlock* cur = _buffer.front(); - size_t const curRegs = cur->getNrRegs(); - - if (_pos == 0) { - // Initial initialization - initializePaths(cur, _pos); - } - - // Iterate more paths: - if (_posInPaths >= _vertices.size()) { - if (!morePaths(atMost)) { - // This input does not have any more paths. maybe the next one has. - // we can only return nullptr iff the buffer is empty. - _usedConstant = false; // must reset this variable because otherwise the traverser's start vertex may not be reset properly - if (++_pos >= cur->size()) { - _buffer.pop_front(); // does not throw - returnBlock(cur); - _pos = 0; - } else { - initializePaths(cur, _pos); + if (_buffer.empty()) { + size_t toFetch = (std::min)(DefaultBatchSize(), atMost); + if (!ExecutionBlock::getBlock(toFetch, toFetch)) { + _done = true; + traceGetSomeEnd(nullptr); + return nullptr; } - auto r = getSome(atMost, atMost); - traceGetSomeEnd(r); - return r; + _pos = 0; // this is in the first block } - } - size_t available = _vertices.size() - _posInPaths; - size_t toSend = (std::min)(atMost, available); + // If we get here, we do have _buffer.front() + AqlItemBlock* cur = _buffer.front(); + size_t const curRegs = cur->getNrRegs(); - RegisterId nrRegs = - getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()]; - - std::unique_ptr res(requestBlock(toSend, nrRegs)); - // automatically freed if we throw - TRI_ASSERT(curRegs <= res->getNrRegs()); - - // only copy 1st row of registers inherited from previous frame(s) - inheritRegisters(cur, res.get(), _pos); - - for (size_t j = 0; j < toSend; j++) { - if (usesVertexOutput()) { - res->setValue(j, _vertexReg, _vertices[_posInPaths].clone()); + if (_pos == 0) { + // Initial initialization + initializePaths(cur, _pos); } - if (usesEdgeOutput()) { - res->setValue(j, _edgeReg, _edges[_posInPaths].clone()); - } - if (usesPathOutput()) { - res->setValue(j, _pathReg, _paths[_posInPaths].clone()); - } - if (j > 0) { - // re-use already copied AqlValues - res->copyValuesFromFirstRow(j, static_cast(curRegs)); - } - ++_posInPaths; - } - // Advance read position: - if (_posInPaths >= _vertices.size()) { - // we have exhausted our local paths buffer - // fetch more paths into our buffer - if (!morePaths(atMost)) { - // nothing more to read, re-initialize fetching of paths - _usedConstant = false; // must reset this variable because otherwise the traverser's start vertex may not be reset properly - if (++_pos >= cur->size()) { - _buffer.pop_front(); // does not throw - returnBlock(cur); - _pos = 0; - } else { - initializePaths(cur, _pos); + // Iterate more paths: + if (_posInPaths >= _vertices.size()) { + if (!morePaths(atMost)) { + // This input does not have any more paths. maybe the next one has. + // we can only return nullptr iff the buffer is empty. + _usedConstant = false; // must reset this variable because otherwise the traverser's start vertex may not be reset properly + if (++_pos >= cur->size()) { + _buffer.pop_front(); // does not throw + returnBlock(cur); + _pos = 0; + } else { + initializePaths(cur, _pos); + } + continue; } } - } - // Clear out registers no longer needed later: - clearRegisters(res.get()); - traceGetSomeEnd(res.get()); - return res.release(); + size_t available = _vertices.size() - _posInPaths; + size_t toSend = (std::min)(atMost, available); + + RegisterId nrRegs = + getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()]; + + std::unique_ptr res(requestBlock(toSend, nrRegs)); + // automatically freed if we throw + TRI_ASSERT(curRegs <= res->getNrRegs()); + + // only copy 1st row of registers inherited from previous frame(s) + inheritRegisters(cur, res.get(), _pos); + + for (size_t j = 0; j < toSend; j++) { + if (usesVertexOutput()) { + res->setValue(j, _vertexReg, _vertices[_posInPaths].clone()); + } + if (usesEdgeOutput()) { + res->setValue(j, _edgeReg, _edges[_posInPaths].clone()); + } + if (usesPathOutput()) { + res->setValue(j, _pathReg, _paths[_posInPaths].clone()); + } + if (j > 0) { + // re-use already copied AqlValues + res->copyValuesFromFirstRow(j, static_cast(curRegs)); + } + ++_posInPaths; + } + // Advance read position: + if (_posInPaths >= _vertices.size()) { + // we have exhausted our local paths buffer + // fetch more paths into our buffer + if (!morePaths(atMost)) { + // nothing more to read, re-initialize fetching of paths + + _usedConstant = false; // must reset this variable because otherwise the traverser's start vertex may not be reset properly + if (++_pos >= cur->size()) { + _buffer.pop_front(); // does not throw + returnBlock(cur); + _pos = 0; + } else { + initializePaths(cur, _pos); + } + } + } + + // Clear out registers no longer needed later: + clearRegisters(res.get()); + traceGetSomeEnd(res.get()); + return res.release(); + } // cppcheck-suppress style DEBUG_END_BLOCK(); diff --git a/js/server/tests/aql/aql-graph-traverser.js b/js/server/tests/aql/aql-graph-traverser.js index dd7b4516cc..35dd063986 100644 --- a/js/server/tests/aql/aql-graph-traverser.js +++ b/js/server/tests/aql/aql-graph-traverser.js @@ -1514,6 +1514,22 @@ function complexInternaSuite () { assertEqual(found, amount); }, + testTailRecursion: function () { + // This test is to make sure their is no + // inifinite callstack in getSome() API + let query = ` + WITH ${vn} + FOR id IN 0..100000 + FOR v IN OUTBOUND CONCAT('${vn}/foobar', id) ${en} + RETURN v + `; + + let res = db._query(query); + assertEqual(res.count(), 0); + // With inifinit callstack in getSome this + // test will segfault + }, + }; } From 2dbd179656d4ff0cff0dc7db4ee8dba4ef8ac2b9 Mon Sep 17 00:00:00 2001 From: Kaveh Vahedipour Date: Fri, 21 Apr 2017 11:10:53 +0200 Subject: [PATCH 3/4] Fix ocasionally hangling shut down on mac --- arangod/Cluster/HeartbeatThread.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arangod/Cluster/HeartbeatThread.cpp b/arangod/Cluster/HeartbeatThread.cpp index 9bed1d93f5..af42ac8bc1 100644 --- a/arangod/Cluster/HeartbeatThread.cpp +++ b/arangod/Cluster/HeartbeatThread.cpp @@ -825,7 +825,10 @@ void HeartbeatThread::syncDBServerStatusQuo() { _backgroundJobScheduledOrRunning = true; // the JobGuard is in the operator() of HeartbeatBackgroundJob - _ioService->post(HeartbeatBackgroundJob(shared_from_this(), TRI_microtime())); + if (!isStopping() && !_ioService->stopped()) { + _ioService-> + post(HeartbeatBackgroundJob(shared_from_this(), TRI_microtime())); + } } //////////////////////////////////////////////////////////////////////////////// From 020277caf3777a0d77c11035b2ecd2b0efb5d4b7 Mon Sep 17 00:00:00 2001 From: Kaveh Vahedipour Date: Fri, 21 Apr 2017 11:50:42 +0200 Subject: [PATCH 4/4] cherry pick of 2e7924e74afb401030625239c3737ef401080784 from 3.1 --- CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 2d188361fa..ed82b9a285 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,8 @@ devel ----- +* potential fix for shutdown hangs on OSX + * added KB, MB, GB prefix for integer parameter, % for integer parameter with a base value