From 5f45c6e856c270eb6a192860099fa7e750248114 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 28 Apr 2025 12:27:56 +0200 Subject: [PATCH 01/28] first draft, added cancel of traversal in one sided enumerator, and to all providers including mock provider - also added unit/gtest for providers --- .../Aql/ExecutionNode/EnumeratePathsNode.cpp | 12 +++-- .../Aql/ExecutionNode/ShortestPathNode.cpp | 11 ++-- arangod/Aql/ExecutionNode/TraversalNode.cpp | 9 ++-- .../Graph/Enumerators/OneSidedEnumerator.cpp | 9 +++- .../Options/OneSidedEnumeratorOptions.cpp | 5 +- .../Graph/Options/OneSidedEnumeratorOptions.h | 7 ++- arangod/Graph/Options/QueryContextObserver.h | 52 ++++++++++++++++++ .../Graph/Providers/BaseProviderOptions.cpp | 18 ++++--- arangod/Graph/Providers/BaseProviderOptions.h | 27 +++++++--- arangod/Graph/Providers/ClusterProvider.cpp | 14 +++++ .../Graph/Providers/SingleServerProvider.cpp | 5 ++ tests/Graph/DFSFinderTest.cpp | 2 +- tests/Graph/GenericGraphProviderTest.cpp | 54 ++++++++++++++++++- tests/Graph/SingleServerProviderTest.cpp | 2 +- tests/Mocks/MockGraphProvider.cpp | 10 +++- tests/Mocks/MockGraphProvider.h | 11 ++-- 16 files changed, 211 insertions(+), 37 deletions(-) create mode 100644 arangod/Graph/Options/QueryContextObserver.h diff --git a/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp b/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp index 7b819b1a1a9b..196d8f7207cf 100644 --- a/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp +++ b/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp @@ -476,13 +476,15 @@ std::unique_ptr EnumeratePathsNode::createBlock( SingleServerBaseProviderOptions forwardProviderOptions( opts->tmpVar(), std::move(usedIndexes), opts->getExpressionCtx(), {}, opts->collectionToShard(), opts->getVertexProjections(), - opts->getEdgeProjections(), opts->produceVertices(), opts->useCache()); + opts->getEdgeProjections(), opts->produceVertices(), opts->useCache(), + opts->query()); SingleServerBaseProviderOptions backwardProviderOptions( opts->tmpVar(), std::move(reversedUsedIndexes), opts->getExpressionCtx(), {}, opts->collectionToShard(), opts->getVertexProjections(), opts->getEdgeProjections(), - opts->produceVertices(), opts->useCache()); + opts->produceVertices(), opts->useCache(), + opts->query()); using Provider = SingleServerProvider; if (opts->query().queryOptions().getTraversalProfileLevel() == @@ -679,10 +681,12 @@ std::unique_ptr EnumeratePathsNode::createBlock( auto cache = std::make_shared( opts->query().resourceMonitor()); ClusterBaseProviderOptions forwardProviderOptions(cache, engines(), false, - opts->produceVertices()); + opts->produceVertices(), + opts->query()); forwardProviderOptions.setClearEdgeCacheOnClear(false); ClusterBaseProviderOptions backwardProviderOptions(cache, engines(), true, - opts->produceVertices()); + opts->produceVertices(), + opts->query()); backwardProviderOptions.setClearEdgeCacheOnClear(false); // A comment is in order here: For all cases covered here // (k-shortest-paths, all shortest paths, k-paths) we do not need to diff --git a/arangod/Aql/ExecutionNode/ShortestPathNode.cpp b/arangod/Aql/ExecutionNode/ShortestPathNode.cpp index 3a7ba73faf4b..11425f15f738 100644 --- a/arangod/Aql/ExecutionNode/ShortestPathNode.cpp +++ b/arangod/Aql/ExecutionNode/ShortestPathNode.cpp @@ -437,13 +437,14 @@ std::unique_ptr ShortestPathNode::createBlock( SingleServerBaseProviderOptions forwardProviderOptions( opts->tmpVar(), std::move(usedIndexes), opts->getExpressionCtx(), {}, opts->collectionToShard(), opts->getVertexProjections(), - opts->getEdgeProjections(), opts->produceVertices(), opts->useCache()); + opts->getEdgeProjections(), opts->produceVertices(), opts->useCache(), + opts->query()); SingleServerBaseProviderOptions backwardProviderOptions( opts->tmpVar(), std::move(reversedUsedIndexes), opts->getExpressionCtx(), {}, opts->collectionToShard(), opts->getVertexProjections(), opts->getEdgeProjections(), - opts->produceVertices(), opts->useCache()); + opts->produceVertices(), opts->useCache(), opts->query()); auto usesWeight = checkWeight(forwardProviderOptions, backwardProviderOptions); @@ -511,9 +512,11 @@ std::unique_ptr ShortestPathNode::createBlock( auto cache = std::make_shared( opts->query().resourceMonitor()); ClusterBaseProviderOptions forwardProviderOptions(cache, engines(), false, - opts->produceVertices()); + opts->produceVertices(), + opts->query()); ClusterBaseProviderOptions backwardProviderOptions(cache, engines(), true, - opts->produceVertices()); + opts->produceVertices(), + opts->query()); auto usesWeight = checkWeight(forwardProviderOptions, backwardProviderOptions); diff --git a/arangod/Aql/ExecutionNode/TraversalNode.cpp b/arangod/Aql/ExecutionNode/TraversalNode.cpp index 123e65415774..3fdf08ac3903 100644 --- a/arangod/Aql/ExecutionNode/TraversalNode.cpp +++ b/arangod/Aql/ExecutionNode/TraversalNode.cpp @@ -818,7 +818,8 @@ std::unique_ptr TraversalNode::createBlock( TraverserOptions* opts = this->options(); arangodb::graph::OneSidedEnumeratorOptions options{opts->minDepth, - opts->maxDepth}; + opts->maxDepth, + opts->query()}; /* * PathValidator Disjoint Helper (TODO [GraphRefactor]: Copy from createBlock) * Clean this up as soon we clean up the whole TraversalNode as well. @@ -915,7 +916,8 @@ ClusterBaseProviderOptions TraversalNode::getClusterBaseProviderOptions( opts->produceVertices(), &opts->getExpressionCtx(), filterConditionVariables, - std::move(availableDepthsSpecificConditions)}; + std::move(availableDepthsSpecificConditions), + opts->query()}; } SingleServerBaseProviderOptions @@ -936,7 +938,8 @@ TraversalNode::getSingleServerBaseProviderOptions( opts->getVertexProjections(), opts->getEdgeProjections(), opts->produceVertices(), - opts->useCache()}; + opts->useCache(), + opts->query()}; } /// @brief creates corresponding ExecutionBlock diff --git a/arangod/Graph/Enumerators/OneSidedEnumerator.cpp b/arangod/Graph/Enumerators/OneSidedEnumerator.cpp index 22893dbd8d5b..c55db9aa159b 100644 --- a/arangod/Graph/Enumerators/OneSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/OneSidedEnumerator.cpp @@ -107,8 +107,13 @@ void OneSidedEnumerator::clearProvider() { } template -auto OneSidedEnumerator::computeNeighbourhoodOfNextVertex() - -> void { +void OneSidedEnumerator::computeNeighbourhoodOfNextVertex() { + if (_options.isKilled()) { + // Clear false may sounds misleading, but this means we do not want to keep the path store + clear(false); + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } + // Pull next element from Queue // Do 1 step search TRI_ASSERT(!_queue.isEmpty()); diff --git a/arangod/Graph/Options/OneSidedEnumeratorOptions.cpp b/arangod/Graph/Options/OneSidedEnumeratorOptions.cpp index ccfd8dcbff5b..2bf295218e5d 100644 --- a/arangod/Graph/Options/OneSidedEnumeratorOptions.cpp +++ b/arangod/Graph/Options/OneSidedEnumeratorOptions.cpp @@ -27,9 +27,8 @@ using namespace arangodb; using namespace arangodb::graph; -OneSidedEnumeratorOptions::OneSidedEnumeratorOptions(size_t minDepth, - size_t maxDepth) - : _minDepth(minDepth), _maxDepth(maxDepth) {} +OneSidedEnumeratorOptions::OneSidedEnumeratorOptions(size_t minDepth, size_t maxDepth, aql::QueryContext& query) + : _minDepth(minDepth), _maxDepth(maxDepth), _observer(query) {} OneSidedEnumeratorOptions::~OneSidedEnumeratorOptions() = default; diff --git a/arangod/Graph/Options/OneSidedEnumeratorOptions.h b/arangod/Graph/Options/OneSidedEnumeratorOptions.h index fcfd34d8f9a1..c1519906e55d 100644 --- a/arangod/Graph/Options/OneSidedEnumeratorOptions.h +++ b/arangod/Graph/Options/OneSidedEnumeratorOptions.h @@ -24,20 +24,25 @@ #pragma once +#include "Graph/Options/QueryContextObserver.h" +// TODO HEIKO: do not include h file here, use forward declaration instead +// TODO HEIKO: do not include h file here, use forward declaration instead #include namespace arangodb::graph { struct OneSidedEnumeratorOptions { public: - OneSidedEnumeratorOptions(size_t minDepth, size_t maxDepth); + OneSidedEnumeratorOptions(size_t minDepth, size_t maxDepth, aql::QueryContext& query); ~OneSidedEnumeratorOptions(); [[nodiscard]] size_t getMinDepth() const noexcept; [[nodiscard]] size_t getMaxDepth() const noexcept; + [[nodiscard]] bool isKilled() const noexcept { return _observer.isKilled(); } private: size_t const _minDepth; size_t const _maxDepth; + QueryContextObserver _observer; }; } // namespace arangodb::graph diff --git a/arangod/Graph/Options/QueryContextObserver.h b/arangod/Graph/Options/QueryContextObserver.h new file mode 100644 index 000000000000..d7afffc0cab9 --- /dev/null +++ b/arangod/Graph/Options/QueryContextObserver.h @@ -0,0 +1,52 @@ +//////////////////////////////////////////////////////////////////////////////// +/// DISCLAIMER +/// +/// Copyright 2014-2024 ArangoDB GmbH, Cologne, Germany +/// Copyright 2004-2014 triAGENS GmbH, Cologne, Germany +/// +/// Licensed under the Business Source License 1.1 (the "License"); +/// you may not use this file except in compliance with the License. +/// You may obtain a copy of the License at +/// +/// https://github.com/arangodb/arangodb/blob/devel/LICENSE +/// +/// 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 Michael Hackstein +/// @author Heiko Kernbach +//////////////////////////////////////////////////////////////////////////////// + +#pragma once + +#include "Aql/QueryContext.h" + +// This class serves as a wrapper around QueryContext to explicitly track where query killing +// is being used in the graph traversal code. It provides a single point of access to check +// if a query has been killed, making it easier to maintain and modify the query killing +// behavior if needed. +// +// While this adds a small layer of indirection, it helps with code clarity and maintainability. +// If profiling shows this wrapper causes significant overhead, we can remove it and use +// QueryContext directly. +// +// We can change this or discuss if this approach is not liked. + +namespace arangodb::graph { + +class QueryContextObserver { + public: + explicit QueryContextObserver(aql::QueryContext& query) : _query(query) {} + + [[nodiscard]] bool isKilled() const { return _query.killed(); } + + private: + aql::QueryContext& _query; +}; + +} // namespace arangodb::graph \ No newline at end of file diff --git a/arangod/Graph/Providers/BaseProviderOptions.cpp b/arangod/Graph/Providers/BaseProviderOptions.cpp index ee005d54239f..a0cc669dcd4e 100644 --- a/arangod/Graph/Providers/BaseProviderOptions.cpp +++ b/arangod/Graph/Providers/BaseProviderOptions.cpp @@ -86,7 +86,8 @@ SingleServerBaseProviderOptions::SingleServerBaseProviderOptions( MonitoredCollectionToShardMap const& collectionToShardMap, aql::Projections const& vertexProjections, aql::Projections const& edgeProjections, bool produceVertices, - bool useCache) + bool useCache, + aql::QueryContext& query) : _temporaryVariable(tmpVar), _indexInformation(std::move(indexInfo)), _expressionContext(expressionContext), @@ -96,7 +97,8 @@ SingleServerBaseProviderOptions::SingleServerBaseProviderOptions( _vertexProjections{vertexProjections}, _edgeProjections{edgeProjections}, _produceVertices(produceVertices), - _useCache(useCache) {} + _useCache(useCache), + _queryObserver(query) {} aql::Variable const* SingleServerBaseProviderOptions::tmpVar() const { return _temporaryVariable; @@ -169,13 +171,15 @@ void SingleServerBaseProviderOptions::unPrepareContext() { ClusterBaseProviderOptions::ClusterBaseProviderOptions( std::shared_ptr cache, std::unordered_map const* engines, bool backward, - bool produceVertices) + bool produceVertices, + aql::QueryContext& query) : _cache(std::move(cache)), _engines(engines), _backward(backward), _produceVertices(produceVertices), _expressionContext(nullptr), - _weightCallback(std::nullopt) { + _weightCallback(std::nullopt), + _queryObserver(query) { TRI_ASSERT(_cache != nullptr); TRI_ASSERT(_engines != nullptr); } @@ -186,7 +190,8 @@ ClusterBaseProviderOptions::ClusterBaseProviderOptions( bool produceVertices, aql::FixedVarExpressionContext* expressionContext, std::vector> filterConditionVariables, - std::unordered_set availableDepthsSpecificConditions) + std::unordered_set availableDepthsSpecificConditions, + aql::QueryContext& query) : _cache(std::move(cache)), _engines(engines), _backward(backward), @@ -195,7 +200,8 @@ ClusterBaseProviderOptions::ClusterBaseProviderOptions( _filterConditionVariables(filterConditionVariables), _weightCallback(std::nullopt), _availableDepthsSpecificConditions( - std::move(availableDepthsSpecificConditions)) { + std::move(availableDepthsSpecificConditions)), + _queryObserver(query) { TRI_ASSERT(_cache != nullptr); TRI_ASSERT(_engines != nullptr); } diff --git a/arangod/Graph/Providers/BaseProviderOptions.h b/arangod/Graph/Providers/BaseProviderOptions.h index 654562dacabb..2d0f79fecc89 100644 --- a/arangod/Graph/Providers/BaseProviderOptions.h +++ b/arangod/Graph/Providers/BaseProviderOptions.h @@ -28,8 +28,10 @@ #include "Aql/InAndOutRowExpressionContext.h" #include "Aql/NonConstExpressionContainer.h" #include "Aql/Projections.h" +#include "Aql/QueryContext.h" #include "Basics/MemoryTypes/MemoryTypes.h" #include "Graph/Cache/RefactoredClusterTraverserCache.h" +#include "Graph/Options/QueryContextObserver.h" #include "Transaction/Methods.h" #ifdef USE_ENTERPRISE @@ -45,7 +47,6 @@ namespace arangodb { namespace aql { -class QueryContext; struct AstNode; class InputAqlItemRow; } // namespace aql @@ -99,10 +100,10 @@ struct SingleServerBaseProviderOptions { MonitoredCollectionToShardMap const& collectionToShardMap, aql::Projections const& vertexProjections, aql::Projections const& edgeProjections, bool produceVertices, - bool useCache); + bool useCache, + aql::QueryContext& query); - SingleServerBaseProviderOptions(SingleServerBaseProviderOptions const&) = - delete; + SingleServerBaseProviderOptions(SingleServerBaseProviderOptions const&) = delete; SingleServerBaseProviderOptions(SingleServerBaseProviderOptions&&) = default; aql::Variable const* tmpVar() const; @@ -132,6 +133,10 @@ struct SingleServerBaseProviderOptions { aql::Projections const& getEdgeProjections() const; + bool isKilled() const noexcept { + return _queryObserver.isKilled(); + } + private: // The temporary Variable used in the Indexes aql::Variable const* _temporaryVariable; @@ -171,6 +176,8 @@ struct SingleServerBaseProviderOptions { bool const _produceVertices; bool const _useCache; + + QueryContextObserver _queryObserver; }; struct ClusterBaseProviderOptions { @@ -181,7 +188,8 @@ struct ClusterBaseProviderOptions { ClusterBaseProviderOptions( std::shared_ptr cache, std::unordered_map const* engines, bool backward, - bool produceVertices); + bool produceVertices, + aql::QueryContext& query); ClusterBaseProviderOptions( std::shared_ptr cache, @@ -189,7 +197,8 @@ struct ClusterBaseProviderOptions { bool produceVertices, aql::FixedVarExpressionContext* expressionContext, std::vector> filterConditionVariables, - std::unordered_set availableDepthsSpecificConditions); + std::unordered_set availableDepthsSpecificConditions, + aql::QueryContext& query); RefactoredClusterTraverserCache* getCache(); @@ -227,6 +236,10 @@ struct ClusterBaseProviderOptions { _clearEdgeCacheOnClear = flag; } + bool isKilled() const noexcept { + return _queryObserver.isKilled(); + } + private: std::shared_ptr _cache; @@ -264,6 +277,8 @@ struct ClusterBaseProviderOptions { // not true and hurts performance. Therefore, for these cases it is possible // to set this flag to `false` to retain cached data across calls to `clear`. bool _clearEdgeCacheOnClear = true; + + QueryContextObserver _queryObserver; }; } // namespace graph diff --git a/arangod/Graph/Providers/ClusterProvider.cpp b/arangod/Graph/Providers/ClusterProvider.cpp index f182ad72403b..38b57b364cf5 100644 --- a/arangod/Graph/Providers/ClusterProvider.cpp +++ b/arangod/Graph/Providers/ClusterProvider.cpp @@ -455,6 +455,10 @@ Result ClusterProvider::fetchEdgesFromEngines(Step* step) { template auto ClusterProvider::fetchVertices( std::vector const& looseEnds) -> std::vector { + if (_opts.isKilled()) { + clear(); + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } std::vector result{}; if (!looseEnds.empty()) { @@ -481,6 +485,11 @@ auto ClusterProvider::fetchVertices( template auto ClusterProvider::fetchEdges( std::vector const& fetchedVertices) -> Result { + if (_opts.isKilled()) { + clear(); + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } + for (auto const& step : fetchedVertices) { if (!_vertexConnectedEdges.contains(step->getVertex().getID())) { auto res = fetchEdgesFromEngines(step); @@ -511,6 +520,11 @@ template auto ClusterProvider::expand( Step const& step, size_t previous, std::function const& callback) -> void { + if (_opts.isKilled()) { + clear(); + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } + TRI_ASSERT(!step.isLooseEnd()); auto const& vertex = step.getVertex(); diff --git a/arangod/Graph/Providers/SingleServerProvider.cpp b/arangod/Graph/Providers/SingleServerProvider.cpp index 0858561c9d0c..3391980213d3 100644 --- a/arangod/Graph/Providers/SingleServerProvider.cpp +++ b/arangod/Graph/Providers/SingleServerProvider.cpp @@ -120,6 +120,11 @@ auto SingleServerProvider::expand( TRI_ASSERT(!step.isLooseEnd()); auto const& vertex = step.getVertex(); + if (_opts.isKilled()) { + clear(); + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } + LOG_TOPIC("c9169", TRACE, Logger::GRAPHS) << " Expanding " << vertex.getID(); diff --git a/tests/Graph/DFSFinderTest.cpp b/tests/Graph/DFSFinderTest.cpp index 95c954b03ee4..a6559c6b80bd 100644 --- a/tests/Graph/DFSFinderTest.cpp +++ b/tests/Graph/DFSFinderTest.cpp @@ -174,7 +174,7 @@ class DFSFinderTest } auto pathFinder(size_t minDepth, size_t maxDepth) -> DFSFinder { - arangodb::graph::OneSidedEnumeratorOptions options{minDepth, maxDepth}; + arangodb::graph::OneSidedEnumeratorOptions options{minDepth, maxDepth, *_query.get()}; PathValidatorOptions validatorOpts{&_tmpVar, _expressionContext}; return DFSFinder( {*_query.get(), diff --git a/tests/Graph/GenericGraphProviderTest.cpp b/tests/Graph/GenericGraphProviderTest.cpp index 34497ce42f66..e8a6784af1a8 100644 --- a/tests/Graph/GenericGraphProviderTest.cpp +++ b/tests/Graph/GenericGraphProviderTest.cpp @@ -37,7 +37,9 @@ #include "Graph/Steps/SingleServerProviderStep.h" #include "Graph/TraverserOptions.h" +#include #include +#include using namespace arangodb; using namespace arangodb::tests; @@ -138,7 +140,8 @@ class GraphProviderTest : public ::testing::Test { std::move(usedIndexes), std::unordered_map>{}), *_expressionContext.get(), {}, _emptyShardMap, _vertexProjections, - _edgeProjections, /*produceVertices*/ true, /*useCache*/ true); + _edgeProjections, /*produceVertices*/ true, /*useCache*/ true, + *query); return SingleServerProvider( *query.get(), std::move(opts), resourceMonitor); } @@ -241,7 +244,7 @@ class GraphProviderTest : public ::testing::Test { std::make_shared(resourceMonitor); ClusterBaseProviderOptions opts(clusterCache, clusterEngines.get(), false, - true); + true, *query); return ClusterProvider(*query.get(), std::move(opts), resourceMonitor); } @@ -451,6 +454,53 @@ TYPED_TEST(GraphProviderTest, destroy_engines) { } } +TYPED_TEST(GraphProviderTest, should_cancel_traversal_when_query_is_aborted) { + // Create a graph with 4 nodes in a circle (bidirectional edges) + MockGraph g{}; + // Add edges in a circle (0->1->2->3->0) and back + g.addEdge(0, 1); + g.addEdge(1, 2); + g.addEdge(2, 3); + g.addEdge(3, 0); + // Add reverse edges + g.addEdge(1, 0); + g.addEdge(2, 1); + g.addEdge(3, 2); + g.addEdge(0, 3); + + std::unordered_map>> const& + expectedVerticesEdgesBundleToFetch = {{0, {}}}; + + auto testee = this->makeProvider(g, expectedVerticesEdgesBundleToFetch); + std::string startString = g.vertexToId(0); + VPackHashedStringRef startH{startString.c_str(), + static_cast(startString.length())}; + auto start = testee.startVertex(startH); + + if (start.isLooseEnd()) { + std::vector looseEnds{}; + looseEnds.emplace_back(&start); + auto futures = testee.fetch(looseEnds); + auto steps = futures.waitAndGet(); + } + + std::thread abortThread([this]() { + this->query->kill(); + std::this_thread::sleep_for(std::chrono::milliseconds(300)); + }); + + EXPECT_THROW( + { + while (true) { + testee.expand(start, 0, + [](typename decltype(testee)::Step n) -> void {}); + } + }, + arangodb::basics::Exception); + + abortThread.join(); +} + } // namespace generic_graph_provider_test } // namespace tests } // namespace arangodb diff --git a/tests/Graph/SingleServerProviderTest.cpp b/tests/Graph/SingleServerProviderTest.cpp index d1ee8dceb19f..9c1077e26b61 100644 --- a/tests/Graph/SingleServerProviderTest.cpp +++ b/tests/Graph/SingleServerProviderTest.cpp @@ -119,7 +119,7 @@ class SingleServerProviderTest : public ::testing::Test { std::move(usedIndexes), std::unordered_map>{}), *_expressionContext.get(), {}, _emptyShardMap, _vertexProjections, - _edgeProjections, /*produceVertices*/ true, /*useCache*/ true); + _edgeProjections, /*produceVertices*/ true, /*useCache*/ true, *query); return {*query.get(), std::move(opts), _resourceMonitor}; } diff --git a/tests/Mocks/MockGraphProvider.cpp b/tests/Mocks/MockGraphProvider.cpp index 53cd2c0fb8a2..de9218724b50 100644 --- a/tests/Mocks/MockGraphProvider.cpp +++ b/tests/Mocks/MockGraphProvider.cpp @@ -101,7 +101,8 @@ MockGraphProvider::MockGraphProvider(arangodb::aql::QueryContext& queryContext, : _trx(queryContext.newTrxContext()), _reverse(opts.reverse()), _looseEnds(opts.looseEnds()), - _stats{} { + _stats{}, + _queryContext(queryContext) { for (auto const& it : opts.data().edges()) { _fromIndex[it._from].push_back(it); _toIndex[it._to].push_back(it); @@ -159,9 +160,16 @@ auto MockGraphProvider::fetch(std::vector const& looseEnds) auto MockGraphProvider::expand(Step const& step, size_t previous, std::function callback) -> void { + if (isKilled()) { + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } + std::vector results{}; results = expand(step, previous); for (auto const& s : results) { + if (isKilled()) { + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } callback(s); } } diff --git a/tests/Mocks/MockGraphProvider.h b/tests/Mocks/MockGraphProvider.h index ca3dc5ead1a7..bb3021f7dd6e 100644 --- a/tests/Mocks/MockGraphProvider.h +++ b/tests/Mocks/MockGraphProvider.h @@ -36,9 +36,11 @@ #include "Transaction/Hints.h" #include "Transaction/Methods.h" +#include "Aql/QueryContext.h" #include "Graph/Providers/BaseStep.h" - -#include +#include "Graph/EdgeDocumentToken.h" +#include "Aql/TraversalStats.h" +#include "VocBase/vocbase.h" namespace arangodb { @@ -288,7 +290,7 @@ class MockGraphProvider { ~MockGraphProvider(); MockGraphProvider& operator=(MockGraphProvider const&) = delete; - MockGraphProvider& operator=(MockGraphProvider&&) = default; + MockGraphProvider& operator=(MockGraphProvider&&) = delete; void destroyEngines(){}; auto startVertex(VertexType vertex, size_t depth = 0, double weight = 0.0) @@ -339,6 +341,8 @@ class MockGraphProvider { _weightCallback = std::move(callback); } + bool isKilled() const { return _queryContext.killed(); } + private: auto decideProcessable() const -> bool; @@ -351,6 +355,7 @@ class MockGraphProvider { arangodb::aql::TraversalStats _stats; // Optional callback to compute the weight of an edge. std::optional _weightCallback; + arangodb::aql::QueryContext& _queryContext; }; } // namespace graph } // namespace tests From 9f751e8a2f92fba6fdbfc444eb88339e44233fee Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Tue, 6 May 2025 12:48:30 +0200 Subject: [PATCH 02/28] added bidirectionalCircle as new graph in graph test suite --- .../aql-graph-traversal-generic-graphs.js | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index acc2b4e8989c..59598100e36e 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -1161,6 +1161,60 @@ protoGraphs.moreAdvancedPath = new ProtoGraph("moreAdvancedPath", [ ); } +/* + * B + * ↕ ↕ + * A C + * ↕ ↕ + * D + */ +protoGraphs.bidirectionalCircle = new ProtoGraph("bidirectionalCircle", [ + ["A", "B", 1], + ["B", "A", 1], + ["B", "C", 1], + ["C", "B", 1], + ["C", "D", 1], + ["D", "C", 1], + ["D", "A", 1], + ["A", "D", 1] + ], + [1, 2, 4], + [ + { + numberOfShards: 1, + vertexSharding: + [ + ["A", 0], + ["B", 0], + ["C", 0], + ["D", 0] + ] + }, + { + numberOfShards: 2, + vertexSharding: + [ + ["A", 0], + ["B", 1], + ["C", 0], + ["D", 1] + ] + }, + { + numberOfShards: 4, + vertexSharding: + [ + ["A", 0], + ["B", 1], + ["C", 2], + ["D", 3] + ] + } + ], + [], + true +); + exports.ProtoGraph = ProtoGraph; exports.protoGraphs = protoGraphs; exports.TestVariants = TestVariants; From 51be50679f711ec758389bcc720a14ce5059b45e Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Tue, 6 May 2025 13:21:53 +0200 Subject: [PATCH 03/28] added hugeCompleteGraph --- .../aql-graph-traversal-generic-graphs.js | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index 59598100e36e..cc5abddb7f83 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -841,6 +841,73 @@ protoGraphs.completeGraph = new ProtoGraph("completeGraph", [ ] ); +// Generate node names +const generateNodeNames = (count) => { + const alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; + const nodes = []; + + // First add single letter nodes + for (let i = 0; i < Math.min(count, alphabet.length); i++) { + nodes.push(alphabet[i]); + } + + // If we need more nodes, add two-letter combinations + if (count > alphabet.length) { + for (let i = 0; i < alphabet.length && nodes.length < count; i++) { + for (let j = 0; j < alphabet.length && nodes.length < count; j++) { + nodes.push(alphabet[i] + alphabet[j]); + } + } + } + + return nodes; +}; + +// Generate edges for complete graph +const generateCompleteGraphEdges = (nodes) => { + const edges = []; + for (let i = 0; i < nodes.length; i++) { + for (let j = 0; j < nodes.length; j++) { + if (i !== j) { // Don't create self-loops + // Generate random weight between 1 and 5 + const weight = Math.floor(Math.random() * 5) + 1; + edges.push([nodes[i], nodes[j], weight]); + } + } + } + return edges; +}; + +// Create the huge complete graph with 100 nodes +/* + * B + * ↙↗ ↑ ↖↘ + * A ← → C // Demonstration of the complete graph + * ↖↘ ↓ ↙↗ // Note: Consists out of 100 nodes + * D + */ +const hugeCompleteGraphNodes = generateNodeNames(100); +const hugeCompleteGraphEdges = generateCompleteGraphEdges(hugeCompleteGraphNodes); + +protoGraphs.hugeCompleteGraph = new ProtoGraph("hugeCompleteGraph", + hugeCompleteGraphEdges, + [1, 2, 5], + [ + { + numberOfShards: 1, + vertexSharding: hugeCompleteGraphNodes.map((node, index) => [node, 0]) + }, + { + numberOfShards: 2, + vertexSharding: hugeCompleteGraphNodes.map((node, index) => [node, index % 2]) + }, + { + numberOfShards: 5, + vertexSharding: hugeCompleteGraphNodes.map((node, index) => [node, index % 5]) + } + ] +); + /* * * From 8f6659adf84bd76192fb3bdcdb5384c141b881c6 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Tue, 13 May 2025 16:10:23 +0200 Subject: [PATCH 04/28] added query cancel observer to twosided enumerator as well, so kpaths shortest paths and kshortestpath, also added tests --- .../Aql/ExecutionNode/EnumeratePathsNode.cpp | 2 +- .../Aql/ExecutionNode/ShortestPathNode.cpp | 2 +- .../Graph/Enumerators/TwoSidedEnumerator.cpp | 15 +- .../Options/TwoSidedEnumeratorOptions.cpp | 5 +- .../Graph/Options/TwoSidedEnumeratorOptions.h | 10 +- ...h-traversal-generic-bidirectional-tests.js | 284 ++++++++++++++++++ .../aql-graph-traversal-generic-graphs.js | 81 +++++ .../aql-graph-traversal-generic-tests.js | 26 ++ tests/Graph/AllShortestPathsFinderTest.cpp | 2 +- tests/Graph/KPathFinderTest.cpp | 2 +- tests/Graph/KShortestPathsFinderTest.cpp | 4 +- tests/Graph/WeightedShortestPathTest.cpp | 2 +- 12 files changed, 420 insertions(+), 15 deletions(-) create mode 100644 js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js diff --git a/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp b/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp index 196d8f7207cf..e1951841c036 100644 --- a/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp +++ b/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp @@ -432,7 +432,7 @@ std::unique_ptr EnumeratePathsNode::createBlock( TRI_ASSERT(pathType() != arangodb::graph::PathType::Type::ShortestPath); arangodb::graph::TwoSidedEnumeratorOptions enumeratorOptions{ - opts->getMinDepth(), opts->getMaxDepth(), pathType()}; + opts->getMinDepth(), opts->getMaxDepth(), pathType(), opts->query()}; PathValidatorOptions validatorOptions(opts->tmpVar(), opts->getExpressionCtx()); diff --git a/arangod/Aql/ExecutionNode/ShortestPathNode.cpp b/arangod/Aql/ExecutionNode/ShortestPathNode.cpp index 11425f15f738..2e7f89328768 100644 --- a/arangod/Aql/ExecutionNode/ShortestPathNode.cpp +++ b/arangod/Aql/ExecutionNode/ShortestPathNode.cpp @@ -419,7 +419,7 @@ std::unique_ptr ShortestPathNode::createBlock( arangodb::graph::TwoSidedEnumeratorOptions enumeratorOptions{ 0, std::numeric_limits::max(), - arangodb::graph::PathType::Type::ShortestPath}; + arangodb::graph::PathType::Type::ShortestPath, opts->query()}; PathValidatorOptions validatorOptions(opts->tmpVar(), opts->getExpressionCtx()); diff --git a/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp b/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp index 60b6ede7d435..e0f37bb1530e 100644 --- a/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp @@ -197,6 +197,11 @@ template:: Ball::computeNeighbourhoodOfNextVertex(Ball& other, ResultList& results) -> void { + if (_graphOptions.isKilled()) { + clear(); + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } + // Pull next element from Queue // Do 1 step search TRI_ASSERT(!_queue.isEmpty()); @@ -248,8 +253,8 @@ auto TwoSidedEnumerator:: template -void TwoSidedEnumerator:: - Ball::testDepthZero(Ball& other, ResultList& results) { +void TwoSidedEnumerator::Ball::testDepthZero(Ball& other, ResultList& results) { for (auto const& step : _shell) { other.matchResultsInShell(step, results, _validator); } @@ -286,9 +291,9 @@ auto TwoSidedEnumerator:: template -auto TwoSidedEnumerator:: - Ball::buildPath(Step const& vertexInShell, - PathResult& path) -> void { +auto TwoSidedEnumerator::Ball::buildPath(Step const& vertexInShell, + PathResult& path) -> void { if (_direction == FORWARD) { _interior.buildPath(vertexInShell, path); } else { diff --git a/arangod/Graph/Options/TwoSidedEnumeratorOptions.cpp b/arangod/Graph/Options/TwoSidedEnumeratorOptions.cpp index 894b52305888..7c1e09416adb 100644 --- a/arangod/Graph/Options/TwoSidedEnumeratorOptions.cpp +++ b/arangod/Graph/Options/TwoSidedEnumeratorOptions.cpp @@ -29,8 +29,9 @@ using namespace arangodb::graph; TwoSidedEnumeratorOptions::TwoSidedEnumeratorOptions(size_t minDepth, size_t maxDepth, - PathType::Type pathType) - : _minDepth(minDepth), _maxDepth(maxDepth), _pathType(pathType) { + PathType::Type pathType, + aql::QueryContext& query) + : _minDepth(minDepth), _maxDepth(maxDepth), _pathType(pathType), _observer(query) { if (getPathType() == PathType::Type::AllShortestPaths) { setStopAtFirstDepth(true); } else if (getPathType() == PathType::Type::ShortestPath) { diff --git a/arangod/Graph/Options/TwoSidedEnumeratorOptions.h b/arangod/Graph/Options/TwoSidedEnumeratorOptions.h index 226a5222232d..a998e2186816 100644 --- a/arangod/Graph/Options/TwoSidedEnumeratorOptions.h +++ b/arangod/Graph/Options/TwoSidedEnumeratorOptions.h @@ -25,17 +25,23 @@ #pragma once #include "Graph/PathType.h" +#include "Graph/Options/QueryContextObserver.h" #include #include namespace arangodb { + +namespace aql { +class QueryContext; +} + namespace graph { struct TwoSidedEnumeratorOptions { public: TwoSidedEnumeratorOptions(size_t minDepth, size_t maxDepth, - PathType::Type pathType); + PathType::Type pathType, aql::QueryContext& query); ~TwoSidedEnumeratorOptions(); @@ -46,6 +52,7 @@ struct TwoSidedEnumeratorOptions { [[nodiscard]] PathType::Type getPathType() const; [[nodiscard]] bool getStopAtFirstDepth() const; [[nodiscard]] bool onlyProduceOnePath() const; + [[nodiscard]] bool isKilled() const noexcept { return _observer.isKilled(); } void setStopAtFirstDepth(bool stopAtFirstDepth); void setOnlyProduceOnePath(bool onlyProduceOnePath); @@ -57,6 +64,7 @@ struct TwoSidedEnumeratorOptions { bool _stopAtFirstDepth{false}; bool _onlyProduceOnePath{false}; PathType::Type _pathType; + QueryContextObserver _observer; }; } // namespace graph } // namespace arangodb diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js new file mode 100644 index 000000000000..c6648911e527 --- /dev/null +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js @@ -0,0 +1,284 @@ +const jsunity = require("jsunity"); +const {assertTrue} = jsunity.jsUnity.assertions; +const protoGraphs = require('@arangodb/testutils/aql-graph-traversal-generic-graphs').protoGraphs; +const arango = internal.arango; + +// seconds to add to execution time for verification +// This is to account for the time it takes for the query to be scheduled and executed +// and for the query to be killed +const VERIFICATION_TIME_BUFFER = 3; + +const localHelper = { + getRunningQueries: function() { + return arango.GET('/_api/query/current'); + }, + normalizeQueryString: function(query) { + // Remove extra whitespace, newlines and normalize spaces + return query.replace(/\s+/g, ' ').trim(); + }, + checkRunningQuery: function(queryString, maxAttempts = 10, debug = false) { + const normalizedQueryString = this.normalizeQueryString(queryString); + if (debug) { + print("Checking for running query:", normalizedQueryString); + } + for (let i = 0; i < maxAttempts; i++) { + const runningQueries = this.getRunningQueries(); + if (debug) { + print("Attempt", i + 1, "of", maxAttempts); + print("Number of running queries:", runningQueries.length); + } + const matchingQuery = runningQueries.find(query => + this.normalizeQueryString(query.query) === normalizedQueryString + ); + if (matchingQuery) { + if (debug) { + print("Found matching query with ID:", matchingQuery.id); + } + return matchingQuery; + } + if (debug) { + print("No matching query found, waiting..."); + } + internal.sleep(1); + } + if (debug) { + print("Failed to find running query after", maxAttempts, "attempts"); + } + assertTrue(false, "Query not found"); + }, + waitForQueryTermination: function(queryId, maxAttempts = 10) { + for (let i = 0; i < maxAttempts; i++) { + const runningQueries = this.getRunningQueries(); + const queryStillRunning = runningQueries.some(query => query.id === queryId); + if (!queryStillRunning) { + return true; + } + internal.sleep(1); + } + assertTrue(false, "Query did not terminate within expected time"); + }, + killQuery: function(queryId) { + const dbName = db._name(); + const response = arango.DELETE('/_db/' + dbName + '/_api/query/' + queryId); + assertEqual(response.code, 200); + return response; + }, + executeAsyncQuery: function(queryString, debug = false, disableAsyncHeader = false) { + // This helper will just accept the query. It will return the async id. + if (debug) { + print("Executing async query:", this.normalizeQueryString(queryString)); + } + const headers = disableAsyncHeader ? {} : {'x-arango-async': 'store'}; + const response = arango.POST_RAW('/_api/cursor', + JSON.stringify({ + query: queryString, + bindVars: {} + }), + headers + ); + if (debug) { + if (disableAsyncHeader) { + print(response) + } + print("Async query response code:", response.code); + print("Async query ID:", response.headers['x-arango-async-id']); + } + assertEqual(response.code, 202); + assertTrue(response.headers.hasOwnProperty("x-arango-async-id")); + + return response.headers['x-arango-async-id']; + }, + checkJobStatus: function(jobId, maxAttempts = 10) { + for (let i = 0; i < maxAttempts; i++) { + const response = arango.GET_RAW('/_api/job/' + jobId); + if (response.code === 200) { + return response; + } + internal.sleep(1); + } + assertTrue(false, "Job not found"); + }, + testKillLongRunningQuery: function(queryString, debug = false, disableAsyncHeader = false) { + // First run - measure execution time + const startTime = Date.now(); + if (debug) { + print("Starting first query execution..."); + } + const queryJobId = this.executeAsyncQuery(queryString, debug, disableAsyncHeader); + if (debug) { + print("First query job ID:", queryJobId); + } + const runningQuery = this.checkRunningQuery(queryString, 10, debug); + const queryId = runningQuery.id; + if (debug) { + print("First query ID:", queryId); + } + this.killQuery(queryId); + this.checkJobStatus(queryJobId); + this.waitForQueryTermination(queryId); + const firstExecutionTime = (Date.now() - startTime) / 1000; // Convert to seconds + if (debug) { + print("First query execution time:", firstExecutionTime, "seconds"); + } + + // Second run - verify query stays running + if (debug) { + print("Starting second query execution..."); + } + const startTime2 = Date.now(); + const queryJobId2 = this.executeAsyncQuery(queryString, debug, disableAsyncHeader); + if (debug) { + print("Second query job ID:", queryJobId2); + } + const runningQuery2 = this.checkRunningQuery(queryString, 10, debug); + const queryId2 = runningQuery2.id; + if (debug) { + print("Second query ID:", queryId2); + } + + // Wait for executionTime + VERIFICATION_TIME_BUFFER seconds while checking if query is still running + const verificationTime = firstExecutionTime + VERIFICATION_TIME_BUFFER; + if (debug) { + print("Verification time:", verificationTime, "seconds (" + VERIFICATION_TIME_BUFFER + " seconds more than the first query)"); + } + const startVerification = Date.now(); + while ((Date.now() - startVerification) / 1000 < verificationTime) { + const runningQueries = this.getRunningQueries(); + const queryStillRunning = runningQueries.some(query => query.id === queryId2); + if (debug) { + print("Query still running:", queryStillRunning); + } + assertTrue(queryStillRunning, "Query terminated before verification time"); + internal.sleep(1); + } + + // Now kill the query and verify termination + if (debug) { + print("Killing second query..."); + } + this.killQuery(queryId2); + this.checkJobStatus(queryJobId2); + this.waitForQueryTermination(queryId2); + + // Verify that second query ran longer than first query + const secondExecutionTime = (Date.now() - startTime2) / 1000; + if (debug) { + print("Second query execution time:", secondExecutionTime, "seconds"); + } + assertTrue(secondExecutionTime > firstExecutionTime, + `Second query (${secondExecutionTime}s) did not run longer than first query (${firstExecutionTime}s). Kill may not have worked.`); + + if (debug) { + print("Test completed successfully"); + } + } +} + +/* + Bidirectional Circle + - DFS + - BFS + - Weighted Path +*/ + +function testBidirectionalCircleDfsLongRunning(testGraph) { + assertTrue(testGraph.name().startsWith(protoGraphs.bidirectionalCircle.name())); + + const queryString = ` + FOR v, e IN 1..999 OUTBOUND "${testGraph.vertex('A')}" + GRAPH ${testGraph.name()} + OPTIONS {order: "dfs", uniqueVertices: "none", uniqueEdges: "none"} + RETURN v.key + `; + + localHelper.testKillLongRunningQuery(queryString); +} + +function testBidirectionalCircleBfsLongRunning(testGraph) { + assertTrue(testGraph.name().startsWith(protoGraphs.bidirectionalCircle.name())); + + const queryString = ` + FOR v, e IN 1..999 OUTBOUND "${testGraph.vertex('A')}" + GRAPH ${testGraph.name()} + OPTIONS {order: "bfs", uniqueVertices: "none", uniqueEdges: "none"} + RETURN v.key + `; + + localHelper.testKillLongRunningQuery(queryString); +} + +function testBidirectionalCircleWeightedPathLongRunning(testGraph) { + assertTrue(testGraph.name().startsWith(protoGraphs.bidirectionalCircle.name())); + + const queryString = ` + FOR v, e IN 1..999 OUTBOUND "${testGraph.vertex('A')}" + GRAPH ${testGraph.name()} + OPTIONS {order: "weighted", weightAttribute: "${testGraph.weightAttribute()}", uniqueVertices: "none", uniqueEdges: "none"} + RETURN v.key + `; + + localHelper.testKillLongRunningQuery(queryString); +} + +/* + HugeCompleteGraph + - K Paths +*/ + +function testHugeCompleteGraphKPathsLongRunning(testGraph) { + assertTrue(testGraph.name().startsWith(protoGraphs.hugeCompleteGraph.name())); + + const queryString = ` + FOR path IN 1..999 ANY K_PATHS "${testGraph.vertex('A')}" TO "${testGraph.vertex('B')}" + GRAPH ${testGraph.name()} + OPTIONS {useCache: false} + RETURN path + `; + + localHelper.testKillLongRunningQuery(queryString); +} + +/* + HugeGridGraph + - Shortest Path + - All Shortest Paths +*/ + +function testHugeGridGraphShortestPathLongRunning(testGraph) { + assertTrue(testGraph.name().startsWith(protoGraphs.hugeGridGraph.name())); + + const queryString = ` + FOR v, e IN OUTBOUND SHORTEST_PATH "${testGraph.vertex('1')}" TO "${testGraph.vertex('1000000')}" + GRAPH ${testGraph.name()} + OPTIONS {useCache: false} + RETURN v.key + `; + + const debug = false; + localHelper.testKillLongRunningQuery(queryString, debug, debug); +} + +function testHugeGridGraphAllShortestPathsLongRunning(testGraph) { + assertTrue(testGraph.name().startsWith(protoGraphs.hugeGridGraph.name())); + + const queryString = ` + FOR path IN ANY ALL_SHORTEST_PATHS "${testGraph.vertex('1')}" TO "${testGraph.vertex('1000000')}" + GRAPH ${testGraph.name()} + OPTIONS {useCache: false} + RETURN path + `; + + localHelper.testKillLongRunningQuery(queryString); +} + +// DFS, BFS, Weighted Path +exports.testBidirectionalCircleDfsLongRunning = testBidirectionalCircleDfsLongRunning; +exports.testBidirectionalCircleBfsLongRunning = testBidirectionalCircleBfsLongRunning; +exports.testBidirectionalCircleWeightedPathLongRunning = testBidirectionalCircleWeightedPathLongRunning; + +// K Paths +exports.testHugeCompleteGraphKPathsLongRunning = testHugeCompleteGraphKPathsLongRunning; + +// Shortest Path, All Shortest Paths +exports.testHugeGridGraphShortestPathLongRunning = testHugeGridGraphShortestPathLongRunning; +exports.testHugeGridGraphAllShortestPathsLongRunning = testHugeGridGraphAllShortestPathsLongRunning; \ No newline at end of file diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index cc5abddb7f83..0cf18d74f9e6 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -239,6 +239,19 @@ class TestGraph { this.verticesByName = TestGraph._fillGraph(this.graphName, this.edges, db[this.vn], db[this.en], this.unconnectedVertices, vertexSharding, this.addProjectionPayload); db[this.en].ensureIndex({type: "persistent", fields: ["_from", graphIndexedAttribute]}); + + // Print first and last 10 vertices for debugging + print("First 10 vertices in graph " + this.graphName + ":"); + const first10Vertices = Object.entries(this.verticesByName).slice(0, 10); + first10Vertices.forEach(([key, id]) => { + print(` ${key}: ${id} (node name: ${key})`); + }); + + print("\nLast 10 vertices in graph " + this.graphName + ":"); + const last10Vertices = Object.entries(this.verticesByName).slice(-10); + last10Vertices.forEach(([key, id]) => { + print(` ${key}: ${id} (node name: ${key})`); + }); } name() { @@ -908,6 +921,74 @@ protoGraphs.hugeCompleteGraph = new ProtoGraph("hugeCompleteGraph", ] ); +/* + * Grid Graph Structure (1000x1000) + * Each node connects to its right and bottom neighbors + * + * 1 → 2 → 3 → ... → 1000 + * ↓ ↓ ↓ ↓ + * 1001 → 1002 → 1003 → ... → 2000 + * ↓ ↓ ↓ ↓ + * ... ... ... ... + * ↓ ↓ ↓ ↓ + * 999001 → 999002 → 999003 → ... → 1000000 + */ + +// Generate grid graph with 1000x1000 nodes +const generateGridGraph = (width, height) => { + const nodes = []; + const edges = []; + + // Generate node names as simple numbers + const getNodeName = (row, col) => { + // Calculate node number: row * width + col + 1 + const nodeNum = (row * width + col + 1).toString(); + return nodeNum; + }; + + // Generate nodes and edges + for (let row = 0; row < height; row++) { + for (let col = 0; col < width; col++) { + const nodeName = getNodeName(row, col); + nodes.push(nodeName); + + // Connect to right neighbor + if (col < width - 1) { + edges.push([nodeName, getNodeName(row, col + 1), 1]); + } + + // Connect to bottom neighbor + if (row < height - 1) { + edges.push([nodeName, getNodeName(row + 1, col), 1]); + } + } + } + + return { nodes, edges }; +}; + +const gridSize = 1000; +const gridGraph = generateGridGraph(gridSize, gridSize); + +protoGraphs.hugeGridGraph = new ProtoGraph("hugeGridGraph", + gridGraph.edges, + [1, 2, 5], + [ + { + numberOfShards: 1, + vertexSharding: gridGraph.nodes.map((node, index) => [node, 0]) + }, + { + numberOfShards: 2, + vertexSharding: gridGraph.nodes.map((node, index) => [node, index % 2]) + }, + { + numberOfShards: 5, + vertexSharding: gridGraph.nodes.map((node, index) => [node, index % 5]) + } + ] +); + /* * * diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-tests.js index f9c3720012bd..c8701cb31882 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-tests.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-tests.js @@ -39,6 +39,20 @@ const {getCompactStatsNodes, TraversalBlock} = require("@arangodb/testutils/aql- const {findExecutionNodes} = require("@arangodb/aql-helper"); const isCluster = require("internal").isCluster(); let IM = global.instanceManager; +const bidirectionalMethods = require('@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests'); +const bidirectionalCircle = { + testBidirectionalCircleDfsLongRunning: bidirectionalMethods.testBidirectionalCircleDfsLongRunning, + testBidirectionalCircleBfsLongRunning: bidirectionalMethods.testBidirectionalCircleBfsLongRunning, + testBidirectionalCircleWeightedPathLongRunning: bidirectionalMethods.testBidirectionalCircleWeightedPathLongRunning, +} +const hugeCompleteGraph = { + testHugeCompleteGraphKPathsLongRunning: bidirectionalMethods.testHugeCompleteGraphKPathsLongRunning, +} +const hugeGridGraph = { + testHugeGridGraphShortestPathLongRunning: bidirectionalMethods.testHugeGridGraphShortestPathLongRunning, + testHugeGridGraphAllShortestPathsLongRunning: bidirectionalMethods.testHugeGridGraphAllShortestPathsLongRunning, +} + /* TODO: We need more tests to cover the following things: @@ -6990,6 +7004,18 @@ const testsByGraph = { testEmptyGraphBfsPath, testEmptyGraphDfsPath, testEmptyGraphWeightedPath, + }, + // Used for DFS, BFS, Weighted Path - Query Cancellation + bidirectionalCircle: { + ...bidirectionalCircle + }, + // Used for K Paths - Query Cancellation + hugeCompleteGraph: { + ...hugeCompleteGraph + }, + // Used for Shortest Path, All Shortest Paths - Query Cancellation + hugeGridGraph: { + ...hugeGridGraph } }; diff --git a/tests/Graph/AllShortestPathsFinderTest.cpp b/tests/Graph/AllShortestPathsFinderTest.cpp index 366eec83eb77..ea7f190a9ec5 100644 --- a/tests/Graph/AllShortestPathsFinderTest.cpp +++ b/tests/Graph/AllShortestPathsFinderTest.cpp @@ -149,7 +149,7 @@ class AllShortestPathsFinderTest arangodb::graph::PathType::Type pathType = arangodb::graph::PathType::Type::AllShortestPaths; arangodb::graph::TwoSidedEnumeratorOptions options{minDepth, maxDepth, - pathType}; + pathType, *_query}; PathValidatorOptions validatorOpts{&_tmpVar, _expressionContext}; return AllShortestPathsFinder{ MockGraphProvider( diff --git a/tests/Graph/KPathFinderTest.cpp b/tests/Graph/KPathFinderTest.cpp index 6a2df9f99e96..b71d34744ee3 100644 --- a/tests/Graph/KPathFinderTest.cpp +++ b/tests/Graph/KPathFinderTest.cpp @@ -169,7 +169,7 @@ class KPathFinderTest arangodb::graph::PathType::Type pathType = arangodb::graph::PathType::Type::KPaths; arangodb::graph::TwoSidedEnumeratorOptions options{minDepth, maxDepth, - pathType}; + pathType, *_query}; options.setStopAtFirstDepth(false); PathValidatorOptions validatorOpts{&_tmpVar, _expressionContext}; return KPathFinder{ diff --git a/tests/Graph/KShortestPathsFinderTest.cpp b/tests/Graph/KShortestPathsFinderTest.cpp index 6d46e363aac2..9b1c26c7d8ca 100644 --- a/tests/Graph/KShortestPathsFinderTest.cpp +++ b/tests/Graph/KShortestPathsFinderTest.cpp @@ -198,7 +198,7 @@ class KShortestPathsFinderTest : public ::testing::Test { arangodb::graph::PathType::Type pathType = arangodb::graph::PathType::Type::KShortestPaths; arangodb::graph::TwoSidedEnumeratorOptions options{minDepth, maxDepth, - pathType}; + pathType, *_query}; options.setStopAtFirstDepth(false); PathValidatorOptions validatorOpts{&_tmpVar, _expressionContext}; auto forwardProviderOptions = @@ -450,7 +450,7 @@ class WeightedKShortestPathsFinderTest : public ::testing::Test { arangodb::graph::PathType::Type pathType = arangodb::graph::PathType::Type::KShortestPaths; arangodb::graph::TwoSidedEnumeratorOptions options{minDepth, maxDepth, - pathType}; + pathType, *_query}; options.setStopAtFirstDepth(false); PathValidatorOptions validatorOpts{&_tmpVar, _expressionContext}; auto forwardProviderOptions = diff --git a/tests/Graph/WeightedShortestPathTest.cpp b/tests/Graph/WeightedShortestPathTest.cpp index 15c68dfbe1c2..426609a792c2 100644 --- a/tests/Graph/WeightedShortestPathTest.cpp +++ b/tests/Graph/WeightedShortestPathTest.cpp @@ -150,7 +150,7 @@ class WeightedShortestPathTest arangodb::graph::PathType::Type pathType = arangodb::graph::PathType::Type::ShortestPath; arangodb::graph::TwoSidedEnumeratorOptions options{minDepth, maxDepth, - pathType}; + pathType, *_query}; options.setStopAtFirstDepth(false); PathValidatorOptions validatorOpts{&_tmpVar, _expressionContext}; auto forwardProviderOptions = From e3f7ab8b01452e0db90c3fdb011d9c8402f900b7 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Wed, 14 May 2025 13:42:33 +0200 Subject: [PATCH 05/28] only print debug creation info if set manually in constructor --- .../aql-graph-traversal-generic-graphs.js | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index 0cf18d74f9e6..3e7375185385 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -148,6 +148,7 @@ class TestGraph { isEnterprise: false, isSatellite: false }; + this.debug = false; } hasProjectionPayload() { @@ -240,18 +241,19 @@ class TestGraph { this.verticesByName = TestGraph._fillGraph(this.graphName, this.edges, db[this.vn], db[this.en], this.unconnectedVertices, vertexSharding, this.addProjectionPayload); db[this.en].ensureIndex({type: "persistent", fields: ["_from", graphIndexedAttribute]}); - // Print first and last 10 vertices for debugging - print("First 10 vertices in graph " + this.graphName + ":"); - const first10Vertices = Object.entries(this.verticesByName).slice(0, 10); - first10Vertices.forEach(([key, id]) => { - print(` ${key}: ${id} (node name: ${key})`); - }); - - print("\nLast 10 vertices in graph " + this.graphName + ":"); - const last10Vertices = Object.entries(this.verticesByName).slice(-10); - last10Vertices.forEach(([key, id]) => { - print(` ${key}: ${id} (node name: ${key})`); - }); + if (this.debug) { + print("First 10 vertices in graph " + this.graphName + ":"); + const first10Vertices = Object.entries(this.verticesByName).slice(0, 10); + first10Vertices.forEach(([key, id]) => { + print(` ${key}: ${id} (node name: ${key})`); + }); + + print("\nLast 10 vertices in graph " + this.graphName + ":"); + const last10Vertices = Object.entries(this.verticesByName).slice(-10); + last10Vertices.forEach(([key, id]) => { + print(` ${key}: ${id} (node name: ${key})`); + }); + } } name() { From 6fec0fcfb420778ae64c61b304f678c09032cd01 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Wed, 14 May 2025 13:54:17 +0200 Subject: [PATCH 06/28] added additional check for cancelation of query in the twosidedenumerator --- .../Graph/Enumerators/TwoSidedEnumerator.cpp | 42 ++++++++++++++----- .../Graph/Enumerators/TwoSidedEnumerator.h | 8 +++- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp b/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp index e0f37bb1530e..668cebe7c31d 100644 --- a/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp @@ -56,7 +56,8 @@ TwoSidedEnumerator:: Ball::Ball(Direction dir, ProviderType&& provider, GraphOptions const& options, PathValidatorOptions validatorOptions, - arangodb::ResourceMonitor& resourceMonitor) + arangodb::ResourceMonitor& resourceMonitor, + TwoSidedEnumerator& parent) : _resourceMonitor(resourceMonitor), _interior(resourceMonitor), _queue(resourceMonitor), @@ -64,7 +65,8 @@ TwoSidedEnumerator:: _validator(_provider, _interior, std::move(validatorOptions)), _direction(dir), _minDepth(options.getMinDepth()), - _graphOptions(options) {} + _graphOptions(options), + _parent(parent) {} template @@ -198,7 +200,12 @@ auto TwoSidedEnumerator:: Ball::computeNeighbourhoodOfNextVertex(Ball& other, ResultList& results) -> void { if (_graphOptions.isKilled()) { + // First clear our own instance (Ball) clear(); + // Then clear the other instance (Ball) + other.clear(); + // Then clear the parent (TwoSidedEnumerator) + _parent.clear(); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } @@ -253,8 +260,8 @@ auto TwoSidedEnumerator:: template -void TwoSidedEnumerator::Ball::testDepthZero(Ball& other, ResultList& results) { +void TwoSidedEnumerator:: + Ball::testDepthZero(Ball& other, ResultList& results) { for (auto const& step : _shell) { other.matchResultsInShell(step, results, _validator); } @@ -291,9 +298,9 @@ auto TwoSidedEnumerator:: template -auto TwoSidedEnumerator::Ball::buildPath(Step const& vertexInShell, - PathResult& path) -> void { +auto TwoSidedEnumerator:: + Ball::buildPath(Step const& vertexInShell, + PathResult& path) -> void { if (_direction == FORWARD) { _interior.buildPath(vertexInShell, path); } else { @@ -317,10 +324,15 @@ TwoSidedEnumerator:: PathValidatorOptions validatorOptions, arangodb::ResourceMonitor& resourceMonitor) : _options(std::move(options)), - _left{Direction::FORWARD, std::move(forwardProvider), _options, - validatorOptions, resourceMonitor}, - _right{Direction::BACKWARD, std::move(backwardProvider), _options, - std::move(validatorOptions), resourceMonitor}, + _left{Direction::FORWARD, std::move(forwardProvider), + _options, validatorOptions, + resourceMonitor, *this}, + _right{Direction::BACKWARD, + std::move(backwardProvider), + _options, + std::move(validatorOptions), + resourceMonitor, + *this}, _baselineDepth(_options.getMaxDepth()), _resultPath{_left.provider(), _right.provider()} {} @@ -456,6 +468,14 @@ void TwoSidedEnumerator::searchMoreResults() { while (_results.empty() && !searchDone()) { _resultsFetched = false; + + // Check for kill signal before proceeding + // We will also do additional checks in computeNeighbourhoodOfNextVertex + if (_options.isKilled()) { + clear(); + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } + if (_searchLeft) { if (ADB_UNLIKELY(_left.doneWithDepth())) { startNextDepth(); diff --git a/arangod/Graph/Enumerators/TwoSidedEnumerator.h b/arangod/Graph/Enumerators/TwoSidedEnumerator.h index 6dd5d1a53182..a13e0bd18eb1 100644 --- a/arangod/Graph/Enumerators/TwoSidedEnumerator.h +++ b/arangod/Graph/Enumerators/TwoSidedEnumerator.h @@ -126,7 +126,8 @@ class TwoSidedEnumerator { public: Ball(Direction dir, ProviderType&& provider, GraphOptions const& options, PathValidatorOptions validatorOptions, - arangodb::ResourceMonitor& resourceMonitor); + arangodb::ResourceMonitor& resourceMonitor, + TwoSidedEnumerator& parent); ~Ball(); auto clear() -> void; auto reset(VertexRef center, size_t depth = 0) -> void; @@ -186,6 +187,11 @@ class TwoSidedEnumerator { Direction _direction; size_t _minDepth{0}; GraphOptions _graphOptions; + + // Reference to the parent TwoSidedEnumerator + // Intention: To be able to call clear() on the parent + // Case: When a kill signal is received + TwoSidedEnumerator& _parent; }; public: From 0268917cce235ba3482bb7253ce351c46aabba1f Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Wed, 14 May 2025 14:37:04 +0200 Subject: [PATCH 07/28] eslint --- .../testutils/aql-graph-traversal-generic-tests.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-tests.js index c8701cb31882..07f7d0db2d3b 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-tests.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-tests.js @@ -44,14 +44,14 @@ const bidirectionalCircle = { testBidirectionalCircleDfsLongRunning: bidirectionalMethods.testBidirectionalCircleDfsLongRunning, testBidirectionalCircleBfsLongRunning: bidirectionalMethods.testBidirectionalCircleBfsLongRunning, testBidirectionalCircleWeightedPathLongRunning: bidirectionalMethods.testBidirectionalCircleWeightedPathLongRunning, -} +}; const hugeCompleteGraph = { testHugeCompleteGraphKPathsLongRunning: bidirectionalMethods.testHugeCompleteGraphKPathsLongRunning, -} +}; const hugeGridGraph = { testHugeGridGraphShortestPathLongRunning: bidirectionalMethods.testHugeGridGraphShortestPathLongRunning, testHugeGridGraphAllShortestPathsLongRunning: bidirectionalMethods.testHugeGridGraphAllShortestPathsLongRunning, -} +}; /* TODO: From 52420855fbc4b2130419a78a372a55d684a04c49 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Fri, 16 May 2025 11:06:57 +0200 Subject: [PATCH 08/28] eslint --- ...l-graph-traversal-generic-bidirectional-tests.js | 13 ++++++++++--- .../testutils/aql-graph-traversal-generic-graphs.js | 1 + 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js index c6648911e527..971705c268e8 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js @@ -1,6 +1,13 @@ +/*jshint globalstrict:true, strict:true, esnext: true */ +/*global print */ + +"use strict"; + const jsunity = require("jsunity"); -const {assertTrue} = jsunity.jsUnity.assertions; +const {assertTrue, assertEqual} = jsunity.jsUnity.assertions; const protoGraphs = require('@arangodb/testutils/aql-graph-traversal-generic-graphs').protoGraphs; +const internal = require("internal"); +const db = internal.db; const arango = internal.arango; // seconds to add to execution time for verification @@ -78,7 +85,7 @@ const localHelper = { ); if (debug) { if (disableAsyncHeader) { - print(response) + print(response); } print("Async query response code:", response.code); print("Async query ID:", response.headers['x-arango-async-id']); @@ -172,7 +179,7 @@ const localHelper = { print("Test completed successfully"); } } -} +}; /* Bidirectional Circle diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index 3e7375185385..fab55cbb6b8d 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -1,4 +1,5 @@ /*jshint globalstrict:true, strict:true, esnext: true */ +/* global print */ "use strict"; From 89f440ee33a13a1cc93bf4f143038ce088aa8120 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Fri, 16 May 2025 11:19:14 +0200 Subject: [PATCH 09/28] clang format --- .../Aql/ExecutionNode/EnumeratePathsNode.cpp | 13 +++++-------- arangod/Aql/ExecutionNode/ShortestPathNode.cpp | 10 ++++------ arangod/Aql/ExecutionNode/TraversalNode.cpp | 5 ++--- .../Graph/Enumerators/OneSidedEnumerator.cpp | 3 ++- .../Options/OneSidedEnumeratorOptions.cpp | 4 +++- .../Graph/Options/OneSidedEnumeratorOptions.h | 3 ++- arangod/Graph/Options/QueryContextObserver.h | 18 +++++++++--------- .../Options/TwoSidedEnumeratorOptions.cpp | 5 ++++- .../Graph/Providers/BaseProviderOptions.cpp | 6 ++---- arangod/Graph/Providers/BaseProviderOptions.h | 17 ++++++----------- 10 files changed, 39 insertions(+), 45 deletions(-) diff --git a/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp b/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp index e1951841c036..3d79e698f8c5 100644 --- a/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp +++ b/arangod/Aql/ExecutionNode/EnumeratePathsNode.cpp @@ -483,8 +483,7 @@ std::unique_ptr EnumeratePathsNode::createBlock( opts->tmpVar(), std::move(reversedUsedIndexes), opts->getExpressionCtx(), {}, opts->collectionToShard(), opts->getVertexProjections(), opts->getEdgeProjections(), - opts->produceVertices(), opts->useCache(), - opts->query()); + opts->produceVertices(), opts->useCache(), opts->query()); using Provider = SingleServerProvider; if (opts->query().queryOptions().getTraversalProfileLevel() == @@ -680,13 +679,11 @@ std::unique_ptr EnumeratePathsNode::createBlock( } else { // Cluster case (on coordinator) auto cache = std::make_shared( opts->query().resourceMonitor()); - ClusterBaseProviderOptions forwardProviderOptions(cache, engines(), false, - opts->produceVertices(), - opts->query()); + ClusterBaseProviderOptions forwardProviderOptions( + cache, engines(), false, opts->produceVertices(), opts->query()); forwardProviderOptions.setClearEdgeCacheOnClear(false); - ClusterBaseProviderOptions backwardProviderOptions(cache, engines(), true, - opts->produceVertices(), - opts->query()); + ClusterBaseProviderOptions backwardProviderOptions( + cache, engines(), true, opts->produceVertices(), opts->query()); backwardProviderOptions.setClearEdgeCacheOnClear(false); // A comment is in order here: For all cases covered here // (k-shortest-paths, all shortest paths, k-paths) we do not need to diff --git a/arangod/Aql/ExecutionNode/ShortestPathNode.cpp b/arangod/Aql/ExecutionNode/ShortestPathNode.cpp index 2e7f89328768..d31d72eb27d6 100644 --- a/arangod/Aql/ExecutionNode/ShortestPathNode.cpp +++ b/arangod/Aql/ExecutionNode/ShortestPathNode.cpp @@ -511,12 +511,10 @@ std::unique_ptr ShortestPathNode::createBlock( using ClusterProvider = ClusterProvider; auto cache = std::make_shared( opts->query().resourceMonitor()); - ClusterBaseProviderOptions forwardProviderOptions(cache, engines(), false, - opts->produceVertices(), - opts->query()); - ClusterBaseProviderOptions backwardProviderOptions(cache, engines(), true, - opts->produceVertices(), - opts->query()); + ClusterBaseProviderOptions forwardProviderOptions( + cache, engines(), false, opts->produceVertices(), opts->query()); + ClusterBaseProviderOptions backwardProviderOptions( + cache, engines(), true, opts->produceVertices(), opts->query()); auto usesWeight = checkWeight(forwardProviderOptions, backwardProviderOptions); diff --git a/arangod/Aql/ExecutionNode/TraversalNode.cpp b/arangod/Aql/ExecutionNode/TraversalNode.cpp index 3fdf08ac3903..1b0b634a9d0a 100644 --- a/arangod/Aql/ExecutionNode/TraversalNode.cpp +++ b/arangod/Aql/ExecutionNode/TraversalNode.cpp @@ -817,9 +817,8 @@ std::unique_ptr TraversalNode::createBlock( bool isSmart) const { TraverserOptions* opts = this->options(); - arangodb::graph::OneSidedEnumeratorOptions options{opts->minDepth, - opts->maxDepth, - opts->query()}; + arangodb::graph::OneSidedEnumeratorOptions options{ + opts->minDepth, opts->maxDepth, opts->query()}; /* * PathValidator Disjoint Helper (TODO [GraphRefactor]: Copy from createBlock) * Clean this up as soon we clean up the whole TraversalNode as well. diff --git a/arangod/Graph/Enumerators/OneSidedEnumerator.cpp b/arangod/Graph/Enumerators/OneSidedEnumerator.cpp index c55db9aa159b..75da2731558f 100644 --- a/arangod/Graph/Enumerators/OneSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/OneSidedEnumerator.cpp @@ -109,7 +109,8 @@ void OneSidedEnumerator::clearProvider() { template void OneSidedEnumerator::computeNeighbourhoodOfNextVertex() { if (_options.isKilled()) { - // Clear false may sounds misleading, but this means we do not want to keep the path store + // Clear false may sounds misleading, but this means we do not want to keep + // the path store clear(false); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } diff --git a/arangod/Graph/Options/OneSidedEnumeratorOptions.cpp b/arangod/Graph/Options/OneSidedEnumeratorOptions.cpp index 2bf295218e5d..446e65d25566 100644 --- a/arangod/Graph/Options/OneSidedEnumeratorOptions.cpp +++ b/arangod/Graph/Options/OneSidedEnumeratorOptions.cpp @@ -27,7 +27,9 @@ using namespace arangodb; using namespace arangodb::graph; -OneSidedEnumeratorOptions::OneSidedEnumeratorOptions(size_t minDepth, size_t maxDepth, aql::QueryContext& query) +OneSidedEnumeratorOptions::OneSidedEnumeratorOptions(size_t minDepth, + size_t maxDepth, + aql::QueryContext& query) : _minDepth(minDepth), _maxDepth(maxDepth), _observer(query) {} OneSidedEnumeratorOptions::~OneSidedEnumeratorOptions() = default; diff --git a/arangod/Graph/Options/OneSidedEnumeratorOptions.h b/arangod/Graph/Options/OneSidedEnumeratorOptions.h index c1519906e55d..4c5f7a192735 100644 --- a/arangod/Graph/Options/OneSidedEnumeratorOptions.h +++ b/arangod/Graph/Options/OneSidedEnumeratorOptions.h @@ -33,7 +33,8 @@ namespace arangodb::graph { struct OneSidedEnumeratorOptions { public: - OneSidedEnumeratorOptions(size_t minDepth, size_t maxDepth, aql::QueryContext& query); + OneSidedEnumeratorOptions(size_t minDepth, size_t maxDepth, + aql::QueryContext& query); ~OneSidedEnumeratorOptions(); [[nodiscard]] size_t getMinDepth() const noexcept; diff --git a/arangod/Graph/Options/QueryContextObserver.h b/arangod/Graph/Options/QueryContextObserver.h index d7afffc0cab9..c4f131b5d7ec 100644 --- a/arangod/Graph/Options/QueryContextObserver.h +++ b/arangod/Graph/Options/QueryContextObserver.h @@ -26,14 +26,14 @@ #include "Aql/QueryContext.h" -// This class serves as a wrapper around QueryContext to explicitly track where query killing -// is being used in the graph traversal code. It provides a single point of access to check -// if a query has been killed, making it easier to maintain and modify the query killing -// behavior if needed. +// This class serves as a wrapper around QueryContext to explicitly track where +// query killing is being used in the graph traversal code. It provides a single +// point of access to check if a query has been killed, making it easier to +// maintain and modify the query killing behavior if needed. // -// While this adds a small layer of indirection, it helps with code clarity and maintainability. -// If profiling shows this wrapper causes significant overhead, we can remove it and use -// QueryContext directly. +// While this adds a small layer of indirection, it helps with code clarity and +// maintainability. If profiling shows this wrapper causes significant overhead, +// we can remove it and use QueryContext directly. // // We can change this or discuss if this approach is not liked. @@ -42,11 +42,11 @@ namespace arangodb::graph { class QueryContextObserver { public: explicit QueryContextObserver(aql::QueryContext& query) : _query(query) {} - + [[nodiscard]] bool isKilled() const { return _query.killed(); } private: aql::QueryContext& _query; }; -} // namespace arangodb::graph \ No newline at end of file +} // namespace arangodb::graph \ No newline at end of file diff --git a/arangod/Graph/Options/TwoSidedEnumeratorOptions.cpp b/arangod/Graph/Options/TwoSidedEnumeratorOptions.cpp index 7c1e09416adb..6a7669f5e771 100644 --- a/arangod/Graph/Options/TwoSidedEnumeratorOptions.cpp +++ b/arangod/Graph/Options/TwoSidedEnumeratorOptions.cpp @@ -31,7 +31,10 @@ TwoSidedEnumeratorOptions::TwoSidedEnumeratorOptions(size_t minDepth, size_t maxDepth, PathType::Type pathType, aql::QueryContext& query) - : _minDepth(minDepth), _maxDepth(maxDepth), _pathType(pathType), _observer(query) { + : _minDepth(minDepth), + _maxDepth(maxDepth), + _pathType(pathType), + _observer(query) { if (getPathType() == PathType::Type::AllShortestPaths) { setStopAtFirstDepth(true); } else if (getPathType() == PathType::Type::ShortestPath) { diff --git a/arangod/Graph/Providers/BaseProviderOptions.cpp b/arangod/Graph/Providers/BaseProviderOptions.cpp index a0cc669dcd4e..4e7cef901842 100644 --- a/arangod/Graph/Providers/BaseProviderOptions.cpp +++ b/arangod/Graph/Providers/BaseProviderOptions.cpp @@ -86,8 +86,7 @@ SingleServerBaseProviderOptions::SingleServerBaseProviderOptions( MonitoredCollectionToShardMap const& collectionToShardMap, aql::Projections const& vertexProjections, aql::Projections const& edgeProjections, bool produceVertices, - bool useCache, - aql::QueryContext& query) + bool useCache, aql::QueryContext& query) : _temporaryVariable(tmpVar), _indexInformation(std::move(indexInfo)), _expressionContext(expressionContext), @@ -171,8 +170,7 @@ void SingleServerBaseProviderOptions::unPrepareContext() { ClusterBaseProviderOptions::ClusterBaseProviderOptions( std::shared_ptr cache, std::unordered_map const* engines, bool backward, - bool produceVertices, - aql::QueryContext& query) + bool produceVertices, aql::QueryContext& query) : _cache(std::move(cache)), _engines(engines), _backward(backward), diff --git a/arangod/Graph/Providers/BaseProviderOptions.h b/arangod/Graph/Providers/BaseProviderOptions.h index 2d0f79fecc89..3718e0edf431 100644 --- a/arangod/Graph/Providers/BaseProviderOptions.h +++ b/arangod/Graph/Providers/BaseProviderOptions.h @@ -100,10 +100,10 @@ struct SingleServerBaseProviderOptions { MonitoredCollectionToShardMap const& collectionToShardMap, aql::Projections const& vertexProjections, aql::Projections const& edgeProjections, bool produceVertices, - bool useCache, - aql::QueryContext& query); + bool useCache, aql::QueryContext& query); - SingleServerBaseProviderOptions(SingleServerBaseProviderOptions const&) = delete; + SingleServerBaseProviderOptions(SingleServerBaseProviderOptions const&) = + delete; SingleServerBaseProviderOptions(SingleServerBaseProviderOptions&&) = default; aql::Variable const* tmpVar() const; @@ -133,9 +133,7 @@ struct SingleServerBaseProviderOptions { aql::Projections const& getEdgeProjections() const; - bool isKilled() const noexcept { - return _queryObserver.isKilled(); - } + bool isKilled() const noexcept { return _queryObserver.isKilled(); } private: // The temporary Variable used in the Indexes @@ -188,8 +186,7 @@ struct ClusterBaseProviderOptions { ClusterBaseProviderOptions( std::shared_ptr cache, std::unordered_map const* engines, bool backward, - bool produceVertices, - aql::QueryContext& query); + bool produceVertices, aql::QueryContext& query); ClusterBaseProviderOptions( std::shared_ptr cache, @@ -236,9 +233,7 @@ struct ClusterBaseProviderOptions { _clearEdgeCacheOnClear = flag; } - bool isKilled() const noexcept { - return _queryObserver.isKilled(); - } + bool isKilled() const noexcept { return _queryObserver.isKilled(); } private: std::shared_ptr _cache; From 298877b21960d1a4234cb163d1a5f877ce8a946d Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Fri, 16 May 2025 11:27:54 +0200 Subject: [PATCH 10/28] clang format --- tests/Graph/DFSFinderTest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Graph/DFSFinderTest.cpp b/tests/Graph/DFSFinderTest.cpp index a6559c6b80bd..571853cc4c32 100644 --- a/tests/Graph/DFSFinderTest.cpp +++ b/tests/Graph/DFSFinderTest.cpp @@ -174,7 +174,8 @@ class DFSFinderTest } auto pathFinder(size_t minDepth, size_t maxDepth) -> DFSFinder { - arangodb::graph::OneSidedEnumeratorOptions options{minDepth, maxDepth, *_query.get()}; + arangodb::graph::OneSidedEnumeratorOptions options{minDepth, maxDepth, + *_query.get()}; PathValidatorOptions validatorOpts{&_tmpVar, _expressionContext}; return DFSFinder( {*_query.get(), From b6c8690e5605e3acbf41c75bcc7951f176d9dc34 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 19 May 2025 10:49:31 +0200 Subject: [PATCH 11/28] batchwise verted and edge insertion during fillGraph (integration tests) --- .../aql-graph-traversal-generic-graphs.js | 36 ++++++++++++++----- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index fab55cbb6b8d..ed5698927f4b 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -401,16 +401,25 @@ class TestGraph { toSave.push(doc); keys.push(vertexKey); } - // Save all vertices in one request, and map the result back to the input. - // This should speed up the tests. - vc.save(toSave).forEach((d, i) => { - verticesByName[keys[i]] = d._id; - return null; - }); + + // Process vertices in batches of 100k + const VERTEX_BATCH_SIZE = 100000; + for (let i = 0; i < toSave.length; i += VERTEX_BATCH_SIZE) { + const batch = toSave.slice(i, i + VERTEX_BATCH_SIZE); + const batchKeys = keys.slice(i, i + VERTEX_BATCH_SIZE); + if (this.debug) { + print(`Saving vertex batch ${i} of ${toSave.length}`); + } + vc.save(batch).forEach((d, idx) => { + verticesByName[batchKeys[idx]] = d._id; + return null; + }); + } } - // Save all edges in one request - ec.save(edges.map(([v, w, weight]) => { + // Process edges in batches of 100k + const EDGE_BATCH_SIZE = 100000; + const edgeDocs = edges.map(([v, w, weight]) => { const edge = { _from: verticesByName[v], _to: verticesByName[w], @@ -431,7 +440,16 @@ class TestGraph { } return edge; - })); + }); + + // Save edges in batches + for (let i = 0; i < edgeDocs.length; i += EDGE_BATCH_SIZE) { + const batch = edgeDocs.slice(i, i + EDGE_BATCH_SIZE); + if (this.debug) { + print(`Saving edge batch ${i} of ${edgeDocs.length}`); + } + ec.save(batch); + } return verticesByName; } From 105dbaeacf7d5c27cdaa4e04359d0cd85600c681 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 19 May 2025 11:40:49 +0200 Subject: [PATCH 12/28] try to help garbage collection v8 --- .../aql-graph-traversal-generic-graphs.js | 50 +++++++++++++------ 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index ed5698927f4b..98263a4fbedd 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -402,8 +402,8 @@ class TestGraph { keys.push(vertexKey); } - // Process vertices in batches of 100k - const VERTEX_BATCH_SIZE = 100000; + // Process vertices in smaller batches to reduce memory pressure + const VERTEX_BATCH_SIZE = 10000; // Reduced from 100k to 10k for (let i = 0; i < toSave.length; i += VERTEX_BATCH_SIZE) { const batch = toSave.slice(i, i + VERTEX_BATCH_SIZE); const batchKeys = keys.slice(i, i + VERTEX_BATCH_SIZE); @@ -414,21 +414,30 @@ class TestGraph { verticesByName[batchKeys[idx]] = d._id; return null; }); + // Clear references to help garbage collection + batch.length = 0; + batchKeys.length = 0; } + // Clear the full arrays after processing + toSave.length = 0; + keys.length = 0; } - // Process edges in batches of 100k - const EDGE_BATCH_SIZE = 100000; - const edgeDocs = edges.map(([v, w, weight]) => { + // Process edges in smaller batches to reduce memory pressure + const EDGE_BATCH_SIZE = 10000; // Reduced from 100k to 10k + let edgeDocs = []; + let currentBatch = []; + let currentBatchSize = 0; + + // Process edges in a streaming fashion + for (const [v, w, weight] of edges) { const edge = { _from: verticesByName[v], _to: verticesByName[w], - // Will be used in filters of tests. secondFrom: verticesByName[v] }; - // check if our edge also has a weight defined and is a number + if (weight && typeof weight === 'number') { - // if found, add attribute "distance" as weightAttribute to the edge document edge[graphWeightAttribute] = weight; edge[graphIndexedAttribute] = weight; } @@ -439,17 +448,28 @@ class TestGraph { edge.payload3 = payloadGen.next().value; } - return edge; - }); + currentBatch.push(edge); + currentBatchSize++; - // Save edges in batches - for (let i = 0; i < edgeDocs.length; i += EDGE_BATCH_SIZE) { - const batch = edgeDocs.slice(i, i + EDGE_BATCH_SIZE); + // When batch is full, save it and clear memory + if (currentBatchSize >= EDGE_BATCH_SIZE) { + if (this.debug) { + print(`Saving edge batch of size ${currentBatchSize}`); + } + ec.save(currentBatch); + currentBatch = []; + currentBatchSize = 0; + } + } + + // Save any remaining edges + if (currentBatch.length > 0) { if (this.debug) { - print(`Saving edge batch ${i} of ${edgeDocs.length}`); + print(`Saving final edge batch of size ${currentBatch.length}`); } - ec.save(batch); + ec.save(currentBatch); } + return verticesByName; } From 9ebfb803a4436ebf32b85b7e0f8308022b799799 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 19 May 2025 12:55:56 +0200 Subject: [PATCH 13/28] slightly increased timeout to 10s in clsuter, 5s in single server --- .../aql-graph-traversal-generic-bidirectional-tests.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js index 971705c268e8..49c61dcbcadd 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js @@ -10,10 +10,10 @@ const internal = require("internal"); const db = internal.db; const arango = internal.arango; -// seconds to add to execution time for verification +// Seconds to add to execution time for verification. // This is to account for the time it takes for the query to be scheduled and executed -// and for the query to be killed -const VERIFICATION_TIME_BUFFER = 3; +// and killed. +const VERIFICATION_TIME_BUFFER = internal.isCluster() ? 10 : 5; const localHelper = { getRunningQueries: function() { From 324aa214dbe85e1db85c0729408407752662ae63 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 19 May 2025 22:41:00 +0200 Subject: [PATCH 14/28] added early exit to weighted two sided enumerator, yens still missing --- .../WeightedTwoSidedEnumerator.cpp | 29 +++++++++++++++---- .../Enumerators/WeightedTwoSidedEnumerator.h | 12 ++++++-- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp index 11be5302f22d..427afa754e53 100644 --- a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp @@ -58,7 +58,8 @@ WeightedTwoSidedEnumerator< PathValidator>::Ball::Ball(Direction dir, ProviderType&& provider, GraphOptions const& options, PathValidatorOptions validatorOptions, - arangodb::ResourceMonitor& resourceMonitor) + arangodb::ResourceMonitor& resourceMonitor, + WeightedTwoSidedEnumerator& parent) : _resourceMonitor(resourceMonitor), _interior(resourceMonitor), _queue(resourceMonitor), @@ -67,7 +68,8 @@ WeightedTwoSidedEnumerator< _direction(dir), _graphOptions(options), _diameter(-std::numeric_limits::infinity()), - _haveSeenOtherSide(false) {} + _haveSeenOtherSide(false), + _parent(parent) {} template @@ -254,6 +256,16 @@ auto WeightedTwoSidedEnumerator::Ball:: computeNeighbourhoodOfNextVertex(Ball& other, CandidatesStore& candidates) -> void { + if (_graphOptions.isKilled()) { + // First clear our own instance (Ball) + clear(); + // Then clear the other instance (Ball) + other.clear(); + // Then clear the parent (WeightedTwoSidedEnumerator) + _parent.clear(); + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } + ensureQueueHasProcessableElement(); auto tmp = _queue.pop(); @@ -414,9 +426,9 @@ WeightedTwoSidedEnumerator bool WeightedTwoSidedEnumerator::isDone() const { + PathValidator>::isDone() { if (!_candidatesStore.isEmpty()) { return false; } @@ -861,7 +873,12 @@ WeightedTwoSidedEnumerator auto WeightedTwoSidedEnumerator::searchDone() const -> bool { + PathValidator>::searchDone() -> bool { + if (_options.isKilled()) { + // Here we're not inside a Ball, so we can clear via main clear method + clear(); + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } if ((_left.noPathLeft() && _right.noPathLeft()) || isAlgorithmFinished()) { return true; } diff --git a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h index 1d69bd73784e..57fc0dae0f37 100644 --- a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h +++ b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h @@ -221,7 +221,8 @@ class WeightedTwoSidedEnumerator { public: Ball(Direction dir, ProviderType&& provider, GraphOptions const& options, PathValidatorOptions validatorOptions, - arangodb::ResourceMonitor& resourceMonitor); + arangodb::ResourceMonitor& resourceMonitor, + WeightedTwoSidedEnumerator& parent); ~Ball(); auto clear() -> void; auto reset(VertexRef center, size_t depth = 0) -> void; @@ -296,6 +297,11 @@ class WeightedTwoSidedEnumerator { GraphOptions _graphOptions; double _diameter = -std::numeric_limits::infinity(); bool _haveSeenOtherSide; + + // Reference to the parent WeightedTwoSidedEnumerator + // Intention: To be able to call clear() on the parent + // Case: When a kill signal is received + WeightedTwoSidedEnumerator& _parent; }; enum BallSearchLocation { LEFT, RIGHT, FINISH }; @@ -345,7 +351,7 @@ class WeightedTwoSidedEnumerator { * @return true There will be no further path. * @return false There is a chance that there is more data available. */ - [[nodiscard]] bool isDone() const; + [[nodiscard]] bool isDone(); /** * @brief Reset to new source and target vertices. @@ -410,7 +416,7 @@ class WeightedTwoSidedEnumerator { _right.setForbiddenEdges(std::move(forbidden)); }; - private : [[nodiscard]] auto searchDone() const -> bool; + private : [[nodiscard]] auto searchDone() -> bool; // Ensure that we have fetched all vertices in the _results list. Otherwise, // we will not be able to generate the resulting path auto fetchResults() -> void; From dad761af61ff67943b207dbea370b661bdd1c366 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 19 May 2025 22:46:10 +0200 Subject: [PATCH 15/28] debug flag on --- ...l-graph-traversal-generic-bidirectional-tests.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js index 49c61dcbcadd..217355fb3dc9 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js @@ -198,7 +198,7 @@ function testBidirectionalCircleDfsLongRunning(testGraph) { RETURN v.key `; - localHelper.testKillLongRunningQuery(queryString); + localHelper.testKillLongRunningQuery(queryString, true); } function testBidirectionalCircleBfsLongRunning(testGraph) { @@ -211,7 +211,7 @@ function testBidirectionalCircleBfsLongRunning(testGraph) { RETURN v.key `; - localHelper.testKillLongRunningQuery(queryString); + localHelper.testKillLongRunningQuery(queryString, true); } function testBidirectionalCircleWeightedPathLongRunning(testGraph) { @@ -224,7 +224,7 @@ function testBidirectionalCircleWeightedPathLongRunning(testGraph) { RETURN v.key `; - localHelper.testKillLongRunningQuery(queryString); + localHelper.testKillLongRunningQuery(queryString, true); } /* @@ -242,7 +242,7 @@ function testHugeCompleteGraphKPathsLongRunning(testGraph) { RETURN path `; - localHelper.testKillLongRunningQuery(queryString); + localHelper.testKillLongRunningQuery(queryString, true); } /* @@ -261,8 +261,7 @@ function testHugeGridGraphShortestPathLongRunning(testGraph) { RETURN v.key `; - const debug = false; - localHelper.testKillLongRunningQuery(queryString, debug, debug); + localHelper.testKillLongRunningQuery(queryString, true); } function testHugeGridGraphAllShortestPathsLongRunning(testGraph) { @@ -275,7 +274,7 @@ function testHugeGridGraphAllShortestPathsLongRunning(testGraph) { RETURN path `; - localHelper.testKillLongRunningQuery(queryString); + localHelper.testKillLongRunningQuery(queryString, true); } // DFS, BFS, Weighted Path From b158f12b1b7c1b13754e960a7316ed9421d9f5fb Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Wed, 21 May 2025 00:13:24 +0200 Subject: [PATCH 16/28] no need to call clear manually. will be called in destructor automatically --- .../Graph/Enumerators/OneSidedEnumerator.cpp | 3 --- .../Graph/Enumerators/TwoSidedEnumerator.cpp | 18 ++++-------------- arangod/Graph/Enumerators/TwoSidedEnumerator.h | 8 +------- .../Enumerators/WeightedTwoSidedEnumerator.cpp | 18 ++++-------------- .../Enumerators/WeightedTwoSidedEnumerator.h | 8 +------- arangod/Graph/Providers/ClusterProvider.cpp | 3 --- .../Graph/Providers/SingleServerProvider.cpp | 1 - 7 files changed, 10 insertions(+), 49 deletions(-) diff --git a/arangod/Graph/Enumerators/OneSidedEnumerator.cpp b/arangod/Graph/Enumerators/OneSidedEnumerator.cpp index 75da2731558f..0a846670a2e4 100644 --- a/arangod/Graph/Enumerators/OneSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/OneSidedEnumerator.cpp @@ -109,9 +109,6 @@ void OneSidedEnumerator::clearProvider() { template void OneSidedEnumerator::computeNeighbourhoodOfNextVertex() { if (_options.isKilled()) { - // Clear false may sounds misleading, but this means we do not want to keep - // the path store - clear(false); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } diff --git a/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp b/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp index 668cebe7c31d..e5e7593a5c66 100644 --- a/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp @@ -56,8 +56,7 @@ TwoSidedEnumerator:: Ball::Ball(Direction dir, ProviderType&& provider, GraphOptions const& options, PathValidatorOptions validatorOptions, - arangodb::ResourceMonitor& resourceMonitor, - TwoSidedEnumerator& parent) + arangodb::ResourceMonitor& resourceMonitor) : _resourceMonitor(resourceMonitor), _interior(resourceMonitor), _queue(resourceMonitor), @@ -65,8 +64,7 @@ TwoSidedEnumerator:: _validator(_provider, _interior, std::move(validatorOptions)), _direction(dir), _minDepth(options.getMinDepth()), - _graphOptions(options), - _parent(parent) {} + _graphOptions(options) {} template @@ -200,12 +198,6 @@ auto TwoSidedEnumerator:: Ball::computeNeighbourhoodOfNextVertex(Ball& other, ResultList& results) -> void { if (_graphOptions.isKilled()) { - // First clear our own instance (Ball) - clear(); - // Then clear the other instance (Ball) - other.clear(); - // Then clear the parent (TwoSidedEnumerator) - _parent.clear(); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } @@ -326,13 +318,12 @@ TwoSidedEnumerator:: : _options(std::move(options)), _left{Direction::FORWARD, std::move(forwardProvider), _options, validatorOptions, - resourceMonitor, *this}, + resourceMonitor}, _right{Direction::BACKWARD, std::move(backwardProvider), _options, std::move(validatorOptions), - resourceMonitor, - *this}, + resourceMonitor}, _baselineDepth(_options.getMaxDepth()), _resultPath{_left.provider(), _right.provider()} {} @@ -472,7 +463,6 @@ void TwoSidedEnumerator void; auto reset(VertexRef center, size_t depth = 0) -> void; @@ -187,11 +186,6 @@ class TwoSidedEnumerator { Direction _direction; size_t _minDepth{0}; GraphOptions _graphOptions; - - // Reference to the parent TwoSidedEnumerator - // Intention: To be able to call clear() on the parent - // Case: When a kill signal is received - TwoSidedEnumerator& _parent; }; public: diff --git a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp index 427afa754e53..5ca32ce2479c 100644 --- a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp @@ -58,8 +58,7 @@ WeightedTwoSidedEnumerator< PathValidator>::Ball::Ball(Direction dir, ProviderType&& provider, GraphOptions const& options, PathValidatorOptions validatorOptions, - arangodb::ResourceMonitor& resourceMonitor, - WeightedTwoSidedEnumerator& parent) + arangodb::ResourceMonitor& resourceMonitor) : _resourceMonitor(resourceMonitor), _interior(resourceMonitor), _queue(resourceMonitor), @@ -68,8 +67,7 @@ WeightedTwoSidedEnumerator< _direction(dir), _graphOptions(options), _diameter(-std::numeric_limits::infinity()), - _haveSeenOtherSide(false), - _parent(parent) {} + _haveSeenOtherSide(false) {} template @@ -257,12 +255,6 @@ auto WeightedTwoSidedEnumerator void { if (_graphOptions.isKilled()) { - // First clear our own instance (Ball) - clear(); - // Then clear the other instance (Ball) - other.clear(); - // Then clear the parent (WeightedTwoSidedEnumerator) - _parent.clear(); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } @@ -426,9 +418,9 @@ WeightedTwoSidedEnumerator::searchDone() -> bool { if (_options.isKilled()) { - // Here we're not inside a Ball, so we can clear via main clear method - clear(); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } if ((_left.noPathLeft() && _right.noPathLeft()) || isAlgorithmFinished()) { diff --git a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h index 57fc0dae0f37..f2e336604707 100644 --- a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h +++ b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h @@ -221,8 +221,7 @@ class WeightedTwoSidedEnumerator { public: Ball(Direction dir, ProviderType&& provider, GraphOptions const& options, PathValidatorOptions validatorOptions, - arangodb::ResourceMonitor& resourceMonitor, - WeightedTwoSidedEnumerator& parent); + arangodb::ResourceMonitor& resourceMonitor); ~Ball(); auto clear() -> void; auto reset(VertexRef center, size_t depth = 0) -> void; @@ -297,11 +296,6 @@ class WeightedTwoSidedEnumerator { GraphOptions _graphOptions; double _diameter = -std::numeric_limits::infinity(); bool _haveSeenOtherSide; - - // Reference to the parent WeightedTwoSidedEnumerator - // Intention: To be able to call clear() on the parent - // Case: When a kill signal is received - WeightedTwoSidedEnumerator& _parent; }; enum BallSearchLocation { LEFT, RIGHT, FINISH }; diff --git a/arangod/Graph/Providers/ClusterProvider.cpp b/arangod/Graph/Providers/ClusterProvider.cpp index 38b57b364cf5..4ab865922310 100644 --- a/arangod/Graph/Providers/ClusterProvider.cpp +++ b/arangod/Graph/Providers/ClusterProvider.cpp @@ -456,7 +456,6 @@ template auto ClusterProvider::fetchVertices( std::vector const& looseEnds) -> std::vector { if (_opts.isKilled()) { - clear(); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } std::vector result{}; @@ -486,7 +485,6 @@ template auto ClusterProvider::fetchEdges( std::vector const& fetchedVertices) -> Result { if (_opts.isKilled()) { - clear(); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } @@ -521,7 +519,6 @@ auto ClusterProvider::expand( Step const& step, size_t previous, std::function const& callback) -> void { if (_opts.isKilled()) { - clear(); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } diff --git a/arangod/Graph/Providers/SingleServerProvider.cpp b/arangod/Graph/Providers/SingleServerProvider.cpp index 3391980213d3..5b1d7cd66966 100644 --- a/arangod/Graph/Providers/SingleServerProvider.cpp +++ b/arangod/Graph/Providers/SingleServerProvider.cpp @@ -121,7 +121,6 @@ auto SingleServerProvider::expand( auto const& vertex = step.getVertex(); if (_opts.isKilled()) { - clear(); THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } From 053e706aa6622af20655412e5bb99c2ae6fe40ed Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Wed, 21 May 2025 00:16:01 +0200 Subject: [PATCH 17/28] format --- arangod/Graph/Enumerators/TwoSidedEnumerator.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp b/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp index e5e7593a5c66..0f0bd630026c 100644 --- a/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/TwoSidedEnumerator.cpp @@ -316,14 +316,10 @@ TwoSidedEnumerator:: PathValidatorOptions validatorOptions, arangodb::ResourceMonitor& resourceMonitor) : _options(std::move(options)), - _left{Direction::FORWARD, std::move(forwardProvider), - _options, validatorOptions, - resourceMonitor}, - _right{Direction::BACKWARD, - std::move(backwardProvider), - _options, - std::move(validatorOptions), - resourceMonitor}, + _left{Direction::FORWARD, std::move(forwardProvider), _options, + validatorOptions, resourceMonitor}, + _right{Direction::BACKWARD, std::move(backwardProvider), _options, + std::move(validatorOptions), resourceMonitor}, _baselineDepth(_options.getMaxDepth()), _resultPath{_left.provider(), _right.provider()} {} From fbc3be7c0f3e4471f38040971701a2bd573a5021 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Thu, 10 Jul 2025 13:11:45 +0200 Subject: [PATCH 18/28] improved wait & kill mechanism - should now be way more stable --- ...h-traversal-generic-bidirectional-tests.js | 174 +++++++++++++----- 1 file changed, 132 insertions(+), 42 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js index 217355fb3dc9..db9f2677a47d 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js @@ -16,6 +16,21 @@ const arango = internal.arango; const VERIFICATION_TIME_BUFFER = internal.isCluster() ? 10 : 5; const localHelper = { + formatTimestamp: function() { + const now = new Date(); + const day = now.getDate().toString().padStart(2, '0'); + const month = (now.getMonth() + 1).toString().padStart(2, '0'); + const year = now.getFullYear(); + const hours = now.getHours().toString().padStart(2, '0'); + const minutes = now.getMinutes().toString().padStart(2, '0'); + const seconds = now.getSeconds().toString().padStart(2, '0'); + const milliseconds = now.getMilliseconds().toString().padStart(3, '0'); + + return `[${day}/${month}/${year} ${hours}:${minutes}:${seconds}.${milliseconds}]`; + }, + debugPrint: function(...args) { + print(this.formatTimestamp(), ...args); + }, getRunningQueries: function() { return arango.GET('/_api/query/current'); }, @@ -26,54 +41,79 @@ const localHelper = { checkRunningQuery: function(queryString, maxAttempts = 10, debug = false) { const normalizedQueryString = this.normalizeQueryString(queryString); if (debug) { - print("Checking for running query:", normalizedQueryString); + this.debugPrint("Checking for running query:", normalizedQueryString); } for (let i = 0; i < maxAttempts; i++) { const runningQueries = this.getRunningQueries(); if (debug) { - print("Attempt", i + 1, "of", maxAttempts); - print("Number of running queries:", runningQueries.length); + this.debugPrint("Attempt", i + 1, "of", maxAttempts); + this.debugPrint("Number of running queries:", runningQueries.length); } const matchingQuery = runningQueries.find(query => this.normalizeQueryString(query.query) === normalizedQueryString ); if (matchingQuery) { if (debug) { - print("Found matching query with ID:", matchingQuery.id); + this.debugPrint("Found matching query with ID:", matchingQuery.id); } return matchingQuery; } if (debug) { - print("No matching query found, waiting..."); + this.debugPrint("No matching query found, waiting..."); } internal.sleep(1); } if (debug) { - print("Failed to find running query after", maxAttempts, "attempts"); + this.debugPrint("Failed to find running query after", maxAttempts, "attempts"); } assertTrue(false, "Query not found"); }, - waitForQueryTermination: function(queryId, maxAttempts = 10) { + waitForQueryTermination: function(queryId, maxAttempts = 10, debug = false) { for (let i = 0; i < maxAttempts; i++) { const runningQueries = this.getRunningQueries(); const queryStillRunning = runningQueries.some(query => query.id === queryId); if (!queryStillRunning) { + if (debug) { + this.debugPrint("Query", queryId, "terminated after", i + 1, "attempts"); + } return true; } + if (debug) { + this.debugPrint("Waiting for query", queryId, "to terminate, attempt", i + 1, "of", maxAttempts); + } internal.sleep(1); } assertTrue(false, "Query did not terminate within expected time"); }, - killQuery: function(queryId) { + killQuery: function(queryId, debug = false) { const dbName = db._name(); + if (debug) { + this.debugPrint("Attempting to kill query ID:", queryId); + } const response = arango.DELETE('/_db/' + dbName + '/_api/query/' + queryId); - assertEqual(response.code, 200); + if (debug) { + this.debugPrint("Kill query response code:", response.code); + if (response.code !== 200) { + this.debugPrint("Kill query failed with error:", response.error, "errorNum:", response.errorNum); + } + } + if (response.code !== 200) { + if (response.code === 404 && response.errorNum === 1591) { + // Query not found - likely already completed + if (debug) { + this.debugPrint("Query not found (404/1591) - likely already completed"); + } + } else { + print("Failed to kill query:", response.code, response.error, response.errorNum); + assertEqual(response.code, 200); + } + } return response; }, executeAsyncQuery: function(queryString, debug = false, disableAsyncHeader = false) { // This helper will just accept the query. It will return the async id. if (debug) { - print("Executing async query:", this.normalizeQueryString(queryString)); + this.debugPrint("Executing async query:", this.normalizeQueryString(queryString)); } const headers = disableAsyncHeader ? {} : {'x-arango-async': 'store'}; const response = arango.POST_RAW('/_api/cursor', @@ -85,10 +125,10 @@ const localHelper = { ); if (debug) { if (disableAsyncHeader) { - print(response); + this.debugPrint(response); } - print("Async query response code:", response.code); - print("Async query ID:", response.headers['x-arango-async-id']); + this.debugPrint("Async query response code:", response.code); + this.debugPrint("Async query ID:", response.headers['x-arango-async-id']); } assertEqual(response.code, 202); assertTrue(response.headers.hasOwnProperty("x-arango-async-id")); @@ -109,74 +149,124 @@ const localHelper = { // First run - measure execution time const startTime = Date.now(); if (debug) { - print("Starting first query execution..."); + this.debugPrint("Starting first query execution..."); } const queryJobId = this.executeAsyncQuery(queryString, debug, disableAsyncHeader); if (debug) { - print("First query job ID:", queryJobId); + this.debugPrint("First query job ID:", queryJobId); } const runningQuery = this.checkRunningQuery(queryString, 10, debug); const queryId = runningQuery.id; if (debug) { - print("First query ID:", queryId); + this.debugPrint("First query ID:", queryId); } - this.killQuery(queryId); + this.killQuery(queryId, debug); this.checkJobStatus(queryJobId); - this.waitForQueryTermination(queryId); + this.waitForQueryTermination(queryId, 10, debug); const firstExecutionTime = (Date.now() - startTime) / 1000; // Convert to seconds if (debug) { - print("First query execution time:", firstExecutionTime, "seconds"); + this.debugPrint("First query execution time:", firstExecutionTime, "seconds"); } // Second run - verify query stays running if (debug) { - print("Starting second query execution..."); + this.debugPrint("Starting second query execution..."); } const startTime2 = Date.now(); const queryJobId2 = this.executeAsyncQuery(queryString, debug, disableAsyncHeader); if (debug) { - print("Second query job ID:", queryJobId2); + this.debugPrint("Second query job ID:", queryJobId2); } const runningQuery2 = this.checkRunningQuery(queryString, 10, debug); const queryId2 = runningQuery2.id; if (debug) { - print("Second query ID:", queryId2); + this.debugPrint("Second query ID:", queryId2); } - // Wait for executionTime + VERIFICATION_TIME_BUFFER seconds while checking if query is still running - const verificationTime = firstExecutionTime + VERIFICATION_TIME_BUFFER; + // Wait for a reasonable time to verify the query is long-running, then kill it + // We'll wait for firstExecutionTime + small buffer, then kill to test the kill functionality + const killAttemptTime = firstExecutionTime + Math.min(VERIFICATION_TIME_BUFFER / 2, 2.0); // Kill after first exec time + max 2 seconds if (debug) { - print("Verification time:", verificationTime, "seconds (" + VERIFICATION_TIME_BUFFER + " seconds more than the first query)"); + this.debugPrint("Will attempt to kill query at:", killAttemptTime, "seconds"); } + const startVerification = Date.now(); - while ((Date.now() - startVerification) / 1000 < verificationTime) { + let queryKilled = false; + + while ((Date.now() - startVerification) / 1000 < killAttemptTime) { const runningQueries = this.getRunningQueries(); const queryStillRunning = runningQueries.some(query => query.id === queryId2); if (debug) { - print("Query still running:", queryStillRunning); + this.debugPrint("Query still running:", queryStillRunning); } - assertTrue(queryStillRunning, "Query terminated before verification time"); + + if (!queryStillRunning) { + // Query completed before we could kill it + const secondExecutionTime = (Date.now() - startTime2) / 1000; + if (debug) { + this.debugPrint("Query completed naturally before kill attempt. Second query execution time:", secondExecutionTime, "seconds"); + this.debugPrint("This indicates the query is completing faster than expected, which defeats the purpose of the kill test."); + } + + assertTrue(false, + `Query completed naturally in ${secondExecutionTime}s before kill attempt. This test requires a query that runs longer than ${killAttemptTime}s to properly test the kill functionality.`); + } + internal.sleep(1); } - // Now kill the query and verify termination + // Now attempt to kill the query if (debug) { - print("Killing second query..."); + this.debugPrint("Attempting to kill query after verification period..."); } - this.killQuery(queryId2); - this.checkJobStatus(queryJobId2); - this.waitForQueryTermination(queryId2); - - // Verify that second query ran longer than first query - const secondExecutionTime = (Date.now() - startTime2) / 1000; + + const runningQueries = this.getRunningQueries(); + const queryStillRunning = runningQueries.some(query => query.id === queryId2); + if (debug) { - print("Second query execution time:", secondExecutionTime, "seconds"); + this.debugPrint("Query still running at kill time:", queryStillRunning); } - assertTrue(secondExecutionTime > firstExecutionTime, - `Second query (${secondExecutionTime}s) did not run longer than first query (${firstExecutionTime}s). Kill may not have worked.`); - - if (debug) { - print("Test completed successfully"); + + if (queryStillRunning) { + const killResponse = this.killQuery(queryId2, debug); + + // Check if kill was successful or query already completed + if (killResponse.code === 200) { + // Query was successfully killed + this.checkJobStatus(queryJobId2); + this.waitForQueryTermination(queryId2, 10, debug); + + // Verify that second query ran longer than first query + const secondExecutionTime = (Date.now() - startTime2) / 1000; + if (debug) { + this.debugPrint("Second query execution time:", secondExecutionTime, "seconds"); + } + assertTrue(secondExecutionTime > firstExecutionTime, + `Second query (${secondExecutionTime}s) did not run longer than first query (${firstExecutionTime}s). Kill may not have worked.`); + + if (debug) { + this.debugPrint("Test completed successfully - query was killed mid-execution"); + } + } else if (killResponse.code === 404 && killResponse.errorNum === 1591) { + // Query completed between the check and kill attempt - race condition + const secondExecutionTime = (Date.now() - startTime2) / 1000; + if (debug) { + this.debugPrint("Query completed between check and kill attempt (race condition). Second query execution time:", secondExecutionTime, "seconds"); + } + + // This is still a race condition, but much less likely now + assertTrue(false, + `Query completed naturally in ${secondExecutionTime}s during kill attempt. Race condition occurred.`); + } + } else { + // Query completed right before kill attempt + const secondExecutionTime = (Date.now() - startTime2) / 1000; + if (debug) { + this.debugPrint("Query completed just before kill attempt. Second query execution time:", secondExecutionTime, "seconds"); + } + + assertTrue(false, + `Query completed naturally in ${secondExecutionTime}s just before kill attempt. This test requires a query that runs longer to properly test the kill functionality.`); } } }; From 7db1e7b435444b409dcd199a1216eee8ba1bb50c Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Thu, 10 Jul 2025 14:12:22 +0200 Subject: [PATCH 19/28] adaptive kill query --- ...ql-graph-traversal-generic-bidirectional-tests.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js index db9f2677a47d..97f1b4694c5d 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js @@ -184,10 +184,14 @@ const localHelper = { } // Wait for a reasonable time to verify the query is long-running, then kill it - // We'll wait for firstExecutionTime + small buffer, then kill to test the kill functionality - const killAttemptTime = firstExecutionTime + Math.min(VERIFICATION_TIME_BUFFER / 2, 2.0); // Kill after first exec time + max 2 seconds + // Use adaptive timing: kill at a percentage of the first query's execution time + // This adapts to both single server (~8s) and cluster (~3s) environments + const killAttemptRatio = 0.6; // Kill at 60% of first query time + const minKillTime = 1.0; // Minimum 1 second to ensure query has started + const killAttemptTime = Math.max(firstExecutionTime * killAttemptRatio, minKillTime); if (debug) { this.debugPrint("Will attempt to kill query at:", killAttemptTime, "seconds"); + this.debugPrint("Kill timing: firstExecutionTime(" + firstExecutionTime + ") * ratio(" + killAttemptRatio + ") = " + (firstExecutionTime * killAttemptRatio) + ", using max with minKillTime(" + minKillTime + ")"); } const startVerification = Date.now(); @@ -209,7 +213,7 @@ const localHelper = { } assertTrue(false, - `Query completed naturally in ${secondExecutionTime}s before kill attempt. This test requires a query that runs longer than ${killAttemptTime}s to properly test the kill functionality.`); + `Query completed naturally in ${secondExecutionTime}s before kill attempt at ${killAttemptTime}s. This test requires a query that runs longer than the kill attempt time to properly test the kill functionality.`); } internal.sleep(1); @@ -266,7 +270,7 @@ const localHelper = { } assertTrue(false, - `Query completed naturally in ${secondExecutionTime}s just before kill attempt. This test requires a query that runs longer to properly test the kill functionality.`); + `Query completed naturally in ${secondExecutionTime}s just before kill attempt at ${killAttemptTime}s. This test requires a query that runs longer to properly test the kill functionality.`); } } }; From d3b10bafc5d3a915523dd081c33252294058ca5b Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Thu, 10 Jul 2025 16:05:44 +0200 Subject: [PATCH 20/28] increase debug logging. Some are optional, some are mandatory. If our CI does not see any log output for 20 minutes - It will abort the job --- .../aql-graph-traversal-generic-graphs.js | 87 +++++++++++++++---- .../aql-graph-traversal-generic-cluster.js | 25 +++++- .../aql-graph-traversal-generic-noncluster.js | 25 +++++- 3 files changed, 114 insertions(+), 23 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index 98263a4fbedd..29abb2b6e6a6 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -151,12 +151,16 @@ class TestGraph { }; this.debug = false; } - + hasProjectionPayload() { return this.addProjectionPayload; } create() { + const startTime = Date.now(); + const estimatedVertices = this.edges.length > 0 ? new Set(this.edges.map(e => [e[0], e[1]]).flat()).size : 0; + print(`[${new Date().toISOString()}] Creating graph: ${this.graphName} (${this.edges.length} edges, estimated ${estimatedVertices} vertices)`); + switch (this.testVariant) { case TestVariants.SingleServer: { cgm._create(this.name(), [this.eRel], [this.on], {}); @@ -229,6 +233,9 @@ class TestGraph { } } + const graphSetupTime = Date.now() - startTime; + print(`[${new Date().toISOString()}] Graph structure created: ${this.graphName} (${graphSetupTime}ms)`); + let vertexSharding = []; if (isCluster && (this.testVariant !== TestVariants.EnterpriseGraph) && (this.testVariant !== TestVariants.DisjointEnterpriseGraph)) { // Only create proper smart/vertex sharding settings in cluster mode @@ -239,9 +246,15 @@ class TestGraph { vertexSharding = this.protoSmartSharding.map(([v, i]) => [v, shardAttrsByShardIndex[i]]); } - this.verticesByName = TestGraph._fillGraph(this.graphName, this.edges, db[this.vn], db[this.en], this.unconnectedVertices, vertexSharding, this.addProjectionPayload); + print(`[${new Date().toISOString()}] Filling graph data: ${this.graphName} (vertices and edges)`); + this.verticesByName = TestGraph._fillGraph(this.graphName, this.edges, db[this.vn], db[this.en], this.unconnectedVertices, vertexSharding, this.addProjectionPayload, this.debug); + + print(`[${new Date().toISOString()}] Creating index on edge collection: ${this.en}`); db[this.en].ensureIndex({type: "persistent", fields: ["_from", graphIndexedAttribute]}); - + + const totalTime = Date.now() - startTime; + print(`[${new Date().toISOString()}] Graph creation completed: ${this.graphName} (total time: ${totalTime}ms)`); + if (this.debug) { print("First 10 vertices in graph " + this.graphName + ":"); const first10Vertices = Object.entries(this.verticesByName).slice(0, 10); @@ -332,9 +345,13 @@ class TestGraph { * key and the second the smart attribute. * @param addProjectionPayload Boolean flag, to define if we should add heavy payload to vertices and edges * in order to test projections + * @param debug Boolean flag to enable detailed batch progress logging * @private */ - static _fillGraph(gn, edges, vc, ec, unconnectedVertices, vertexSharding = [], addProjectionPayload = false) { + static _fillGraph(gn, edges, vc, ec, unconnectedVertices, vertexSharding = [], addProjectionPayload = false, debug = false) { + const fillStartTime = Date.now(); + print(`[${new Date().toISOString()}] Starting _fillGraph for ${gn}: ${edges.length} edges, ${unconnectedVertices.length} unconnected vertices`); + const vertices = new Map(vertexSharding); for (const edge of edges) { if (!vertices.has(edge[0])) { @@ -350,6 +367,10 @@ class TestGraph { } } + const vertexMapTime = Date.now() - fillStartTime; + print(`[${new Date().toISOString()}] Vertex mapping completed for ${gn}: ${vertices.size} total vertices (${vertexMapTime}ms)`); + + function* payloadGenerator() { // Just some hardcoded repeated payload. // We will never check the content it should just produce @@ -404,12 +425,19 @@ class TestGraph { // Process vertices in smaller batches to reduce memory pressure const VERTEX_BATCH_SIZE = 10000; // Reduced from 100k to 10k + const vertexStartTime = Date.now(); + print(`[${new Date().toISOString()}] Processing vertices for ${gn}: ${toSave.length} vertices in batches of ${VERTEX_BATCH_SIZE}`); + for (let i = 0; i < toSave.length; i += VERTEX_BATCH_SIZE) { const batch = toSave.slice(i, i + VERTEX_BATCH_SIZE); const batchKeys = keys.slice(i, i + VERTEX_BATCH_SIZE); - if (this.debug) { - print(`Saving vertex batch ${i} of ${toSave.length}`); + const batchNum = Math.floor(i / VERTEX_BATCH_SIZE) + 1; + const totalBatches = Math.ceil(toSave.length / VERTEX_BATCH_SIZE); + + if (debug) { + print(`[${new Date().toISOString()}] Saving vertex batch ${batchNum}/${totalBatches} (${batch.length} vertices, ${i}-${i + batch.length - 1})`); } + vc.save(batch).forEach((d, idx) => { verticesByName[batchKeys[idx]] = d._id; return null; @@ -418,6 +446,10 @@ class TestGraph { batch.length = 0; batchKeys.length = 0; } + + const vertexProcessTime = Date.now() - vertexStartTime; + print(`[${new Date().toISOString()}] Vertex processing completed for ${gn}: ${toSave.length} vertices (${vertexProcessTime}ms)`); + // Clear the full arrays after processing toSave.length = 0; keys.length = 0; @@ -425,9 +457,14 @@ class TestGraph { // Process edges in smaller batches to reduce memory pressure const EDGE_BATCH_SIZE = 10000; // Reduced from 100k to 10k + const edgeStartTime = Date.now(); + print(`[${new Date().toISOString()}] Processing edges for ${gn}: ${edges.length} edges in batches of ${EDGE_BATCH_SIZE}`); + let edgeDocs = []; let currentBatch = []; let currentBatchSize = 0; + let edgeCount = 0; + let batchNum = 0; // Process edges in a streaming fashion for (const [v, w, weight] of edges) { @@ -436,7 +473,7 @@ class TestGraph { _to: verticesByName[w], secondFrom: verticesByName[v] }; - + if (weight && typeof weight === 'number') { edge[graphWeightAttribute] = weight; edge[graphIndexedAttribute] = weight; @@ -450,12 +487,16 @@ class TestGraph { currentBatch.push(edge); currentBatchSize++; + edgeCount++; // When batch is full, save it and clear memory if (currentBatchSize >= EDGE_BATCH_SIZE) { - if (this.debug) { - print(`Saving edge batch of size ${currentBatchSize}`); + batchNum++; + const totalBatches = Math.ceil(edges.length / EDGE_BATCH_SIZE); + if (debug) { + print(`[${new Date().toISOString()}] Saving edge batch ${batchNum}/${totalBatches} (${currentBatchSize} edges, total processed: ${edgeCount})`); } + ec.save(currentBatch); currentBatch = []; currentBatchSize = 0; @@ -464,12 +505,20 @@ class TestGraph { // Save any remaining edges if (currentBatch.length > 0) { - if (this.debug) { - print(`Saving final edge batch of size ${currentBatch.length}`); + batchNum++; + const totalBatches = Math.ceil(edges.length / EDGE_BATCH_SIZE); + if (debug) { + print(`[${new Date().toISOString()}] Saving final edge batch ${batchNum}/${totalBatches} (${currentBatch.length} edges)`); } + ec.save(currentBatch); } + const edgeProcessTime = Date.now() - edgeStartTime; + const totalFillTime = Date.now() - fillStartTime; + print(`[${new Date().toISOString()}] Edge processing completed for ${gn}: ${edges.length} edges (${edgeProcessTime}ms)`); + print(`[${new Date().toISOString()}] _fillGraph completed for ${gn}: total time ${totalFillTime}ms`); + return verticesByName; } @@ -899,12 +948,12 @@ protoGraphs.completeGraph = new ProtoGraph("completeGraph", [ const generateNodeNames = (count) => { const alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; const nodes = []; - + // First add single letter nodes for (let i = 0; i < Math.min(count, alphabet.length); i++) { nodes.push(alphabet[i]); } - + // If we need more nodes, add two-letter combinations if (count > alphabet.length) { for (let i = 0; i < alphabet.length && nodes.length < count; i++) { @@ -913,7 +962,7 @@ const generateNodeNames = (count) => { } } } - + return nodes; }; @@ -943,7 +992,7 @@ const generateCompleteGraphEdges = (nodes) => { const hugeCompleteGraphNodes = generateNodeNames(100); const hugeCompleteGraphEdges = generateCompleteGraphEdges(hugeCompleteGraphNodes); -protoGraphs.hugeCompleteGraph = new ProtoGraph("hugeCompleteGraph", +protoGraphs.hugeCompleteGraph = new ProtoGraph("hugeCompleteGraph", hugeCompleteGraphEdges, [1, 2, 5], [ @@ -965,7 +1014,7 @@ protoGraphs.hugeCompleteGraph = new ProtoGraph("hugeCompleteGraph", /* * Grid Graph Structure (1000x1000) * Each node connects to its right and bottom neighbors - * + * * 1 → 2 → 3 → ... → 1000 * ↓ ↓ ↓ ↓ * 1001 → 1002 → 1003 → ... → 2000 @@ -979,7 +1028,7 @@ protoGraphs.hugeCompleteGraph = new ProtoGraph("hugeCompleteGraph", const generateGridGraph = (width, height) => { const nodes = []; const edges = []; - + // Generate node names as simple numbers const getNodeName = (row, col) => { // Calculate node number: row * width + col + 1 @@ -992,12 +1041,12 @@ const generateGridGraph = (width, height) => { for (let col = 0; col < width; col++) { const nodeName = getNodeName(row, col); nodes.push(nodeName); - + // Connect to right neighbor if (col < width - 1) { edges.push([nodeName, getNodeName(row, col + 1), 1]); } - + // Connect to bottom neighbor if (row < height - 1) { edges.push([nodeName, getNodeName(row + 1, col), 1]); diff --git a/tests/js/client/aql/aql-graph-traversal-generic-cluster.js b/tests/js/client/aql/aql-graph-traversal-generic-cluster.js index 1bed11b6a22c..809b363e5462 100644 --- a/tests/js/client/aql/aql-graph-traversal-generic-cluster.js +++ b/tests/js/client/aql/aql-graph-traversal-generic-cluster.js @@ -47,13 +47,34 @@ function graphTraversalGenericGeneralGraphClusterSuite() { const suite = { setUpAll: function () { try { + const startTime = Date.now(); const numGraphs = _.sumBy(_.values(testGraphs), g => _.keys(g).length); - console.info(`Creating ${numGraphs} graphs, this might take a few seconds.`); - _.each(testGraphs, function (graphs) { + const graphsByType = _.mapValues(testGraphs, g => _.keys(g).length); + + print(`[${new Date().toISOString()}] ========== GRAPH CREATION STARTED ==========`); + print(`[${new Date().toISOString()}] Creating ${numGraphs} graphs total:`); + _.each(graphsByType, (count, type) => { + if (count > 0) { + print(`[${new Date().toISOString()}] - ${type}: ${count} graphs`); + } + }); + + let graphCount = 0; + _.each(testGraphs, function (graphs, protoGraphName) { + if (_.keys(graphs).length > 0) { + print(`[${new Date().toISOString()}] Creating ${protoGraphName} graphs (${_.keys(graphs).length} variations)...`); + } _.each(graphs, function (graph) { + graphCount++; + print(`[${new Date().toISOString()}] [${graphCount}/${numGraphs}] Starting: ${graph.name()}`); graph.create(); + print(`[${new Date().toISOString()}] [${graphCount}/${numGraphs}] Completed: ${graph.name()}`); }); }); + + const totalTime = Date.now() - startTime; + print(`[${new Date().toISOString()}] ========== GRAPH CREATION COMPLETED ==========`); + print(`[${new Date().toISOString()}] All ${numGraphs} graphs created successfully in ${totalTime}ms (${(totalTime/1000).toFixed(1)}s)`); } catch (e) { console.error(e); console.error(e.stack); diff --git a/tests/js/client/aql/aql-graph-traversal-generic-noncluster.js b/tests/js/client/aql/aql-graph-traversal-generic-noncluster.js index 1b3a6d03a300..7f965b891f1d 100644 --- a/tests/js/client/aql/aql-graph-traversal-generic-noncluster.js +++ b/tests/js/client/aql/aql-graph-traversal-generic-noncluster.js @@ -43,13 +43,34 @@ function graphTraversalGenericGeneralGraphStandaloneSuite() { const suite = { setUpAll: function () { try { + const startTime = Date.now(); const numGraphs = _.sumBy(_.values(testGraphs), g => _.keys(g).length); - console.info(`Creating ${numGraphs} graphs, this might take a few seconds.`); - _.each(testGraphs, function (graphs) { + const graphsByType = _.mapValues(testGraphs, g => _.keys(g).length); + + print(`[${new Date().toISOString()}] ========== GRAPH CREATION STARTED ==========`); + print(`[${new Date().toISOString()}] Creating ${numGraphs} graphs total:`); + _.each(graphsByType, (count, type) => { + if (count > 0) { + print(`[${new Date().toISOString()}] - ${type}: ${count} graphs`); + } + }); + + let graphCount = 0; + _.each(testGraphs, function (graphs, protoGraphName) { + if (_.keys(graphs).length > 0) { + print(`[${new Date().toISOString()}] Creating ${protoGraphName} graphs (${_.keys(graphs).length} variations)...`); + } _.each(graphs, function (graph) { + graphCount++; + print(`[${new Date().toISOString()}] [${graphCount}/${numGraphs}] Starting: ${graph.name()}`); graph.create(); + print(`[${new Date().toISOString()}] [${graphCount}/${numGraphs}] Completed: ${graph.name()}`); }); }); + + const totalTime = Date.now() - startTime; + print(`[${new Date().toISOString()}] ========== GRAPH CREATION COMPLETED ==========`); + print(`[${new Date().toISOString()}] All ${numGraphs} graphs created successfully in ${totalTime}ms (${(totalTime/1000).toFixed(1)}s)`); } catch (e) { console.error(e); console.error(e.stack); From 670eb987178286d9021e365caa3e3eec94f77b6b Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Thu, 10 Jul 2025 17:14:02 +0200 Subject: [PATCH 21/28] eslint --- tests/js/client/aql/aql-graph-traversal-generic-cluster.js | 2 +- tests/js/client/aql/aql-graph-traversal-generic-noncluster.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/js/client/aql/aql-graph-traversal-generic-cluster.js b/tests/js/client/aql/aql-graph-traversal-generic-cluster.js index 809b363e5462..0ef0592662f6 100644 --- a/tests/js/client/aql/aql-graph-traversal-generic-cluster.js +++ b/tests/js/client/aql/aql-graph-traversal-generic-cluster.js @@ -1,5 +1,5 @@ /*jshint globalstrict:true, strict:true, esnext: true */ -/*global assertTrue, instanceManager */ +/*global assertTrue, instanceManager, print */ "use strict"; diff --git a/tests/js/client/aql/aql-graph-traversal-generic-noncluster.js b/tests/js/client/aql/aql-graph-traversal-generic-noncluster.js index 7f965b891f1d..f6351383e380 100644 --- a/tests/js/client/aql/aql-graph-traversal-generic-noncluster.js +++ b/tests/js/client/aql/aql-graph-traversal-generic-noncluster.js @@ -1,4 +1,5 @@ /*jshint globalstrict:true, strict:true, esnext: true */ +/*global print */ "use strict"; From 80271fb88ccf84988c1fef54749f91b238b57b02 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Fri, 11 Jul 2025 10:18:50 +0200 Subject: [PATCH 22/28] more test output --- .../aql-graph-traversal-generic-graphs.js | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index 29abb2b6e6a6..96d9b114aa31 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -423,19 +423,24 @@ class TestGraph { keys.push(vertexKey); } - // Process vertices in smaller batches to reduce memory pressure - const VERTEX_BATCH_SIZE = 10000; // Reduced from 100k to 10k + // Process vertices in smaller batches to reduce memory pressure and get more frequent progress updates + const VERTEX_BATCH_SIZE = 5000; // Optimized for large graphs const vertexStartTime = Date.now(); print(`[${new Date().toISOString()}] Processing vertices for ${gn}: ${toSave.length} vertices in batches of ${VERTEX_BATCH_SIZE}`); + if (toSave.length > 100000) { + print(`[${new Date().toISOString()}] Large vertex count detected - progress updates every 10 batches (~${VERTEX_BATCH_SIZE * 10} vertices)`); + } + for (let i = 0; i < toSave.length; i += VERTEX_BATCH_SIZE) { const batch = toSave.slice(i, i + VERTEX_BATCH_SIZE); const batchKeys = keys.slice(i, i + VERTEX_BATCH_SIZE); const batchNum = Math.floor(i / VERTEX_BATCH_SIZE) + 1; const totalBatches = Math.ceil(toSave.length / VERTEX_BATCH_SIZE); - if (debug) { - print(`[${new Date().toISOString()}] Saving vertex batch ${batchNum}/${totalBatches} (${batch.length} vertices, ${i}-${i + batch.length - 1})`); + // Progress update every 10 batches or on debug mode + if (debug || (batchNum % 10 === 0)) { + print(`[${new Date().toISOString()}] Saving vertex batch ${batchNum}/${totalBatches} (${batch.length} vertices, processed: ${i + batch.length}/${toSave.length})`); } vc.save(batch).forEach((d, idx) => { @@ -455,11 +460,15 @@ class TestGraph { keys.length = 0; } - // Process edges in smaller batches to reduce memory pressure - const EDGE_BATCH_SIZE = 10000; // Reduced from 100k to 10k + // Process edges in smaller batches to reduce memory pressure and get more frequent progress updates + const EDGE_BATCH_SIZE = 5000; // Optimized for large graphs const edgeStartTime = Date.now(); print(`[${new Date().toISOString()}] Processing edges for ${gn}: ${edges.length} edges in batches of ${EDGE_BATCH_SIZE}`); + if (edges.length > 100000) { + print(`[${new Date().toISOString()}] Large edge count detected - progress updates every 10 batches (~${EDGE_BATCH_SIZE * 10} edges)`); + } + let edgeDocs = []; let currentBatch = []; let currentBatchSize = 0; @@ -493,8 +502,10 @@ class TestGraph { if (currentBatchSize >= EDGE_BATCH_SIZE) { batchNum++; const totalBatches = Math.ceil(edges.length / EDGE_BATCH_SIZE); - if (debug) { - print(`[${new Date().toISOString()}] Saving edge batch ${batchNum}/${totalBatches} (${currentBatchSize} edges, total processed: ${edgeCount})`); + + // Progress update every 10 batches or on debug mode + if (debug || (batchNum % 10 === 0)) { + print(`[${new Date().toISOString()}] Saving edge batch ${batchNum}/${totalBatches} (${currentBatchSize} edges, total processed: ${edgeCount}/${edges.length})`); } ec.save(currentBatch); @@ -507,9 +518,9 @@ class TestGraph { if (currentBatch.length > 0) { batchNum++; const totalBatches = Math.ceil(edges.length / EDGE_BATCH_SIZE); - if (debug) { - print(`[${new Date().toISOString()}] Saving final edge batch ${batchNum}/${totalBatches} (${currentBatch.length} edges)`); - } + + // Always show final batch completion + print(`[${new Date().toISOString()}] Saving final edge batch ${batchNum}/${totalBatches} (${currentBatch.length} edges)`); ec.save(currentBatch); } From 9ab0e8ce9bea291428b64e2bb351474c7b7d7a95 Mon Sep 17 00:00:00 2001 From: Jure Bajic Date: Fri, 11 Jul 2025 14:28:13 +0200 Subject: [PATCH 23/28] Change test suite params --- tests/test-definitions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-definitions.txt b/tests/test-definitions.txt index 0e340b3a94a7..06dfc5753c3a 100644 --- a/tests/test-definitions.txt +++ b/tests/test-definitions.txt @@ -91,7 +91,7 @@ shell_client_multi priority=1500 cluster suffix=http -- --http true shell_client_multi priority=1500 cluster suffix=http2 -- --http2 true # different number of buckets in cluster -shell_client_aql priority=1000 size=medium+ cluster buckets=20 -- --dumpAgencyOnError true +shell_client_aql priority=1000 size=large cluster buckets=5 -- --dumpAgencyOnError true shell_client priority=500 cluster size=medium+ buckets=20 -- --dumpAgencyOnError true shell_client_transaction priority=500 cluster parallelity=5 size=small buckets=5 -- --dumpAgencyOnError true #shell_client_replication2_recovery priority=500 size=medium cluster -- --dumpAgencyOnError true From a42d44f13089aaf70d8ed80c4f58fa6d3f30ac9f Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 14 Jul 2025 11:48:57 +0200 Subject: [PATCH 24/28] refactored generic graph tests based on Jures suggestion --- js/client/modules/@arangodb/testsuites/aql.js | 25 +++++++++++++++++++ .../aql-graph-traversal-generic-cluster.js | 0 .../aql-graph-traversal-generic-noncluster.js | 0 tests/test-definitions.txt | 3 +++ 4 files changed, 28 insertions(+) rename tests/js/client/aql/{ => graph}/aql-graph-traversal-generic-cluster.js (100%) rename tests/js/client/aql/{ => graph}/aql-graph-traversal-generic-noncluster.js (100%) diff --git a/js/client/modules/@arangodb/testsuites/aql.js b/js/client/modules/@arangodb/testsuites/aql.js index e0e136a19d5c..bcaf4ad0b87a 100644 --- a/js/client/modules/@arangodb/testsuites/aql.js +++ b/js/client/modules/@arangodb/testsuites/aql.js @@ -33,6 +33,7 @@ const functionsDocumentation = { 'shell_client_multi': 'shell client tests to be run in multiple protocol environments', 'shell_client_aql': 'AQL tests in the client', 'shell_client_aql_vector': 'AQL tests in the client with vector index feature enabled', + 'shell_client_aql_generic_graph': 'AQL tests in the client with generic graph creation', 'shell_server_only': 'server specific tests', 'shell_client_transaction': 'transaction tests', 'shell_client_replication2_recovery': 'replication2 cluster recovery tests', @@ -60,6 +61,7 @@ const testPaths = { 'shell_server_only': [ tu.pathForTesting('server/shell') ], 'shell_client_aql': [ tu.pathForTesting('client/aql'), tu.pathForTesting('common/aql') ], 'shell_client_aql_vector': [ tu.pathForTesting('client/aql/vector') ], + 'shell_client_aql_generic_graph': [ tu.pathForTesting('client/aql/graph') ], 'shell_client_transaction': [ tu.pathForTesting('client/shell/transaction')], 'shell_client_replication2_recovery': [ tu.pathForTesting('client/shell/transaction/replication2_recovery')], 'shell_client_traffic': [ tu.pathForTesting('client/shell/traffic') ], @@ -241,6 +243,28 @@ function shellClientAqlVector (options) { return rc; } +// ////////////////////////////////////////////////////////////////////////////// +// / @brief TEST: shell_client_aql_generic_graph +// ////////////////////////////////////////////////////////////////////////////// + +function shellClientAqlGenericGraph(options) { + // Note: Created due to the fact that we're generating multiple big graphs in + // this test suite. We need to be able to run this test suite in more + // controlled environment. Also, based on those graphs, we run a lot of + // different graph related tests. It is estimated that this test suite + // consumes time for graph creation and AQL based graph tests. + + let name = 'shell_client_aql_generic_graph'; + let testCases = tu.scanTestPaths(testPaths.shell_client_aql_generic_graph, options); + testCases = tu.splitBuckets(options, testCases); + + let opts = ensureServers(options, 3); + + let rc = new trs.runLocalInArangoshRunner(opts, name, {}).run(testCases); + options.cleanup = options.cleanup && opts.cleanup; + return rc; +} + // ////////////////////////////////////////////////////////////////////////////// // / @brief TEST: shell_client_traffic // ////////////////////////////////////////////////////////////////////////////// @@ -312,6 +336,7 @@ exports.setup = function (testFns, opts, fnDocs, optionsDoc, allTestPaths) { testFns['shell_client_multi'] = shellClientMulti; testFns['shell_client_aql'] = shellClientAql; testFns['shell_client_aql_vector'] = shellClientAqlVector; + testFns['shell_client_aql_generic_graph'] = shellClientAqlGenericGraph; testFns['shell_server_only'] = shellServerOnly; testFns['shell_client_transaction'] = shellClientTransaction; testFns['shell_client_replication2_recovery'] = shellClientReplication2Recovery; diff --git a/tests/js/client/aql/aql-graph-traversal-generic-cluster.js b/tests/js/client/aql/graph/aql-graph-traversal-generic-cluster.js similarity index 100% rename from tests/js/client/aql/aql-graph-traversal-generic-cluster.js rename to tests/js/client/aql/graph/aql-graph-traversal-generic-cluster.js diff --git a/tests/js/client/aql/aql-graph-traversal-generic-noncluster.js b/tests/js/client/aql/graph/aql-graph-traversal-generic-noncluster.js similarity index 100% rename from tests/js/client/aql/aql-graph-traversal-generic-noncluster.js rename to tests/js/client/aql/graph/aql-graph-traversal-generic-noncluster.js diff --git a/tests/test-definitions.txt b/tests/test-definitions.txt index 06dfc5753c3a..f6ecb284f91a 100644 --- a/tests/test-definitions.txt +++ b/tests/test-definitions.txt @@ -102,6 +102,9 @@ rta_makedata enterprise cluster name=rta_makedata_force_oneshard parallelity=5 - # shell aql vector index tests shell_client_aql_vector priority=1000 size=medium+ cluster buckets=1 -- --dumpAgencyOnError true +# shell aql generic graph tests +shell_client_aql_generic_graph priority=1000 size=medium+ cluster buckets=1 -- --dumpAgencyOnError true + # Common Tests importing,export name=import_export parallelity=5 size=small cluster -- --dumpAgencyOnError true From 7717203f209d675145824fb4a23b5747c24caa39 Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Mon, 14 Jul 2025 13:24:18 +0200 Subject: [PATCH 25/28] incr. completegraph to consist out of 1k nodes to make computation more expensive --- ...h-traversal-generic-bidirectional-tests.js | 2 +- .../aql-graph-traversal-generic-graphs.js | 36 ++++++------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js index 97f1b4694c5d..098465351e3a 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js @@ -330,7 +330,7 @@ function testHugeCompleteGraphKPathsLongRunning(testGraph) { assertTrue(testGraph.name().startsWith(protoGraphs.hugeCompleteGraph.name())); const queryString = ` - FOR path IN 1..999 ANY K_PATHS "${testGraph.vertex('A')}" TO "${testGraph.vertex('B')}" + FOR path IN 1..999 ANY K_PATHS "${testGraph.vertex('0')}" TO "${testGraph.vertex('999')}" GRAPH ${testGraph.name()} OPTIONS {useCache: false} RETURN path diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js index 96d9b114aa31..80c789e494bf 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-graphs.js @@ -955,28 +955,6 @@ protoGraphs.completeGraph = new ProtoGraph("completeGraph", [ ] ); -// Generate node names -const generateNodeNames = (count) => { - const alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; - const nodes = []; - - // First add single letter nodes - for (let i = 0; i < Math.min(count, alphabet.length); i++) { - nodes.push(alphabet[i]); - } - - // If we need more nodes, add two-letter combinations - if (count > alphabet.length) { - for (let i = 0; i < alphabet.length && nodes.length < count; i++) { - for (let j = 0; j < alphabet.length && nodes.length < count; j++) { - nodes.push(alphabet[i] + alphabet[j]); - } - } - } - - return nodes; -}; - // Generate edges for complete graph const generateCompleteGraphEdges = (nodes) => { const edges = []; @@ -992,15 +970,23 @@ const generateCompleteGraphEdges = (nodes) => { return edges; }; -// Create the huge complete graph with 100 nodes +// Create the huge complete graph with 1000 nodes /* * B * ↙↗ ↑ ↖↘ * A ← → C // Demonstration of the complete graph - * ↖↘ ↓ ↙↗ // Note: Consists out of 100 nodes + * ↖↘ ↓ ↙↗ // Note: Consists out of 1000 nodes with numeric keys * D */ -const hugeCompleteGraphNodes = generateNodeNames(100); +const generateNumericNodeNames = (count) => { + const nodes = []; + for (let i = 0; i < count; i++) { + nodes.push(i.toString()); + } + return nodes; +}; + +const hugeCompleteGraphNodes = generateNumericNodeNames(1000); const hugeCompleteGraphEdges = generateCompleteGraphEdges(hugeCompleteGraphNodes); protoGraphs.hugeCompleteGraph = new ProtoGraph("hugeCompleteGraph", From 00dc6247adaba68eb64ddac0cdcfa7408dca3d17 Mon Sep 17 00:00:00 2001 From: Markus Pfeiffer Date: Thu, 4 Sep 2025 16:28:32 +0100 Subject: [PATCH 26/28] Some cleanups --- .../WeightedTwoSidedEnumerator.cpp | 2 +- .../Enumerators/WeightedTwoSidedEnumerator.h | 23 +++++++++++-------- .../Graph/Options/OneSidedEnumeratorOptions.h | 2 -- tests/Graph/GenericGraphProviderTest.cpp | 2 -- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp index 5ca32ce2479c..dc72071371cf 100644 --- a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp @@ -865,7 +865,7 @@ WeightedTwoSidedEnumerator auto WeightedTwoSidedEnumerator::searchDone() -> bool { + PathValidator>::searchDone() const -> bool { if (_options.isKilled()) { THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); } diff --git a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h index f2e336604707..0519547f0a9e 100644 --- a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h +++ b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h @@ -259,13 +259,15 @@ class WeightedTwoSidedEnumerator { return _haveSeenOtherSide; } - auto setForbiddenVertices(std::shared_ptr forbidden) - -> void requires HasForbidden { + auto setForbiddenVertices(std::shared_ptr forbidden) -> void + requires HasForbidden + { _validator.setForbiddenVertices(std::move(forbidden)); }; - auto setForbiddenEdges(std::shared_ptr forbidden) - -> void requires HasForbidden { + auto setForbiddenEdges(std::shared_ptr forbidden) -> void + requires HasForbidden + { _validator.setForbiddenEdges(std::move(forbidden)); }; @@ -398,19 +400,22 @@ class WeightedTwoSidedEnumerator { */ auto stealStats() -> aql::TraversalStats; - auto setForbiddenVertices(std::shared_ptr forbidden) - -> void requires HasForbidden { + auto setForbiddenVertices(std::shared_ptr forbidden) -> void + requires HasForbidden + { _left.setForbiddenVertices(forbidden); _right.setForbiddenVertices(std::move(forbidden)); }; - auto setForbiddenEdges(std::shared_ptr forbidden) - -> void requires HasForbidden { + auto setForbiddenEdges(std::shared_ptr forbidden) -> void + requires HasForbidden + { _left.setForbiddenEdges(forbidden); _right.setForbiddenEdges(std::move(forbidden)); }; - private : [[nodiscard]] auto searchDone() -> bool; + private: + [[nodiscard]] auto searchDone() const -> bool; // Ensure that we have fetched all vertices in the _results list. Otherwise, // we will not be able to generate the resulting path auto fetchResults() -> void; diff --git a/arangod/Graph/Options/OneSidedEnumeratorOptions.h b/arangod/Graph/Options/OneSidedEnumeratorOptions.h index 4c5f7a192735..2ca06af8a72c 100644 --- a/arangod/Graph/Options/OneSidedEnumeratorOptions.h +++ b/arangod/Graph/Options/OneSidedEnumeratorOptions.h @@ -25,8 +25,6 @@ #pragma once #include "Graph/Options/QueryContextObserver.h" -// TODO HEIKO: do not include h file here, use forward declaration instead -// TODO HEIKO: do not include h file here, use forward declaration instead #include namespace arangodb::graph { diff --git a/tests/Graph/GenericGraphProviderTest.cpp b/tests/Graph/GenericGraphProviderTest.cpp index e8a6784af1a8..8d0367afcb0a 100644 --- a/tests/Graph/GenericGraphProviderTest.cpp +++ b/tests/Graph/GenericGraphProviderTest.cpp @@ -37,9 +37,7 @@ #include "Graph/Steps/SingleServerProviderStep.h" #include "Graph/TraverserOptions.h" -#include #include -#include using namespace arangodb; using namespace arangodb::tests; From af1e1ae5a846c9f72ba539ede9b955d138173b39 Mon Sep 17 00:00:00 2001 From: Markus Pfeiffer Date: Thu, 4 Sep 2025 16:32:37 +0100 Subject: [PATCH 27/28] Re-constify --- arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp | 2 +- arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp index dc72071371cf..e342de1adb1a 100644 --- a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp +++ b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.cpp @@ -470,7 +470,7 @@ void WeightedTwoSidedEnumerator bool WeightedTwoSidedEnumerator::isDone() { + PathValidator>::isDone() const { if (!_candidatesStore.isEmpty()) { return false; } diff --git a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h index 0519547f0a9e..c27bb4316835 100644 --- a/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h +++ b/arangod/Graph/Enumerators/WeightedTwoSidedEnumerator.h @@ -347,7 +347,7 @@ class WeightedTwoSidedEnumerator { * @return true There will be no further path. * @return false There is a chance that there is more data available. */ - [[nodiscard]] bool isDone(); + [[nodiscard]] bool isDone() const; /** * @brief Reset to new source and target vertices. From b3bb9748b170b6a34efff0503a66a1aa160ee596 Mon Sep 17 00:00:00 2001 From: Markus Pfeiffer Date: Fri, 5 Sep 2025 15:18:31 +0100 Subject: [PATCH 28/28] Guard against a query finishing before it is observed as running --- ...h-traversal-generic-bidirectional-tests.js | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js index 098465351e3a..b0869e38dea4 100644 --- a/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js +++ b/js/client/modules/@arangodb/testutils/aql-graph-traversal-generic-bidirectional-tests.js @@ -38,11 +38,32 @@ const localHelper = { // Remove extra whitespace, newlines and normalize spaces return query.replace(/\s+/g, ' ').trim(); }, - checkRunningQuery: function(queryString, maxAttempts = 10, debug = false) { + getDoneJobs: function() { + const response = arango.GET_RAW('/_api/job/done'); + if(response.code == 200) { + this.debugPrint(`donejerbs ${JSON.stringify(response.parsedBody)}`); + return response.parsedBody; + } else { + assertFalse(true, `getDoneJobs failed. Response: ${response}`); + } + }, + isJobDone: function(jobId) { + const dj = this.getDoneJobs(); + return dj.some(x => x == jobId); + }, + checkRunningQuery: function(queryString, maxAttempts = 10, debug = false, queryJobId = undefined) { const normalizedQueryString = this.normalizeQueryString(queryString); if (debug) { this.debugPrint("Checking for running query:", normalizedQueryString); } + + if(this.isJobDone(queryJobId)) { + if (debug) { + this.debugPrint(`Query job done already ${queryJobId}`); + } + return "done"; + } + for (let i = 0; i < maxAttempts; i++) { const runningQueries = this.getRunningQueries(); if (debug) { @@ -135,9 +156,12 @@ const localHelper = { return response.headers['x-arango-async-id']; }, + getJobStatus: function(jobId) { + return arango.GET_RAW(`/_api/job/${jobId}`); + }, checkJobStatus: function(jobId, maxAttempts = 10) { for (let i = 0; i < maxAttempts; i++) { - const response = arango.GET_RAW('/_api/job/' + jobId); + const response = this.getJobStatus(jobId); if (response.code === 200) { return response; } @@ -155,7 +179,10 @@ const localHelper = { if (debug) { this.debugPrint("First query job ID:", queryJobId); } - const runningQuery = this.checkRunningQuery(queryString, 10, debug); + const runningQuery = this.checkRunningQuery(queryString, 10, debug, queryJobId); + if(runningQuery == "done") { + assertTrue(false, "query has already finished."); + } const queryId = runningQuery.id; if (debug) { this.debugPrint("First query ID:", queryId); @@ -177,7 +204,10 @@ const localHelper = { if (debug) { this.debugPrint("Second query job ID:", queryJobId2); } - const runningQuery2 = this.checkRunningQuery(queryString, 10, debug); + const runningQuery2 = this.checkRunningQuery(queryString, 10, debug, queryJobId2); + if(runningQuery === "done") { + assertTrue(false, "query has already finished."); + } const queryId2 = runningQuery2.id; if (debug) { this.debugPrint("Second query ID:", queryId2); @@ -381,4 +411,4 @@ exports.testHugeCompleteGraphKPathsLongRunning = testHugeCompleteGraphKPathsLong // Shortest Path, All Shortest Paths exports.testHugeGridGraphShortestPathLongRunning = testHugeGridGraphShortestPathLongRunning; -exports.testHugeGridGraphAllShortestPathsLongRunning = testHugeGridGraphAllShortestPathsLongRunning; \ No newline at end of file +exports.testHugeGridGraphAllShortestPathsLongRunning = testHugeGridGraphAllShortestPathsLongRunning;