Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix_shortest_path_crash #4352

Merged
merged 1 commit into from
Jun 24, 2022
Merged

fix_shortest_path_crash #4352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/graph/executor/algo/BatchShortestPath.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ folly::Future<Status> BatchShortestPath::getNeighbors(size_t rowNum, size_t step
nullptr)
.via(qctx_->rctx()->runner())
.thenValue([this, rowNum, reverse, stepNum, getNbrTime](auto&& resp) {
addStats(resp, stats_, stepNum, getNbrTime.elapsedInUSec(), reverse);
addStats(resp, stepNum, getNbrTime.elapsedInUSec(), reverse);
return buildPath(rowNum, std::move(resp), reverse);
});
}
Expand Down
17 changes: 9 additions & 8 deletions src/graph/executor/algo/ShortestPathBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ folly::Future<std::vector<Value>> ShortestPathBase::getMeetVidsProps(
nullptr)
.via(qctx_->rctx()->runner())
.thenValue([this, getPropsTime](PropRpcResponse&& resp) {
addStats(resp, stats_, getPropsTime.elapsedInUSec());
addStats(resp, getPropsTime.elapsedInUSec());
return handlePropResp(std::move(resp));
});
}
Expand Down Expand Up @@ -168,7 +168,6 @@ Status ShortestPathBase::handleErrorCode(nebula::cpp2::ErrorCode code, Partition
}

void ShortestPathBase::addStats(RpcResponse& resp,
std::unordered_map<std::string, std::string>* stats,
size_t stepNum,
int64_t timeInUSec,
bool reverse) const {
Expand All @@ -193,15 +192,17 @@ void ShortestPathBase::addStats(RpcResponse& resp,
}
ss << "\n}";
if (reverse) {
stats->emplace(folly::sformat("reverse step {}", stepNum), ss.str());
statsLock_.lock();
stats_->emplace(folly::sformat("reverse step {}", stepNum), ss.str());
statsLock_.unlock();
} else {
stats->emplace(folly::sformat("step {}", stepNum), ss.str());
statsLock_.lock();
stats_->emplace(folly::sformat("step {}", stepNum), ss.str());
statsLock_.unlock();
}
}

void ShortestPathBase::addStats(PropRpcResponse& resp,
std::unordered_map<std::string, std::string>* stats,
int64_t timeInUSec) const {
void ShortestPathBase::addStats(PropRpcResponse& resp, int64_t timeInUSec) const {
auto& hostLatency = resp.hostLatency();
std::stringstream ss;
ss << "{\n";
Expand All @@ -217,7 +218,7 @@ void ShortestPathBase::addStats(PropRpcResponse& resp,
ss << "\n}";
}
ss << "\n}";
stats->emplace(folly::sformat("get_prop "), ss.str());
stats_->emplace(folly::sformat("get_prop "), ss.str());
}

} // namespace graph
Expand Down
11 changes: 3 additions & 8 deletions src/graph/executor/algo/ShortestPathBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,9 @@ class ShortestPathBase {

Status handleErrorCode(nebula::cpp2::ErrorCode code, PartitionID partId) const;

void addStats(RpcResponse& resp,
std::unordered_map<std::string, std::string>* stats,
size_t stepNum,
int64_t timeInUSec,
bool reverse) const;
void addStats(RpcResponse& resp, size_t stepNum, int64_t timeInUSec, bool reverse) const;

void addStats(PropRpcResponse& resp,
std::unordered_map<std::string, std::string>* stats,
int64_t timeInUSec) const;
void addStats(PropRpcResponse& resp, int64_t timeInUSec) const;

template <typename Resp>
StatusOr<Result::State> handleCompleteness(const storage::StorageRpcResponse<Resp>& rpcResp,
Expand Down Expand Up @@ -86,6 +80,7 @@ class ShortestPathBase {
std::unordered_map<std::string, std::string>* stats_{nullptr};
size_t maxStep_;
bool singleShortest_{true};
folly::SpinLock statsLock_;
Copy link
Contributor

@Shylock-Hg Shylock-Hg Jun 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about other members?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other members will not be written by multiple threads at the same time


std::vector<DataSet> resultDs_;
std::vector<DataSet> leftVids_;
Expand Down
5 changes: 4 additions & 1 deletion src/graph/executor/algo/SingleShortestPath.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ folly::Future<Status> SingleShortestPath::getNeighbors(size_t rowNum,
nullptr)
.via(qctx_->rctx()->runner())
.thenValue([this, rowNum, stepNum, getNbrTime, reverse](auto&& resp) {
addStats(resp, stats_, stepNum, getNbrTime.elapsedInUSec(), reverse);
addStats(resp, stepNum, getNbrTime.elapsedInUSec(), reverse);
return buildPath(rowNum, std::move(resp), reverse);
});
}
Expand Down Expand Up @@ -264,6 +264,9 @@ folly::Future<bool> SingleShortestPath::buildEvenPath(size_t rowNum,
return false;
}
for (auto& meetVertex : vertices) {
if (!meetVertex.isVertex()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this isn't vertex?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vertices may contain EMPTY values ​​in exceptional cases

continue;
}
auto meetVid = meetVertex.getVertex().vid;
auto leftPaths = createLeftPath(rowNum, meetVid);
auto rightPaths = createRightPath(rowNum, meetVid, false);
Expand Down