Skip to content

Commit

Permalink
fix go n-hop query hanging, and graphd takes up too much memory util …
Browse files Browse the repository at this point in the history 
…OOM (vesoft-inc#2386)
  • Loading branch information
@liang-Q8797
liang-Q8797 authored Nov 9, 2020
1 parent 92745cd commit 65a9cc3
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 14 deletions.
21 changes: 13 additions & 8 deletions src/graph/GoExecutor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ std::vector<VertexID> GoExecutor::getDstIdsFromRespWithBackTrack(const RpcRespon
// 7 , 6
// Will mistake lead to 7->6 if insert edge(dst, src) one by one
// So read all roots of current step first , then insert them
std::multimap<VertexID, VertexID> backTrace;
std::unordered_set<std::pair<VertexID, VertexID>, HashPair> curBackTrace;
std::unordered_set<VertexID> set;
for (const auto &resp : rpcResp.responses()) {
if (!resp.__isset.vertices) {
Expand All @@ -693,12 +693,17 @@ std::vector<VertexID> GoExecutor::getDstIdsFromRespWithBackTrack(const RpcRespon
for (const auto& edge : edata.get_edges()) {
auto dst = edge.get_dst();
if (!isFinalStep() && backTracker_ != nullptr) {
auto range = backTracker_->get(vdata.get_vertex_id());
if (range.first == range.second) { // not found root
backTrace.emplace(dst, vdata.get_vertex_id());
}
for (auto trace = range.first; trace != range.second; ++trace) {
backTrace.emplace(dst, trace->second);
// vertex_id is rootID
if (curStep_ == 1) {
auto key = std::pair<VertexID, VertexID>(dst, vdata.get_vertex_id());
curBackTrace.emplace(key);
} else {
// find rootID from preStep
auto preRange = backTracker_->get(curStep_ - 1, vdata.get_vertex_id());
for (auto trace = preRange.first; trace != preRange.second; ++trace) {
auto key = std::pair<VertexID, VertexID>(dst, trace->second);
curBackTrace.emplace(key);
}
}
}
set.emplace(dst);
Expand All @@ -707,7 +712,7 @@ std::vector<VertexID> GoExecutor::getDstIdsFromRespWithBackTrack(const RpcRespon
}
}
if (!isFinalStep() && backTracker_ != nullptr) {
backTracker_->inject(backTrace);
backTracker_->inject(curBackTrace, curStep_);
}
return std::vector<VertexID>(set.begin(), set.end());
}
Expand Down
22 changes: 16 additions & 6 deletions src/graph/GoExecutor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ class QueryResponse;

namespace graph {

struct HashPair {
template <class T1, class T2> size_t operator()(const std::pair<T1, T2>& p) const {
auto hash1 = std::hash<T1>{}(p.first);
auto hash2 = std::hash<T2>{}(p.second);
return hash1 ^ hash2;
}
};

class GoExecutor final : public TraverseExecutor {
public:
GoExecutor(Sentence *sentence, ExecutionContext *ectx);
Expand Down Expand Up @@ -180,20 +188,22 @@ class GoExecutor final : public TraverseExecutor {

class VertexBackTracker final {
public:
void inject(const std::multimap<VertexID, VertexID> &backTrace) {
void inject(const std::unordered_set<std::pair<VertexID, VertexID>, HashPair> &backTrace,
uint32_t curStep) {
// TODO(shylock) c++17 merge directly
for (const auto iter : backTrace) {
mapping_.emplace(iter.first, iter.second);
mapping_.emplace(std::pair<uint32_t, VertexID>(curStep, iter.first), iter.second);
}
}

auto get(VertexID id) const {
auto range = mapping_.equal_range(id);
auto get(uint32_t curStep, VertexID id) const {
auto range = mapping_.equal_range(std::pair<uint32_t, VertexID>(curStep, id));
return range;
}

private:
std::multimap<VertexID, VertexID> mapping_;
// Record the path from the dst node of each step to the root node.((curStep,dstID),rootID)
std::multimap<std::pair<uint32_t, VertexID>, VertexID> mapping_;
};

enum FromType {
Expand All @@ -212,7 +222,7 @@ class GoExecutor final : public TraverseExecutor {
ids.emplace_back(srcId);
return ids;
}
const auto range = DCHECK_NOTNULL(backTracker_)->get(srcId);
const auto range = DCHECK_NOTNULL(backTracker_)->get(record - 1, srcId);
for (auto i = range.first; i != range.second; ++i) {
ids.emplace_back(i->second);
}
Expand Down
42 changes: 42 additions & 0 deletions src/graph/test/GoTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3066,6 +3066,48 @@ TEST_P(GoTest, issueBackTrackOverlap) {
}
}

TEST_P(GoTest, NStepQueryHangAndOOM) {
{
std::vector<std::tuple<VertexID>> expected = {
{players_["Tony Parker"].vid()},
{players_["Manu Ginobili"].vid()},
{players_["Tim Duncan"].vid()},
{players_["Tim Duncan"].vid()},
{players_["LaMarcus Aldridge"].vid()},
{players_["Manu Ginobili"].vid()},
{players_["Tony Parker"].vid()},
{players_["Manu Ginobili"].vid()},
{players_["Tim Duncan"].vid()},
{players_["Tim Duncan"].vid()},
{players_["Tony Parker"].vid()},
};
{
cpp2::ExecutionResponse resp;
auto *fmt = "GO 1 TO 3 STEPS FROM %ld OVER like YIELD like._dst as dst";
auto query = folly::stringPrintf(fmt, players_["Tim Duncan"].vid());
auto code = client_->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
ASSERT_TRUE(verifyResult(resp, expected));
}
{
cpp2::ExecutionResponse resp;
auto *fmt = "YIELD %ld as id "
"| GO 1 TO 3 STEPS FROM $-.id OVER like YIELD like._dst as dst";
auto query = folly::stringPrintf(fmt, players_["Tim Duncan"].vid());
auto code = client_->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
ASSERT_TRUE(verifyResult(resp, expected));
}
{
cpp2::ExecutionResponse resp;
auto *fmt = "GO 1 TO 40 STEPS FROM %ld OVER like YIELD like._dst as dst";
auto query = folly::stringPrintf(fmt, players_["Tim Duncan"].vid());
auto code = client_->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
}
}

INSTANTIATE_TEST_CASE_P(IfPushdownFilter, GoTest, ::testing::Bool());

} // namespace graph
Expand Down

0 comments on commit 65a9cc3

Please sign in to comment.