bug fix: delete vertices processor may core, if this vertex also have edges and index (#2335)

* delete vertices may core, if this vertex also have edges and index

* different tagId will use the same vertexPrefix, so can not break if iterator an edge

Author: liuyu85cn

src/common/utils/NebulaKeyUtils.h

@@ -137,11 +137,8 @@ class NebulaKeyUtils final {
             msg << " rawKey.size() != kVertexLen."
                 << "\nrawKey.size()=" << rawKey.size()
                 << "\nkVertexLen=" << kVertexLen
-                << "\nrawkey string format=" << rawKey
-                << "\nrawkey dec format:";
-            for (auto i = 0U; i != rawKey.size(); ++i) {
-                msg << "\nrawKey[" << i << "]=" << static_cast<int>(rawKey[i]);
-            }
+                << "\nhexDump:\n"
+                << folly::hexDump(rawKey.data(), rawKey.size());
             LOG(FATAL) << msg.str();
         }
         auto offset = sizeof(PartitionID) + sizeof(VertexID);

src/storage/mutate/DeleteVerticesProcessor.cpp

@@ -95,12 +95,14 @@ DeleteVerticesProcessor::deleteVertices(GraphSpaceID spaceId,
         TagID latestVVId = -1;
         while (iter->valid()) {
             auto key = iter->key();
+            if (!NebulaKeyUtils::isVertex(key)) {
+                iter->next();
+                continue;
+            }
             auto tagId = NebulaKeyUtils::getTagId(key);
             if (FLAGS_enable_vertex_cache && vertexCache_ != nullptr) {
-                if (NebulaKeyUtils::isVertex(key)) {
-                    VLOG(3) << "Evict vertex cache for vertex ID " << vertex << ", tagId " << tagId;
-                    vertexCache_->evict(std::make_pair(vertex, tagId));
-                }
+                VLOG(3) << "Evict vertex cache for vertex ID " << vertex << ", tagId " << tagId;
+                vertexCache_->evict(std::make_pair(vertex, tagId));
             }
 
             /**

src/storage/test/DeleteVerticesTest.cpp

@@ -10,7 +10,9 @@
 #include "fs/TempDir.h"
 #include "storage/test/TestUtils.h"
 #include "storage/mutate/DeleteVerticesProcessor.h"
+#include "storage/mutate/DeleteEdgesProcessor.h"
 #include "storage/mutate/AddVerticesProcessor.h"
+#include "storage/mutate/AddEdgesProcessor.h"
 #include "utils/NebulaKeyUtils.h"
 
 
@@ -90,6 +92,102 @@ TEST(DeleteVerticesTest, SimpleTest) {
     }
 }
 
+TEST(DeleteVerticesTest, DeleteVerticesMayCoreIfThereAreEdges) {
+    fs::TempDir rootPath("/tmp/DeleteVertexTest.XXXXXX");
+    std::unique_ptr<kvstore::KVStore> kv(TestUtils::initKV(rootPath.path()));
+    auto schemaMan = TestUtils::mockSchemaMan();
+    auto indexMan = TestUtils::mockIndexMan();
+    // Add vertices
+    {
+        auto* processor = AddVerticesProcessor::instance(kv.get(),
+                                                         schemaMan.get(),
+                                                         indexMan.get(),
+                                                         nullptr);
+        cpp2::AddVerticesRequest req;
+        req.space_id = 0;
+        req.overwritable = false;
+        // partId => List<Vertex>
+        for (PartitionID partId = 0; partId < 3; partId++) {
+            auto vertices = TestUtils::setupVertices(partId, partId * 10, 10 * (partId + 1));
+            req.parts.emplace(partId, std::move(vertices));
+        }
+
+        auto fut = processor->getFuture();
+        processor->process(req);
+        auto resp = std::move(fut).get();
+        EXPECT_EQ(0, resp.result.failed_codes.size());
+
+        for (PartitionID partId = 0; partId < 3; partId++) {
+            for (VertexID vertexId = 10 * partId; vertexId < 10 * (partId + 1); vertexId++) {
+                auto prefix = NebulaKeyUtils::vertexPrefix(partId, vertexId);
+                std::unique_ptr<kvstore::KVIterator> iter;
+                EXPECT_EQ(kvstore::ResultCode::SUCCEEDED, kv->prefix(0, partId, prefix, &iter));
+                TagID tagId = 0;
+                while (iter->valid()) {
+                    EXPECT_EQ(TestUtils::encodeValue(partId, vertexId, tagId), iter->val());
+                    tagId++;
+                    iter->next();
+                }
+                EXPECT_EQ(10, tagId);
+            }
+        }
+    }
+
+    // Add edges
+    {
+        auto* processor = AddEdgesProcessor::instance(kv.get(),
+                                                      schemaMan.get(),
+                                                      indexMan.get(),
+                                                      nullptr);
+        cpp2::AddEdgesRequest req;
+        req.space_id = 0;
+        req.overwritable = true;
+        for (PartitionID partId = 1; partId <= 3; partId++) {
+            auto edges = TestUtils::setupEdges(partId, partId * 10, 10 * (partId + 1));
+            req.parts.emplace(partId, std::move(edges));
+        }
+
+        auto fut = processor->getFuture();
+        processor->process(req);
+        auto resp = std::move(fut).get();
+        EXPECT_EQ(0, resp.result.failed_codes.size());
+    }
+
+    // Delete vertices
+    {
+        cpp2::DeleteVerticesRequest req;
+        req.set_space_id(0);
+        for (PartitionID partId = 0; partId < 3; partId++) {
+            std::vector<VertexID> vertices;
+            for (VertexID vertexId = 10 * partId; vertexId < 10 * (partId + 1); vertexId++) {
+                vertices.emplace_back(vertexId);
+            }
+            req.parts.emplace(partId, std::move(vertices));
+        }
+
+        auto* processor = DeleteVerticesProcessor::instance(kv.get(),
+                                                            schemaMan.get(),
+                                                            indexMan.get(),
+                                                            nullptr);
+
+        // pretend there are some index
+
+        auto fut = processor->getFuture();
+        processor->process(req);
+        auto resp = std::move(fut).get();
+        EXPECT_EQ(0, resp.result.failed_codes.size());
+    }
+
+    for (PartitionID partId = 0; partId < 3; partId++) {
+        for (VertexID vertexId = 10 * partId; vertexId < 10 * (partId + 1); vertexId++) {
+            auto prefix = NebulaKeyUtils::vertexPrefix(partId, vertexId);
+            std::unique_ptr<kvstore::KVIterator> iter;
+            EXPECT_EQ(kvstore::ResultCode::SUCCEEDED, kv->prefix(0, partId, prefix, &iter));
+            CHECK(!NebulaKeyUtils::isVertex(iter->key()));
+        }
+    }
+}
+
 }  // namespace storage
 }  // namespace nebula