Hawkin
/
arangodb
mirror of https://gitee.com/bigwinds/arangodb
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Bug fix 3.5/k shortest paths (#10390) 

* Fix a bug in KShortestPathFinder

The finder was too eager to return a "shortest" path when there was actually
potentially a shorter path still to be discovered

* A little bit of cleanup

* Add a test that reproduces k Shortest Paths bug

* Add hacked up variant of optional

* Fixup tests

* Update CHANGELOG
This commit is contained in:
Markus Pfeiffer 2019-11-11 17:10:04 +00:00 committed by KVS85
parent a775e954c9
commit 13df0d23de
5 changed files with 117 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGELOG
View File

@ -1,6 +1,9 @@
v3.5.3 (XXXX-XX-XX)
-------------------
* Fixed issue #10371: K_SHORTEST_PATHS LIMIT 1 can not return the shortest path.
Now the shortest path is returned as the first one in such queries.
* Improve killability of some types of cluster AQL queries. Previously, several
cluster queries, especially those containing a `DistributeNode` in their
execution plans, did not respond to a kill instruction.

39
arangod/Graph/KShortestPathsFinder.cpp
View File

@ -70,27 +70,36 @@ bool KShortestPathsFinder::computeShortestPath(VertexRef const& start, VertexRef
std::unordered_set<VertexRef> const& forbiddenVertices,
std::unordered_set<Edge> const& forbiddenEdges,
Path& result) {
bool found = false;
Ball left(start, FORWARD);
Ball right(end, BACKWARD);
VertexRef join;
result.clear();
while (!left._frontier.empty() && !right._frontier.empty() && !found) {
auto currentBest = boost::optional<double>{};
// We will not improve anymore if we have found a best path and the smallest
// combined distance between left and right is bigger than that path
while (!left._frontier.empty() && !right._frontier.empty() &&
!(currentBest.has_value() &&
(left._closest + right._closest > currentBest.value()))) {
_options.isQueryKilledCallback();
// Choose the smaller frontier to expand.
if (left._frontier.size() < right._frontier.size()) {
found = advanceFrontier(left, right, forbiddenVertices, forbiddenEdges, join);
advanceFrontier(left, right, forbiddenVertices, forbiddenEdges, join, currentBest);
} else {
found = advanceFrontier(right, left, forbiddenVertices, forbiddenEdges, join);
advanceFrontier(right, left, forbiddenVertices, forbiddenEdges, join, currentBest);
}
}
if (found) {
if (currentBest.has_value()) {
reconstructPath(left, right, join, result);
return true;
} else {
// No path found
return false;
}
return found;
}
void KShortestPathsFinder::computeNeighbourhoodOfVertexCache(VertexRef vertex,
@ -178,10 +187,11 @@ void KShortestPathsFinder::computeNeighbourhoodOfVertex(VertexRef vertex, Direct
}
}
bool KShortestPathsFinder::advanceFrontier(Ball& source, Ball const& target,
void KShortestPathsFinder::advanceFrontier(Ball& source, Ball const& target,
std::unordered_set<VertexRef> const& forbiddenVertices,
std::unordered_set<Edge> const& forbiddenEdges,
VertexRef& join) {
VertexRef& join,
boost::optional<double>& currentBest) {
VertexRef vr;
DijkstraInfo *v, *w;
std::vector<Step>* neighbours;
@ -190,7 +200,7 @@ bool KShortestPathsFinder::advanceFrontier(Ball& source, Ball const& target,
TRI_ASSERT(v != nullptr);
TRI_ASSERT(vr == v->_vertex);
if (!success) {
return false;
return;
}
computeNeighbourhoodOfVertexCache(vr, source._direction, neighbours);
@ -217,14 +227,17 @@ bool KShortestPathsFinder::advanceFrontier(Ball& source, Ball const& target,
}
}
v->_done = true;
source._closest = v->_weight;
w = target._frontier.find(v->_vertex);
if (w != nullptr && w->_done) {
join = v->_vertex;
return true;
// The total weight of the found path
double totalWeight = v->_weight + w->_weight;
if (!currentBest.has_value() || totalWeight < currentBest.value()) {
join = v->_vertex;
currentBest = v->_weight + w->_weight;
}
}
return false;
}
void KShortestPathsFinder::reconstructPath(Ball const& left, Ball const& right,

18
arangod/Graph/KShortestPathsFinder.h
View File

@ -34,6 +34,8 @@
#include <list>
#include <boost/optional.hpp>
namespace arangodb {
namespace velocypack {
@ -141,9 +143,9 @@ class KShortestPathsFinder : public ShortestPathFinder {
void setWeight(double weight) { _weight = weight; }
DijkstraInfo(VertexRef const& vertex, Edge const&& edge, VertexRef const& pred, double weight)
: _vertex(vertex), _edge(std::move(edge)), _pred(pred), _weight(weight), _done(false) {}
: _vertex(vertex), _edge(std::move(edge)), _pred(pred), _weight(weight), _done(false) {}
explicit DijkstraInfo(VertexRef const& vertex)
: _vertex(vertex), _weight(0), _done(true) {}
: _vertex(vertex), _weight(0), _done(true) {}
};
typedef ShortestPathPriorityQueue<VertexRef, DijkstraInfo, double> Frontier;
@ -154,10 +156,13 @@ class KShortestPathsFinder : public ShortestPathFinder {
VertexRef _centre;
Direction _direction;
Frontier _frontier;
// The distance of the last node that has been fully expanded
// from _centre
double _closest;
Ball() {}
Ball(VertexRef const& centre, Direction direction)
: _centre(centre), _direction(direction) {
: _centre(centre), _direction(direction), _closest(0) {
_frontier.insert(centre, std::make_unique<DijkstraInfo>(centre));
}
~Ball() {}
@ -193,7 +198,7 @@ class KShortestPathsFinder : public ShortestPathFinder {
std::vector<size_t> _paths;
explicit FoundVertex(VertexRef const& vertex)
: _vertex(vertex), _hasCachedOutNeighbours(false), _hasCachedInNeighbours(false) {}
: _vertex(vertex), _hasCachedOutNeighbours(false), _hasCachedInNeighbours(false) {}
};
// Contains the vertices that were found while searching
// for a shortest path between start and end together with
@ -246,9 +251,10 @@ class KShortestPathsFinder : public ShortestPathFinder {
void computeNeighbourhoodOfVertex(VertexRef vertex, Direction direction,
std::vector<Step>& steps);
bool advanceFrontier(Ball& source, Ball const& target,
void advanceFrontier(Ball& source, Ball const& target,
std::unordered_set<VertexRef> const& forbiddenVertices,
std::unordered_set<Edge> const& forbiddenEdges, VertexRef& join);
std::unordered_set<Edge> const& forbiddenEdges,
VertexRef& join, boost::optional<double>& currentBest);
private:
bool _pathAvailable;

40
tests/Graph/GraphTestTools.h
View File

@ -50,6 +50,8 @@
#include "../Mocks/StorageEngineMock.h"
#include "IResearch/common.h"
#include "boost/optional.hpp"
using namespace arangodb;
using namespace arangodb::aql;
using namespace arangodb::graph;
@ -184,9 +186,18 @@ struct MockGraphDatabase {
EXPECT_TRUE(vertices->type());
}
struct EdgeDef {
EdgeDef(size_t from, size_t to) : _from(from), _to(to){};
EdgeDef(size_t from, size_t to, double weight)
: _from(from), _to(to), _weight(weight){};
size_t _from;
size_t _to;
boost::optional<double> _weight;
};
// Create a collection with name <name> of edges given by <edges>
void addEdgeCollection(std::string name, std::string vertexCollection,
std::vector<std::pair<size_t, size_t>> edgedef) {
std::vector<EdgeDef> edgedef) {
std::shared_ptr<arangodb::LogicalCollection> edges;
auto createJson = velocypack::Parser::fromJson("{ \"name\": \"" + name +
"\", \"type\": 3 }");
@ -205,10 +216,18 @@ struct MockGraphDatabase {
for (auto& p : edgedef) {
// std::cout << "edge: " << vertexCollection << " " << p.first << " -> "
// << p.second << std::endl;
auto docJson = velocypack::Parser::fromJson(
"{ \"_from\": \"" + vertexCollection + "/" + std::to_string(p.first) +
"\"" + ", \"_to\": \"" + vertexCollection + "/" +
std::to_string(p.second) + "\" }");
// This is moderately horrible
auto docJson =
p._weight.has_value()
? velocypack::Parser::fromJson(
"{ \"_from\": \"" + vertexCollection + "/" +
std::to_string(p._from) + "\"" + ", \"_to\": \"" +
vertexCollection + "/" + std::to_string(p._to) +
"\", \"cost\": " + std::to_string(p._weight.value()) + "}")
: velocypack::Parser::fromJson(
"{ \"_from\": \"" + vertexCollection + "/" +
std::to_string(p._from) + "\"" + ", \"_to\": \"" +
vertexCollection + "/" + std::to_string(p._to) + "\" }");
docs.emplace_back(docJson);
}
@ -282,12 +301,13 @@ struct MockGraphDatabase {
spos.emplace_back(spo);
return spo;
}
};
}; // namespace graph
bool checkPath(ShortestPathOptions* spo, ShortestPathResult result, std::vector<std::string> vertices,
bool checkPath(ShortestPathOptions* spo, ShortestPathResult result,
std::vector<std::string> vertices,
std::vector<std::pair<std::string, std::string>> edges, std::string& msgs);
}
}
}
} // namespace graph
} // namespace tests
} // namespace arangodb
#endif

46
tests/Graph/KShortestPathsFinder.cpp
View File

@ -212,6 +212,52 @@ TEST_F(KShortestPathsFinderTest, many_edges_between_two_nodes) {
ASSERT_TRUE(false == finder->getNextPathShortestPathResult(result));
}
class KShortestPathsFinderTestWeights : public ::testing::Test {
protected:
GraphTestSetup s;
MockGraphDatabase gdb;
arangodb::aql::Query* query;
arangodb::graph::ShortestPathOptions* spo;
KShortestPathsFinder* finder;
KShortestPathsFinderTestWeights() : gdb("testVocbase") {
gdb.addVertexCollection("v", 10);
gdb.addEdgeCollection("e", "v",
{{1, 2, 10},
{1, 3, 10},
{1, 10, 100},
{2, 4, 10},
{3, 4, 20},
{7, 3, 10},
{8, 3, 10},
{9, 3, 10}});
query = gdb.getQuery("RETURN 1");
spo = gdb.getShortestPathOptions(query);
spo->weightAttribute = "cost";
finder = new KShortestPathsFinder(*spo);
}
~KShortestPathsFinderTestWeights() { delete finder; }
};
TEST_F(KShortestPathsFinderTestWeights, diamond_path) {
auto start = velocypack::Parser::fromJson("\"v/1\"");
auto end = velocypack::Parser::fromJson("\"v/4\"");
ShortestPathResult result;
std::string msgs;
finder->startKShortestPathsTraversal(start->slice(), end->slice());
ASSERT_TRUE(finder->getNextPathShortestPathResult(result));
auto cpr = checkPath(spo, result, {"1", "2", "4"},
{{}, {"v/1", "v/2"}, {"v/2", "v/4"}}, msgs);
ASSERT_TRUE(cpr) << msgs;
}
} // namespace graph
} // namespace tests
} // namespace arangodb