fix storage crash (#4305)

* check vid length first * address comment Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
vesoft-inc · Jun 8, 2022 · 35b6e20 · 35b6e20
1 parent bcf508e
commit 35b6e20
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 21 deletions.
diff --git a/src/storage/mutate/AddEdgesProcessor.cpp b/src/storage/mutate/AddEdgesProcessor.cpp
@@ -156,29 +156,20 @@ void AddEdgesProcessor::doProcessWithIndex(const cpp2::AddEdgesRequest& req) {
  auto partId = part.first;
  const auto& edges = part.second;
  // cache edgeKey
- std::unordered_set<std::string> visited;
- visited.reserve(edges.size());
  std::vector<kvstore::KV> kvs;
  kvs.reserve(edges.size());
- auto code = nebula::cpp2::ErrorCode::SUCCEEDED;
- deleteDupEdge(const_cast<std::vector<cpp2::NewEdge>&>(edges));
+ auto code = deleteDupEdge(const_cast<std::vector<cpp2::NewEdge>&>(edges));
+ if (code != nebula::cpp2::ErrorCode::SUCCEEDED) {
+ handleAsync(spaceId_, partId, code);
+ continue;
+ }
  for (const auto& edge : edges) {
  auto edgeKey = *edge.key_ref();
  VLOG(3) << "PartitionID: " << partId << ", VertexID: " << *edgeKey.src_ref()
  << ", EdgeType: " << *edgeKey.edge_type_ref()
  << ", EdgeRanking: " << *edgeKey.ranking_ref()
  << ", VertexID: " << *edgeKey.dst_ref();
 
- if (!NebulaKeyUtils::isValidVidLen(
- spaceVidLen_, edgeKey.src_ref()->getStr(), edgeKey.dst_ref()->getStr())) {
- LOG(ERROR) << "Space " << spaceId_ << " vertex length invalid, "
- << "space vid len: " << spaceVidLen_ << ", edge srcVid: " << *edgeKey.src_ref()
- << ", dstVid: " << *edgeKey.dst_ref() << ", ifNotExists_: " << std::boolalpha
- << ifNotExists_;
- code = nebula::cpp2::ErrorCode::E_INVALID_VID;
- break;
- }
-
  auto schema = env_->schemaMan_->getEdgeSchema(spaceId_, std::abs(*edgeKey.edge_type_ref()));
  if (!schema) {
  LOG(ERROR) << "Space " << spaceId_ << ", Edge " << *edgeKey.edge_type_ref() << " invalid";
@@ -192,11 +183,6 @@ void AddEdgesProcessor::doProcessWithIndex(const cpp2::AddEdgesRequest& req) {
  *edgeKey.edge_type_ref(),
  *edgeKey.ranking_ref(),
  (*edgeKey.dst_ref()).getStr());
- if (ifNotExists_ && !visited.emplace(key).second) {
- LOG(INFO) << "skip " << edgeKey.src_ref()->getStr();
- continue;
- }
-
  // collect values
  WriteResult writeResult;
  const auto& props = edge.get_props();
@@ -386,13 +372,22 @@ std::vector<std::string> AddEdgesProcessor::indexKeys(
  * ifNotExist_ is true. Only keep the first one when edgeKey is same
  * ifNotExist_ is false. Only keep the last one when edgeKey is same
  */
-void AddEdgesProcessor::deleteDupEdge(std::vector<cpp2::NewEdge>& edges) {
+nebula::cpp2::ErrorCode AddEdgesProcessor::deleteDupEdge(std::vector<cpp2::NewEdge>& edges) {
  std::unordered_set<std::string> visited;
  visited.reserve(edges.size());
  if (ifNotExists_) {
  auto iter = edges.begin();
  while (iter != edges.end()) {
  auto edgeKeyRef = iter->key_ref();
+ if (!NebulaKeyUtils::isValidVidLen(
+ spaceVidLen_, edgeKeyRef->src_ref()->getStr(), edgeKeyRef->dst_ref()->getStr())) {
+ LOG(ERROR) << "Space " << spaceId_ << " vertex length invalid, "
+ << "space vid len: " << spaceVidLen_
+ << ", edge srcVid: " << *edgeKeyRef->src_ref()
+ << ", dstVid: " << *edgeKeyRef->dst_ref() << ", ifNotExists_: " << std::boolalpha
+ << ifNotExists_;
+ return nebula::cpp2::ErrorCode::E_INVALID_VID;
+ }
  auto key = NebulaKeyUtils::edgeKey(spaceVidLen_,
  0, // it's ok, just distinguish between different edgekey
  edgeKeyRef->src_ref()->getStr(),
@@ -409,6 +404,15 @@ void AddEdgesProcessor::deleteDupEdge(std::vector<cpp2::NewEdge>& edges) {
  auto iter = edges.rbegin();
  while (iter != edges.rend()) {
  auto edgeKeyRef = iter->key_ref();
+ if (!NebulaKeyUtils::isValidVidLen(
+ spaceVidLen_, edgeKeyRef->src_ref()->getStr(), edgeKeyRef->dst_ref()->getStr())) {
+ LOG(ERROR) << "Space " << spaceId_ << " vertex length invalid, "
+ << "space vid len: " << spaceVidLen_
+ << ", edge srcVid: " << *edgeKeyRef->src_ref()
+ << ", dstVid: " << *edgeKeyRef->dst_ref() << ", ifNotExists_: " << std::boolalpha
+ << ifNotExists_;
+ return nebula::cpp2::ErrorCode::E_INVALID_VID;
+ }
  auto key = NebulaKeyUtils::edgeKey(spaceVidLen_,
  0, // it's ok, just distinguish between different edgekey
  edgeKeyRef->src_ref()->getStr(),
@@ -422,6 +426,7 @@ void AddEdgesProcessor::deleteDupEdge(std::vector<cpp2::NewEdge>& edges) {
  }
  }
  }
+ return nebula::cpp2::ErrorCode::SUCCEEDED;
 }
 
 } // namespace storage

diff --git a/src/storage/mutate/AddEdgesProcessor.h b/src/storage/mutate/AddEdgesProcessor.h
@@ -50,7 +50,7 @@ class AddEdgesProcessor : public BaseProcessor<cpp2::ExecResponse> {
  std::shared_ptr<nebula::meta::cpp2::IndexItem> index,
  const meta::SchemaProviderIf* latestSchema);
 
- void deleteDupEdge(std::vector<cpp2::NewEdge>& edges);
+ nebula::cpp2::ErrorCode deleteDupEdge(std::vector<cpp2::NewEdge>& edges);
 
  private:
  GraphSpaceID spaceId_;

diff --git a/tests/tck/features/insert/Insert.feature b/tests/tck/features/insert/Insert.feature
@@ -614,4 +614,19 @@ Feature: Insert string vid of vertex and edge
  Then the result should be, in any order:
  | src | dst |
  | "300" | "400" |
+ When try to execute query:
+ """
+ INSERT EDGE like(grade) VALUES "3000000000000000000000000000000000000000000000000000000000" -> "400":(888)
+ """
+ Then a ExecutionError should be raised at runtime: Storage Error: The VID must be a 64-bit integer or a string fitting space vertex id length limit.
+ When try to execute query:
+ """
+ INSERT EDGE like(grade) VALUES "300" -> "4000000000000000000000000000000000000000000000000000000000":(888)
+ """
+ Then a ExecutionError should be raised at runtime: Storage Error: The VID must be a 64-bit integer or a string fitting space vertex id length limit.
+ When try to execute query:
+ """
+ INSERT EDGE like(grade) VALUES "300000000000000000000000000000000000000000000000000" -> "4000000000000000000000000000000000000000000000000000000000":(888)
+ """
+ Then a ExecutionError should be raised at runtime: Storage Error: The VID must be a 64-bit integer or a string fitting space vertex id length limit.
  Then drop the used space