8000 KShortestPathsExecutor must reset its KShortestPathFinder, including … · arangodb/arangodb@552a82a · GitHub
[go: up one dir, main page]

Skip to content

Commit 552a82a

Browse files
authored
KShortestPathsExecutor must reset its KShortestPathFinder, including all caches. (#11312)
* Reset KShortestPathsExecutor * Rename reset method to clear to be in line with Traversal * clear caches in shortest paths executor
1 parent 174b5d0 commit 552a82a

11 files changed

+89
-44
lines changed

arangod/Aql/KShortestPathsExecutor.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,10 @@ KShortestPathsExecutor::KShortestPathsExecutor(Fetcher& fetcher, Infos& infos)
146146
if (!_infos.useRegisterForTargetInput()) {
147147
_targetBuilder.add(VPackValue(_infos.getTargetInputValue()));
148148
}
149+
// Make sure the finder is in a defined state, because we could
150+
// get any old junk here, because infos are not recreated in between
151+
// initializeCursor calls.
152+
_finder.clear();
149153
}
150154

151155
// Shutdown query
@@ -211,6 +215,7 @@ auto KShortestPathsExecutor::skipRowsRange(AqlItemBlockInputRange& input, AqlCal
211215

212216
auto KShortestPathsExecutor::fetchPaths(AqlItemBlockInputRange& input) -> bool {
213217
TRI_ASSERT(_finder.isDone());
218+
_finder.clear();
214219
while (input.hasDataRow()) {
215220
auto source = VPackSlice{};
216221
auto target = VPackSlice{};

arangod/Aql/ShortestPathExecutor.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,9 @@ ShortestPathExecutor::ShortestPathExecutor(Fetcher&, Infos& infos)
156156
if (!_infos.useRegisterForTargetInput()) {
157157
_targetBuilder.add(VPackValue(_infos.getTargetInputValue()));
158158
}
159+
// Make sure the finder does not contain any leftovers in case of
160+
// the executor being reconstructed.
161+
_finder.clear();
159162
}
160163

161164
// Shutdown query
@@ -209,6 +212,7 @@ auto ShortestPathExecutor::doSkipPath(AqlCall& call) -> size_t {
209212
auto ShortestPathExecutor::fetchPath(AqlItemBlockInputRange& input) -> bool {
210213
// We only want to call fetchPath if we don't have a path currently available
211214
TRI_ASSERT(pathLengthAvailable() == 0);
215+
_finder.clear();
212216
_path->clear();
213217
_posInPath = 0;
214218

arangod/Graph/AttributeWeightShortestPathFinder.cpp

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,16 @@ AttributeWeightShortestPathFinder::AttributeWeightShortestPathFinder(ShortestPat
166166

167167
AttributeWeightShortestPathFinder::~AttributeWeightShortestPathFinder() = default;
168168

169+
void AttributeWeightShortestPathFinder::clear() {
170+
options().cache()->clear();
171+
_highscoreSet = false;
172+
_highscore = 0;
173+
_bingo = false;
174+
_resultCode = TRI_ERROR_NO_ERROR;
175+
_intermediateSet = false;
176+
_intermediate = arangodb::velocypack::StringRef{};
177+
}
178+
169179
bool AttributeWeightShortestPathFinder::shortestPath(arangodb::velocypack::Slice const& st,
170180
arangodb::velocypack::Slice const& ta,
171181
ShortestPathResult& result) {
@@ -275,18 +285,17 @@ bool AttributeWeightShortestPathFinder::shortestPath(arangodb::velocypack::Slice
275285
}
276286

277287
void AttributeWeightShortestPathFinder::inserter(
278-
std::unordered_map<arangodb::velocypack::StringRef, size_t>& candidates,
279-
std::vector<std::unique_ptr<Step>>& result,
280-
arangodb::velocypack::StringRef const& s, arangodb::velocypack::StringRef const& t,
281-
double currentWeight, EdgeDocumentToken&& edge) {
282-
283 9E88 -
auto [cand, emplaced] = candidates.try_emplace(
284-
t,
285-
arangodb::lazyConstruct([&]{
286-
result.emplace_back(std::make_unique<Step>(t, s, currentWeight, std::move(edge)));
287-
return result.size() - 1;
288-
})
289-
);
288+
std::unordered_map<arangodb::velocypack::StringRef, size_t>& candidates,
289+
std::vector<std::unique_ptr<Step>>& result,
290+
arangodb::velocypack::StringRef const& s, arangodb::velocypack::StringRef const& t,
291+
double currentWeight, EdgeDocumentToken&& edge) {
292+
auto [cand, emplaced] =
293+
candidates.try_emplace(t, arangodb::lazyConstruct([&] {
294+
result.emplace_back(
295+
std::make_unique<Step>(t, s, currentWeight,
296+
std::move(edge)));
297+
return result.size() - 1;
298+
}));
290299

291300
if (!emplaced) {
292301
// Compare weight

arangod/Graph/AttributeWeightShortestPathFinder.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,8 @@ class AttributeWeightShortestPathFinder : public ShortestPathFinder {
200200

201201
~AttributeWeightShortestPathFinder();
202202

203+
void clear() override;
204+
203205
//////////////////////////////////////////////////////////////////////////////
204206
/// @brief Find the shortest path between start and target.
205207
/// Only edges having the given direction are followed.

arangod/Graph/ConstantWeightShortestPathFinder.cpp

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ ConstantWeightShortestPathFinder::~ConstantWeightShortestPathFinder() {
5656
clearVisited();
5757
}
5858

59+
void ConstantWeightShortestPathFinder::clear() {
60+
clearVisited();
61+
options().cache()->clear();
62+
}
63+
5964
bool ConstantWeightShortestPathFinder::shortestPath(
6065
arangodb::velocypack::Slice const& s, arangodb::velocypack::Slice const& e,
6166
arangodb::graph::ShortestPathResult& result) {
@@ -102,10 +107,9 @@ bool ConstantWeightShortestPathFinder::shortestPath(
102107
return false;
103108
}
104109

105-
bool ConstantWeightShortestPathFinder::expandClosure(Closure& sourceClosure,
106-
Snippets& sourceSnippets,
107-
Snippets& targetSnippets,
108-
bool isBackward, arangodb::velocypack::StringRef& result) {
110+
bool ConstantWeightShortestPathFinder::expandClosure(
111+
Closure& sourceClosure, Snippets& sourceSnippets, Snippets& targetSnippets,
112+
bool isBackward, arangodb::velocypack::StringRef& result) {
109113
_nextClosure.clear();
110114
for (auto& v : sourceClosure) {
111115
_edges.clear();
@@ -118,12 +122,10 @@ bool ConstantWeightShortestPathFinder::expandClosure(Closure& sourceClosure,
118122
auto const& n = _neighbors[i];
119123

120124
bool emplaced = false;
121-
std::tie(std::ignore, emplaced) = sourceSnippets.try_emplace(
122-
_neighbors[i],
123-
arangodb::lazyConstruct([&]{
124-
return new PathSnippet(v, std::move(_edges[i]));
125-
})
126-
);
125+
std::tie(std::ignore, emplaced) =
126+
sourceSnippets.try_emplace(_neighbors[i], arangodb::lazyConstruct([&] {
127+
return new PathSnippet(v, std::move(_edges[i]));
128+
}));
127129

128130
if (emplaced) {
129131
// NOTE: _edges[i] stays intact after move
@@ -174,21 +176,25 @@ void ConstantWeightShortestPathFinder::fillResult(arangodb::velocypack::StringRe
174176
clearVisited();
175177
}
176178

177-
void ConstantWeightShortestPathFinder::expandVertex(bool backward, arangodb::velocypack::StringRef vertex) {
179+
void ConstantWeightShortestPathFinder::expandVertex(bool backward,
180+
arangodb::velocypack::StringRef vertex) {
178181
EdgeCursor* cursor = backward ? _backwardCursor.get() : _forwardCursor.get();
179182
cursor->rearm(vertex, 0);
180183

181184
cursor->readAll([&](EdgeDocumentToken&& eid, VPackSlice edge, size_t cursorIdx) -> void {
182185
if (edge.isString()) {
183186
if (edge.compareString(vertex.data(), vertex.length()) != 0) {
184-
arangodb::velocypack::StringRef id = _options.cache()->persistString(arangodb::velocypack::StringRef(edge));
187+
arangodb::velocypack::StringRef id =
188+
_options.cache()->persistString(arangodb::velocypack::StringRef(edge));
185189
_edges.emplace_back(std::move(eid));
186190
_neighbors.emplace_back(id);
187191
}
188192
} else {
189-
arangodb::velocypack::StringRef other(transaction::helpers::extractFromFromDocument(edge));
193+
arangodb::velocypack::StringRef other(
194+
transaction::helpers::extractFromFromDocument(edge));
190195
if (other == vertex) {
191-
other = arangodb::velocypack::StringRef(transaction::helpers::extractToFromDocument(edge));
196+
other = arangodb::velocypack::StringRef(
197+
transaction::helpers::extractToFromDocument(edge));
192198
}
193199
if (other != vertex) {
194200
arangodb::velocypack::StringRef id = _options.cache()->persistString(other);

arangod/Graph/ConstantWeightShortestPathFinder.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,18 @@ class ConstantWeightShortestPathFinder : public ShortestPathFinder {
6363
arangodb::velocypack::Slice const& end,
6464
arangodb::graph::ShortestPathResult& result) override;
6565

66+
void clear() override;
67+
6668
private:
6769
void expandVertex(bool backward, arangodb::velocypack::StringRef vertex);
6870

6971
void clearVisited();
7072

71-
bool expandClosure(Closure& sourceClosure, Snippets& sourceSnippets,
72-
Snippets& targetSnippets, bool direction, arangodb::velocypack::StringRef& result);
73+
bool expandClosure(Closure& sourceClosure, Snippets& sourceSnippets, Snippets& targetSnippets,
74+
bool direction, arangodb::velocypack::StringRef& result);
7375

74-
void fillResult(arangodb::velocypack::StringRef& n, arangodb::graph::ShortestPathResult& result);
76+
void fillResult(arangodb::velocypack::StringRef& n,
77+
arangodb::graph::ShortestPathResult& result);
7578

7679
private:
7780
Snippets _leftFound;

arangod/Graph/KShortestPathsFinder.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,13 @@ KShortestPathsFinder::KShortestPathsFinder(ShortestPathOptions& options)
5050

5151
KShortestPathsFinder::~KShortestPathsFinder() = default;
5252

53+
void KShortestPathsFinder::clear() {
54+
_shortestPaths.clear();
55+
_candidatePaths.clear();
56+
_vertexCache.clear();
57+
_traversalDone = true;
58+
}
59+
5360
// Sets up k-shortest-paths traversal from start to end
5461
bool KShortestPathsFinder::startKShortestPathsTraversal(
5562
arangodb::velocypack::Slice const& start, arangodb::velocypack::Slice const& end) {
@@ -58,11 +65,12 @@ bool KShortestPathsFinder::startKShortestPathsTraversal(
5865
_start = arangodb::velocypack::StringRef(start);
5966
_end = arangodb::velocypack::StringRef(end);
6067

61-
_traversalDone = false;
62-
68+
_vertexCache.clear();
6369
_shortestPaths.clear();
6470
_candidatePaths.clear();
6571

72+
_traversalDone = false;
73+
6674
TRI_IF_FAILURE("Travefalse") { THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG); }
6775

6876
return true;

arangod/Graph/KShortestPathsFinder.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,11 @@ class KShortestPathsFinder : public ShortestPathFinder {
211211
explicit KShortestPathsFinder(ShortestPathOptions& options);
212212
~KShortestPathsFinder();
213213

214+
// reset the traverser; this is mainly needed because the traverser is
215+
// part of the KShortestPathsExecutorInfos, and hence not recreated when
216+
// a cursor is initialised.
217+
void clear() override;
218+
214219
// This is here because we inherit from ShortestPathFinder (to get the destroyEngines function)
215220
// TODO: Remove
216221
bool shortestPath(arangodb::velocypack::Slice const& start,

arangod/Graph/ShortestPathFinder.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ using namespace arangodb;
3939
using namespace arangodb::graph;
4040

4141
ShortestPathFinder::ShortestPathFinder(ShortestPathOptions& options)
42-
: _options(options),
43-
_httpRequests(0) {}
42+
: _options(options), _httpRequests(0) {}
4443

4544
void ShortestPathFinder::destroyEngines() {
4645
if (!ServerState::instance()->isCoordinator()) {
@@ -54,22 +53,22 @@ void ShortestPathFinder::destroyEngines() {
5453
return;
5554
}
5655
// nullptr only happens on controlled server shutdown
57-
56+
5857
auto ch = reinterpret_cast<ClusterTraverserCache*>(_options.cache());
59-
58+
6059
network::RequestOptions options;
6160
options.database = _options.trx()->vocbase().name();
6261
options.timeout = network::Timeout(30.0);
63-
options.skipScheduler = true; // hack to speed up future.get()
62+
options.skipScheduler = true; // hack to speed up future.get()
6463

6564
for (auto const& it : *ch->engines()) {
6665
incHttpRequests(1);
6766

68-
auto res =
69-
network::sendRequest(pool, "server:" + it.first, fuerte::RestVerb::Delete,
70-
"/_internal/traverser/" + arangodb::basics::StringUtils::itoa(it.second),
71-
VPackBuffer<uint8_t>(), options)
72-
.get();
67+
auto res = network::sendRequest(pool, "server:" + it.first, fuerte::RestVerb::Delete,
68+
"/_internal/traverser/" +
69+
arangodb::basics::StringUtils::itoa(it.second),
70+
VPackBuffer<uint8_t>(), options)
71+
.get();
7372

7473
if (res.error != fuerte::Error::NoError) {
7574
// Note If there was an error on server side we do not have

arangod/Graph/ShortestPathFinder.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,23 @@ class ShortestPathFinder {
4848

4949
ShortestPathOptions& options() const;
5050

51+
virtual void clear() = 0;
52+
5153
/// @brief return number of HTTP requests made, and reset it to 0
52-
size_t getAndResetHttpRequests() {
54+
size_t getAndResetHttpRequests() {
5355
size_t value = _httpRequests;
5456
_httpRequests = 0;
5557
return value;
5658
}
57-
59+
5860
void incHttpRequests(size_t requests) { _httpRequests += requests; }
5961

6062
protected:
6163
//////////////////////////////////////////////////////////////////////////////
6264
/// @brief The options to modify this shortest path computation
6365
//////////////////////////////////////////////////////////////////////////////
6466
ShortestPathOptions& _options;
65-
67+
6668
//////////////////////////////////////////////////////////////////////////////
6769
/// @brief Number of HTTP requests made
6870
//////////////////////////////////////////////////////////////////////////////

tests/Aql/ShortestPathExecutorTest.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ class FakePathFinder : public ShortestPathFinder {
130130

131131
~FakePathFinder() = default;
132132

133+
void clear() override{};
134+
133135
void addPath(std::vector<std::string>&& path) {
134136
_paths.emplace_back(std::move(path));
135137
}

0 commit comments

Comments
 (0)
0