From b3224eed1d55ad320a8335a5f66998036a304831 Mon Sep 17 00:00:00 2001 From: Markus Pfeiffer Date: Fri, 12 Apr 2019 09:39:01 +0100 Subject: [PATCH] Add isQueryKilled callback to ShortestPathOptions and use it (#8739) * Add isQueryKilled callback to ShortestPathOptions * Use isQueryKilledCallback in * ConstantWeightShortestPathFinder * AttributeWeightShortestPathFinder * ShortestPathExecutor --- arangod/Aql/ShortestPathExecutor.cpp | 6 +----- arangod/Graph/AttributeWeightShortestPathFinder.cpp | 8 ++++---- arangod/Graph/AttributeWeightShortestPathFinder.h | 3 +-- arangod/Graph/ConstantWeightShortestPathFinder.cpp | 4 ++-- arangod/Graph/ConstantWeightShortestPathFinder.h | 3 +-- arangod/Graph/ShortestPathFinder.h | 3 +-- arangod/Graph/ShortestPathOptions.cpp | 9 ++++++++- arangod/Graph/ShortestPathOptions.h | 2 ++ tests/Aql/ShortestPathExecutorTest.cpp | 3 +-- tests/Graph/ConstantWeightShortestPathFinder.cpp | 12 ++++++------ 10 files changed, 27 insertions(+), 26 deletions(-) diff --git a/arangod/Aql/ShortestPathExecutor.cpp b/arangod/Aql/ShortestPathExecutor.cpp index 7e15b4b56d..875c5621b3 100644 --- a/arangod/Aql/ShortestPathExecutor.cpp +++ b/arangod/Aql/ShortestPathExecutor.cpp @@ -205,11 +205,7 @@ bool ShortestPathExecutor::fetchPath() { TRI_ASSERT(start.isString()); TRI_ASSERT(end.isString()); _path->clear(); - } while (!_finder.shortestPath(start, end, *_path, [this]() { - if (_finder.options().query()->killed()) { - THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); - } - })); + } while (!_finder.shortestPath(start, end, *_path)); _posInPath = 0; return true; } diff --git a/arangod/Graph/AttributeWeightShortestPathFinder.cpp b/arangod/Graph/AttributeWeightShortestPathFinder.cpp index 8baa9019b5..4f6ee0d0cc 100644 --- a/arangod/Graph/AttributeWeightShortestPathFinder.cpp +++ b/arangod/Graph/AttributeWeightShortestPathFinder.cpp @@ -160,9 +160,9 @@ AttributeWeightShortestPathFinder::AttributeWeightShortestPathFinder(ShortestPat AttributeWeightShortestPathFinder::~AttributeWeightShortestPathFinder() {} -bool AttributeWeightShortestPathFinder::shortestPath( - arangodb::velocypack::Slice const& st, arangodb::velocypack::Slice const& ta, - ShortestPathResult& result, std::function const& callback) { +bool AttributeWeightShortestPathFinder::shortestPath(arangodb::velocypack::Slice const& st, + arangodb::velocypack::Slice const& ta, + ShortestPathResult& result) { // For the result: result.clear(); _highscoreSet = false; @@ -205,7 +205,7 @@ bool AttributeWeightShortestPathFinder::shortestPath( if (++counter == 10) { // check for abortion - callback(); + options().isQueryKilledCallback(); counter = 0; } } diff --git a/arangod/Graph/AttributeWeightShortestPathFinder.h b/arangod/Graph/AttributeWeightShortestPathFinder.h index ca72c0602d..5bc1070d23 100644 --- a/arangod/Graph/AttributeWeightShortestPathFinder.h +++ b/arangod/Graph/AttributeWeightShortestPathFinder.h @@ -208,8 +208,7 @@ class AttributeWeightShortestPathFinder : public ShortestPathFinder { // path bool shortestPath(arangodb::velocypack::Slice const& start, arangodb::velocypack::Slice const& target, - arangodb::graph::ShortestPathResult& result, - std::function const& callback) override; + arangodb::graph::ShortestPathResult& result) override; void inserter(std::unordered_map& candidates, std::vector& result, arangodb::velocypack::StringRef const& s, diff --git a/arangod/Graph/ConstantWeightShortestPathFinder.cpp b/arangod/Graph/ConstantWeightShortestPathFinder.cpp index 38acae26a4..5efaa1ddbe 100644 --- a/arangod/Graph/ConstantWeightShortestPathFinder.cpp +++ b/arangod/Graph/ConstantWeightShortestPathFinder.cpp @@ -54,7 +54,7 @@ ConstantWeightShortestPathFinder::~ConstantWeightShortestPathFinder() { bool ConstantWeightShortestPathFinder::shortestPath( arangodb::velocypack::Slice const& s, arangodb::velocypack::Slice const& e, - arangodb::graph::ShortestPathResult& result, std::function const& callback) { + arangodb::graph::ShortestPathResult& result) { result.clear(); TRI_ASSERT(s.isString()); TRI_ASSERT(e.isString()); @@ -81,7 +81,7 @@ bool ConstantWeightShortestPathFinder::shortestPath( arangodb::velocypack::StringRef n; while (!_leftClosure.empty() && !_rightClosure.empty()) { - callback(); + options().isQueryKilledCallback(); if (_leftClosure.size() < _rightClosure.size()) { if (expandClosure(_leftClosure, _leftFound, _rightFound, false, n)) { diff --git a/arangod/Graph/ConstantWeightShortestPathFinder.h b/arangod/Graph/ConstantWeightShortestPathFinder.h index a7b1d1829d..15519696de 100644 --- a/arangod/Graph/ConstantWeightShortestPathFinder.h +++ b/arangod/Graph/ConstantWeightShortestPathFinder.h @@ -59,8 +59,7 @@ class ConstantWeightShortestPathFinder : public ShortestPathFinder { bool shortestPath(arangodb::velocypack::Slice const& start, arangodb::velocypack::Slice const& end, - arangodb::graph::ShortestPathResult& result, - std::function const& callback) override; + arangodb::graph::ShortestPathResult& result) override; private: void expandVertex(bool backward, arangodb::velocypack::StringRef vertex); diff --git a/arangod/Graph/ShortestPathFinder.h b/arangod/Graph/ShortestPathFinder.h index 603b1d7173..d0d44e3926 100644 --- a/arangod/Graph/ShortestPathFinder.h +++ b/arangod/Graph/ShortestPathFinder.h @@ -42,8 +42,7 @@ class ShortestPathFinder { virtual bool shortestPath(arangodb::velocypack::Slice const& start, arangodb::velocypack::Slice const& target, - arangodb::graph::ShortestPathResult& result, - std::function const& callback) = 0; + arangodb::graph::ShortestPathResult& result) = 0; void destroyEngines(); diff --git a/arangod/Graph/ShortestPathOptions.cpp b/arangod/Graph/ShortestPathOptions.cpp index 536c9c64f7..440fefa45d 100644 --- a/arangod/Graph/ShortestPathOptions.cpp +++ b/arangod/Graph/ShortestPathOptions.cpp @@ -203,7 +203,8 @@ EdgeCursor* ShortestPathOptions::nextReverseCursorCoordinator(arangodb::velocypa return cursor.release(); } -void ShortestPathOptions::fetchVerticesCoordinator(std::deque const& vertexIds) { +void ShortestPathOptions::fetchVerticesCoordinator( + std::deque const& vertexIds) { // TRI_ASSERT(arangodb::ServerState::instance()->isCoordinator()); if (!arangodb::ServerState::instance()->isCoordinator()) { return; @@ -229,3 +230,9 @@ void ShortestPathOptions::fetchVerticesCoordinator(std::dequedatalake(), *(leased.get())); } } + +void ShortestPathOptions::isQueryKilledCallback() const { + if (query()->killed()) { + THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); + } +} diff --git a/arangod/Graph/ShortestPathOptions.h b/arangod/Graph/ShortestPathOptions.h index 8b2f2ee999..7476cceaa2 100644 --- a/arangod/Graph/ShortestPathOptions.h +++ b/arangod/Graph/ShortestPathOptions.h @@ -97,6 +97,8 @@ struct ShortestPathOptions : public BaseOptions { void fetchVerticesCoordinator(std::deque const& vertexIds); + void isQueryKilledCallback() const; + private: EdgeCursor* nextCursorCoordinator(arangodb::velocypack::StringRef vid); EdgeCursor* nextReverseCursorCoordinator(arangodb::velocypack::StringRef vid); diff --git a/tests/Aql/ShortestPathExecutorTest.cpp b/tests/Aql/ShortestPathExecutorTest.cpp index cd22dc90b6..5512cbb8ac 100644 --- a/tests/Aql/ShortestPathExecutorTest.cpp +++ b/tests/Aql/ShortestPathExecutorTest.cpp @@ -121,8 +121,7 @@ class FakePathFinder : public ShortestPathFinder { } bool shortestPath(VPackSlice const& source, VPackSlice const& target, - arangodb::graph::ShortestPathResult& result, - std::function const& callback) override { + arangodb::graph::ShortestPathResult& result) override { REQUIRE(source.isString()); REQUIRE(target.isString()); _calledWith.emplace_back(std::make_pair(source.copyString(), target.copyString())); diff --git a/tests/Graph/ConstantWeightShortestPathFinder.cpp b/tests/Graph/ConstantWeightShortestPathFinder.cpp index b1d005ab20..abf825aaec 100644 --- a/tests/Graph/ConstantWeightShortestPathFinder.cpp +++ b/tests/Graph/ConstantWeightShortestPathFinder.cpp @@ -350,7 +350,7 @@ TEST_CASE("ConstantWeightShortestPathFinder", "[graph]") { auto end = velocypack::Parser::fromJson("\"v/0\""); ShortestPathResult result; - REQUIRE(true == finder->shortestPath(start->slice(), end->slice(), result, []() {})); + REQUIRE(true == finder->shortestPath(start->slice(), end->slice(), result)); } SECTION("no path exists") { @@ -358,7 +358,7 @@ TEST_CASE("ConstantWeightShortestPathFinder", "[graph]") { auto end = velocypack::Parser::fromJson("\"v/1\""); ShortestPathResult result; - REQUIRE(false == finder->shortestPath(start->slice(), end->slice(), result, []() {})); + REQUIRE(false == finder->shortestPath(start->slice(), end->slice(), result)); CHECK(result.length() == 0); } @@ -367,7 +367,7 @@ TEST_CASE("ConstantWeightShortestPathFinder", "[graph]") { auto end = velocypack::Parser::fromJson("\"v/2\""); ShortestPathResult result; - auto rr = finder->shortestPath(start->slice(), end->slice(), result, []() {}); + auto rr = finder->shortestPath(start->slice(), end->slice(), result); REQUIRE(rr); REQUIRE(checkPath(result, {"1", "2"}, {{}, {"v/1", "v/2"}})); } @@ -377,7 +377,7 @@ TEST_CASE("ConstantWeightShortestPathFinder", "[graph]") { auto end = velocypack::Parser::fromJson("\"v/4\""); ShortestPathResult result; - auto rr = finder->shortestPath(start->slice(), end->slice(), result, []() {}); + auto rr = finder->shortestPath(start->slice(), end->slice(), result); REQUIRE(rr); REQUIRE(checkPath(result, {"1", "2", "3", "4"}, {{}, {"v/1", "v/2"}, {"v/2", "v/3"}, {"v/3", "v/4"}})); @@ -390,7 +390,7 @@ TEST_CASE("ConstantWeightShortestPathFinder", "[graph]") { ShortestPathResult result; { - auto rr = finder->shortestPath(start->slice(), end->slice(), result, []() {}); + auto rr = finder->shortestPath(start->slice(), end->slice(), result); REQUIRE(rr); // One of the two has to be returned @@ -410,7 +410,7 @@ TEST_CASE("ConstantWeightShortestPathFinder", "[graph]") { } { - auto rr = finder->shortestPath(end->slice(), start->slice(), result, []() {}); + auto rr = finder->shortestPath(end->slice(), start->slice(), result); REQUIRE(!rr); }