Solved memory leaks

src/memgraph.cpp

@@ -775,7 +775,8 @@ int main(int argc, char **argv) {
         // libstd.
         auto gil = memgraph::py::EnsureGIL();
         // NOLINTNEXTLINE(hicpp-signed-bitwise)
-        auto *flag = PyLong_FromLong(RTLD_NOW | RTLD_DEEPBIND);
+        // auto *flag = PyLong_FromLong(RTLD_NOW | RTLD_DEEPBIND);
+        auto *flag = PyLong_FromLong(RTLD_NOW);
         auto *setdl = PySys_GetObject("setdlopenflags");
         MG_ASSERT(setdl);
         auto *arg = PyTuple_New(1);

src/query/procedure/module.cpp

@@ -845,7 +845,8 @@ bool SharedLibraryModule::Load(const std::filesystem::path &file_path) {
   file_path_ = file_path;
   dlerror();  // Clear any existing error.
   // NOLINTNEXTLINE(hicpp-signed-bitwise)
-  handle_ = dlopen(file_path.c_str(), RTLD_NOW | RTLD_LOCAL | RTLD_DEEPBIND);
+  // handle_ = dlopen(file_path.c_str(), RTLD_NOW | RTLD_LOCAL | RTLD_DEEPBIND);
+  handle_ = dlopen(file_path.c_str(), RTLD_NOW | RTLD_LOCAL);
   if (!handle_) {
     spdlog::error(
         utils::MessageWithLink("Unable to load module {}; {}.", file_path, dlerror(), "https://memgr.ph/modules"));

src/storage/rocks/storage.hpp

@@ -15,6 +15,7 @@
 #include <rocksdb/iterator.h>
 #include <rocksdb/options.h>
 #include <rocksdb/status.h>
+#include <boost/filesystem.hpp>
 #include <iterator>
 #include <numeric>
 #include <optional>
@@ -32,6 +33,7 @@
 #include "storage/v2/edge.hpp"
 #include "storage/v2/edge_accessor.hpp"
 #include "storage/v2/id_types.hpp"
+#include "storage/v2/property_store.hpp"
 #include "storage/v2/property_value.hpp"
 #include "storage/v2/result.hpp"
 #include "storage/v2/storage.hpp"
@@ -40,6 +42,7 @@
 #include "storage/v2/view.hpp"
 #include "utils/algorithm.hpp"
 #include "utils/exceptions.hpp"
+#include "utils/file.hpp"
 #include "utils/logging.hpp"
 #include "utils/string.hpp"
 
@@ -62,7 +65,7 @@ class RocksDBStorage {
   explicit RocksDBStorage() {
     options_.create_if_missing = true;
     // options_.OptimizeLevelStyleCompaction();
-    std::filesystem::path rocksdb_path = "./rocks_experiment_unit_test";
+    std::filesystem::path rocksdb_path = "./rocks_experiment_unit";
     MG_ASSERT(utils::EnsureDir(rocksdb_path), "Unable to create storage folder on the disk.");
     AssertRocksDBStatus(rocksdb::DB::Open(options_, rocksdb_path, &db_));
     AssertRocksDBStatus(db_->CreateColumnFamily(rocksdb::ColumnFamilyOptions(), "vertex", &vertex_chandle));
@@ -74,6 +77,8 @@ class RocksDBStorage {
   ~RocksDBStorage() {
     AssertRocksDBStatus(db_->DropColumnFamily(vertex_chandle));
     AssertRocksDBStatus(db_->DropColumnFamily(edge_chandle));
+    AssertRocksDBStatus(db_->DestroyColumnFamilyHandle(vertex_chandle));
+    AssertRocksDBStatus(db_->DestroyColumnFamilyHandle(edge_chandle));
     AssertRocksDBStatus(db_->Close());
     delete db_;
   }
@@ -234,14 +239,16 @@ class RocksDBStorage {
   /// This should again be changed when we have mulitple same vertices
   std::optional<query::VertexAccessor> FindVertex(const std::string_view gid, query::DbAccessor &dba) {
     rocksdb::Iterator *it = db_->NewIterator(rocksdb::ReadOptions(), vertex_chandle);
+    std::optional<query::VertexAccessor> result = {};
     for (it->SeekToFirst(); it->Valid(); it->Next()) {
-      const std::string_view key = it->key().ToStringView();
+      const auto &key = it->key().ToString();
       if (const auto vertex_parts = utils::Split(key, "|"); vertex_parts[1] == gid) {
-        return DeserializeVertex(key, it->value().ToStringView(), dba);
+        result = DeserializeVertex(key, it->value().ToStringView(), dba);
+        break;
       }
     }
     delete it;
-    return std::nullopt;
+    return result;
   }
 
   /// Read all vertices stored in the database by a label
@@ -260,6 +267,7 @@ class RocksDBStorage {
     return vertices;
   }
 
+  /// TODO: unique ptr on iterators
   /// TODO: rewrite the code with means of lambda operation
   /// TODO: we need to this, otherwise we will have to change a lot of things as we are dealing on the low level
   /// certainly can be a bit optimized so that new object isn't created if can be discarded
@@ -315,7 +323,7 @@ class RocksDBStorage {
   query::VertexAccessor DeserializeVertex(const std::string_view key, const std::string_view value,
                                           query::DbAccessor &dba) {
     // Create vertex
-    auto impl = query::VertexAccessor(dba.InsertVertex());
+    auto impl = dba.InsertVertex();
     spdlog::info("Key to deserialize: {}", key);
     const auto vertex_parts = utils::Split(key, "|");
     // Deserialize labels
@@ -391,13 +399,12 @@ class RocksDBStorage {
     const auto edge_type_id = storage::EdgeTypeId::FromUint(std::stoull(edge_parts[3]));
     // TODO: remove to deserialization edge type id method
     const auto edge_gid = storage::Gid::FromUint(std::stoull(edge_parts[4]));
-    const auto maybe_edge = dba.InsertEdge(&*from_acc, &*to_acc, edge_type_id);
+    auto maybe_edge = dba.InsertEdge(&*from_acc, &*to_acc, edge_type_id);
     MG_ASSERT(maybe_edge.HasValue());
-    auto edge_impl = query::EdgeAccessor(*maybe_edge);
     // in the new storage API, setting gid must be done atomically
-    edge_impl.SetGid(edge_gid);
+    maybe_edge->SetGid(edge_gid);
-    edge_impl.SetPropertyStore(value);
+    maybe_edge->SetPropertyStore(value);
-    return edge_impl;
+    return *maybe_edge;
   }
 
  private:

src/storage/v2/property_store.cpp

@@ -1222,18 +1222,36 @@ bool PropertyStore::ClearProperties() {
   return true;
 }
 
-std::string PropertyStore::StringBuffer() const {
+std::string PropertyStore::StringBuffer() {
-  std::string arr(sizeof(buffer_), ' ');
+  uint64_t size = 0;
-  for (uint i = 0; i < sizeof(buffer_); ++i) {
+  uint8_t *data = nullptr;
-    arr[i] = static_cast<char>(buffer_[i]);
+  std::tie(size, data) = GetSizeData(buffer_);
+  if (size % 8 != 0) {  // We are storing the data in the local buffer.
+    size = sizeof(buffer_) - 1;
+    data = &buffer_[1];
+  }
+  std::string arr(size, ' ');
+  for (uint i = 0; i < size; ++i) {
+    arr[i] = static_cast<char>(data[i]);
   }
   return arr;
 }
 
 void PropertyStore::SetBuffer(const std::string_view buffer) {
-  MG_ASSERT(buffer.size() == sizeof(buffer_));
+  uint64_t size = 0;
-  for (uint i = 0; i < sizeof(buffer_); ++i) {
+  uint8_t *data = nullptr;
-    buffer_[i] = static_cast<uint8_t>(buffer[i]);
+  if (buffer.size() == sizeof(buffer_) - 1) {  // use local buffer
+    buffer_[0] = kUseLocalBuffer;
+    size = buffer.size() - 1;
+    data = &buffer_[1];
+  } else {
+    size = buffer.size();
+    data = new uint8_t[size];
+    SetSizeData(buffer_, size, data);
+  }
+
+  for (uint i = 0; i < size; ++i) {
+    data[i] = static_cast<uint8_t>(buffer[i]);
   }
 }
 

src/storage/v2/property_store.hpp

@@ -72,7 +72,7 @@ class PropertyStore {
   bool ClearProperties();
 
   /// Return property buffer as a string
-  std::string StringBuffer() const;
+  std::string StringBuffer();
 
   /// Sets buffer
   void SetBuffer(std::string_view buffer);

tests/unit/storage_rocks.cpp

@@ -53,7 +53,6 @@ TEST_F(RocksDBStorageTest, SerializeVertexGID) {
   }
 }
 
-// TODO: clean labels string
 TEST_F(RocksDBStorageTest, SerializeVertexGIDLabels) {
   // serialize vertex's gid with its single label
   auto storage_dba = storage.Access(memgraph::storage::IsolationLevel::READ_UNCOMMITTED);
@@ -111,25 +110,25 @@ TEST_F(RocksDBStorageTest, SerializeVertexGIDMutlipleLabels) {
   }
 }
 
-// TEST_F(RocksDBStorageTest, GetVerticesByLabel) {
+TEST_F(RocksDBStorageTest, GetVerticesByLabel) {
-//   // search vertices by label
+  // search vertices by label
-//   auto storage_dba = storage.Access(memgraph::storage::IsolationLevel::READ_UNCOMMITTED);
+  auto storage_dba = storage.Access(memgraph::storage::IsolationLevel::READ_UNCOMMITTED);
-//   memgraph::query::DbAccessor dba(&storage_dba);
+  memgraph::query::DbAccessor dba(&storage_dba);
-//   // prepare labels
+  // prepare labels
-//   std::vector<memgraph::storage::LabelId> label_ids{dba.NameToLabel("Player"), dba.NameToLabel("Player"),
+  std::vector<memgraph::storage::LabelId> label_ids{dba.NameToLabel("Player"), dba.NameToLabel("Player"),
-//                                                     dba.NameToLabel("Ball")};
+                                                    dba.NameToLabel("Ball")};
-//   // insert vertices
+  // insert vertices
-//   for (int i = 0; i < 5; ++i) {
+  for (int i = 0; i < 5; ++i) {
-//     auto impl = dba.InsertVertex();
+    auto impl = dba.InsertVertex();
-//     impl.AddLabel(label_ids[i % 3]);
+    impl.AddLabel(label_ids[i % 3]);
-//     db.StoreVertex(impl);
+    db.StoreVertex(impl);
-//   }
+  }
-//   // load vertices from disk
+  // load vertices from disk
-//   auto player_vertices = db.Vertices(dba, dba.NameToLabel("Player"));
+  auto player_vertices = db.Vertices(dba, dba.NameToLabel("Player"));
-//   auto ball_vertices = db.Vertices(dba, dba.NameToLabel("Ball"));
+  auto ball_vertices = db.Vertices(dba, dba.NameToLabel("Ball"));
-//   ASSERT_EQ(player_vertices.size(), 4);
+  ASSERT_EQ(player_vertices.size(), 4);
-//   ASSERT_EQ(ball_vertices.size(), 1);
+  ASSERT_EQ(ball_vertices.size(), 1);
-// }
+}
 
 TEST_F(RocksDBStorageTest, GetVerticesByProperty) {
   // search vertices by property value
@@ -160,6 +159,82 @@ TEST_F(RocksDBStorageTest, GetVerticesByProperty) {
   ASSERT_EQ(hdd_vertices_non_existing.size(), 0);
 }
 
+TEST_F(RocksDBStorageTest, DeleteVertex) {
+  // auto storage_dba = storage.Access(memgraph::storage::IsolationLevel::READ_UNCOMMITTE);
+  auto storage_dba = storage.Access();
+  memgraph::query::DbAccessor dba(&storage_dba);
+  std::map<memgraph::storage::PropertyId, memgraph::storage::PropertyValue> properties;
+  // samo 1 property stane
+  properties.emplace(dba.NameToProperty("sum"), memgraph::storage::PropertyValue("2TB"));
+  properties.emplace(dba.NameToProperty("same_type"), memgraph::storage::PropertyValue(true));
+  // properties.emplace(dba.NameToProperty("cluster_price"), memgraph::storage::PropertyValue(2000.42));
+  // create vertex
+  auto impl = dba.InsertVertex();
+  impl.AddLabel(dba.NameToLabel("Player"));
+  memgraph::query::MultiPropsInitChecked(&impl, properties);
+  db.StoreVertex(impl);
+  // find vertex should work now
+  ASSERT_TRUE(db.FindVertex(std::to_string(impl.Gid().AsUint()), dba).has_value());
+  db.FindVertex(std::to_string(impl.Gid().AsUint()), dba);
+  // RocksDB doesn't physically delete entry so deletion will pass two times
+  ASSERT_TRUE(db.DeleteVertex(impl).has_value());
+  ASSERT_TRUE(db.DeleteVertex(impl).has_value());
+  // second time you shouldn't be able to find the vertex
+  ASSERT_FALSE(db.FindVertex(std::to_string(impl.Gid().AsUint()), dba).has_value());
+}
+
+TEST_F(RocksDBStorageTest, SerializeVertexGIDProperties) {
+  // serializes vertex's gid, multiple labels and properties
+  auto storage_dba = storage.Access(memgraph::storage::IsolationLevel::READ_UNCOMMITTED);
+  memgraph::query::DbAccessor dba(&storage_dba);
+  // prepare labels
+  std::vector<memgraph::storage::LabelId> label_ids{dba.NameToLabel("Player"), dba.NameToLabel("Person"),
+                                                    dba.NameToLabel("Ball")};
+  // prepare properties
+  std::map<memgraph::storage::PropertyId, memgraph::storage::PropertyValue> properties;
+  properties.emplace(dba.NameToProperty("name"), memgraph::storage::PropertyValue("disk"));
+  properties.emplace(dba.NameToProperty("memory"), memgraph::storage::PropertyValue("1TB"));
+  properties.emplace(dba.NameToProperty("price"), memgraph::storage::PropertyValue(1000.21));
+  properties.emplace(dba.NameToProperty("price2"), memgraph::storage::PropertyValue(1000.212));
+  // gids
+  std::unordered_set<uint64_t> gids;
+  for (int i = 0; i < 5; ++i) {
+    gids.insert(i);
+    auto impl = dba.InsertVertex();
+    impl.SetGid(memgraph::storage::Gid::FromUint(i));
+    impl.AddLabel(label_ids[i % 3]);
+    impl.AddLabel(label_ids[(i + 1) % 3]);
+    memgraph::query::MultiPropsInitChecked(&impl, properties);
+    db.StoreVertex(impl);
+  }
+  // load vertices from disk
+  auto loaded_vertices = db.Vertices(dba);
+  ASSERT_EQ(loaded_vertices.size(), 5);
+  for (const auto &vertex_acc : loaded_vertices) {
+    ASSERT_TRUE(gids.contains(vertex_acc.Gid().AsUint()));
+    // labels
+    auto labels = vertex_acc.Labels(memgraph::storage::View::OLD);
+    ASSERT_EQ(labels->size(), 2);
+    ASSERT_TRUE(std::all_of(labels->begin(), labels->end(), [&label_ids](const auto &label_id) {
+      return std::find(label_ids.begin(), label_ids.end(), label_id) != label_ids.end();
+    }));
+    // check properties
+    auto props = vertex_acc.Properties(memgraph::storage::View::OLD);
+    ASSERT_FALSE(props.HasError());
+    auto prop_name = vertex_acc.GetProperty(memgraph::storage::View::OLD, dba.NameToProperty("name"));
+    auto prop_memory = vertex_acc.GetProperty(memgraph::storage::View::OLD, dba.NameToProperty("memory"));
+    auto prop_price = vertex_acc.GetProperty(memgraph::storage::View::OLD, dba.NameToProperty("price"));
+    auto prop_unexisting = vertex_acc.GetProperty(memgraph::storage::View::OLD, dba.NameToProperty("random"));
+    ASSERT_TRUE(prop_name->IsString());
+    ASSERT_EQ(prop_name->ValueString(), "disk");
+    ASSERT_TRUE(prop_memory->IsString());
+    ASSERT_EQ(prop_memory->ValueString(), "1TB");
+    ASSERT_TRUE(prop_price->IsDouble());
+    ASSERT_DOUBLE_EQ(prop_price->ValueDouble(), 1000.21);
+    ASSERT_TRUE(prop_unexisting->IsNull());
+  }
+}
+
 TEST_F(RocksDBStorageTest, SerializeEdge) {
   // create two vertices and edge between them
   // search by one of the vertices, return edge
@@ -221,8 +296,6 @@ TEST_F(RocksDBStorageTest, SerializeEdge) {
   ASSERT_EQ(src_out_edge.EdgeType(), dba.NameToEdgeType(edge_type_id));
   ASSERT_EQ(*src_out_edge.Properties(memgraph::storage::View::OLD), edge_properties);
   // Test in edge of the destination vertex
-  dba.DetachRemoveVertex(&src_vertex);
-  dba.DetachRemoveVertex(&dest_vertex);
   auto dest_in_edges = db.InEdges(dest_vertex, dba);
   ASSERT_EQ(dest_in_edges.size(), 1);
   auto dest_in_edge = dest_in_edges[0];
@@ -248,75 +321,3 @@ TEST_F(RocksDBStorageTest, SerializeEdge) {
   ASSERT_EQ(*dest_in_edge.Properties(memgraph::storage::View::OLD),
             *src_out_edge.Properties(memgraph::storage::View::OLD));
 }
-
-TEST_F(RocksDBStorageTest, DeleteVertex) {
-  auto storage_dba = storage.Access(memgraph::storage::IsolationLevel::READ_UNCOMMITTED);
-  memgraph::query::DbAccessor dba(&storage_dba);
-  std::map<memgraph::storage::PropertyId, memgraph::storage::PropertyValue> properties;
-  properties.emplace(dba.NameToProperty("sum"), memgraph::storage::PropertyValue("2TB"));
-  properties.emplace(dba.NameToProperty("same_type"), memgraph::storage::PropertyValue(true));
-  properties.emplace(dba.NameToProperty("cluster_price"), memgraph::storage::PropertyValue(2000.42));
-  // create vertex
-  auto impl = dba.InsertVertex();
-  impl.AddLabel(dba.NameToLabel("Player"));
-  memgraph::query::MultiPropsInitChecked(&impl, properties);
-  db.StoreVertex(impl);
-  // find vertex should work now
-  ASSERT_TRUE(db.FindVertex(std::to_string(impl.Gid().AsUint()), dba).has_value());
-  // RocksDB doesn't physically delete entry so deletion will pass two times
-  ASSERT_TRUE(db.DeleteVertex(impl).has_value());
-  ASSERT_TRUE(db.DeleteVertex(impl).has_value());
-  // second time you shouldn't be able to find the vertex
-  ASSERT_FALSE(db.FindVertex(std::to_string(impl.Gid().AsUint()), dba).has_value());
-}
-
-// TEST_F(RocksDBStorageTest, SerializeVertexGIDProperties) {
-//   // serializes vertex's gid, multiple labels and properties
-//   auto storage_dba = storage.Access(memgraph::storage::IsolationLevel::READ_UNCOMMITTED);
-//   memgraph::query::DbAccessor dba(&storage_dba);
-//   // prepare labels
-//   std::vector<memgraph::storage::LabelId> label_ids{dba.NameToLabel("Player"), dba.NameToLabel("Person"),
-//                                                     dba.NameToLabel("Ball")};
-//   // prepare properties
-//   std::map<memgraph::storage::PropertyId, memgraph::storage::PropertyValue> properties;
-//   properties.emplace(dba.NameToProperty("name"), memgraph::storage::PropertyValue("disk"));
-//   properties.emplace(dba.NameToProperty("memory"), memgraph::storage::PropertyValue("1TB"));
-//   properties.emplace(dba.NameToProperty("price"), memgraph::storage::PropertyValue(1000.21));
-//   // gids
-//   std::unordered_set<uint64_t> gids;
-//   for (int i = 0; i < 5; ++i) {
-//     gids.insert(i);
-//     auto impl = dba.InsertVertex();
-//     impl.SetGid(memgraph::storage::Gid::FromUint(i));
-//     impl.AddLabel(label_ids[i % 3]);
-//     impl.AddLabel(label_ids[(i + 1) % 3]);
-//     memgraph::query::MultiPropsInitChecked(&impl, properties);
-//     db.StoreVertex(impl);
-//   }
-//   // load vertices from disk
-//   auto loaded_vertices = db.Vertices(dba);
-//   ASSERT_EQ(loaded_vertices.size(), 5);
-//   for (const auto &vertex_acc : loaded_vertices) {
-//     ASSERT_TRUE(gids.contains(vertex_acc.Gid().AsUint()));
-//     // labels
-//     auto labels = vertex_acc.Labels(memgraph::storage::View::OLD);
-//     ASSERT_EQ(labels->size(), 2);
-//     ASSERT_TRUE(std::all_of(labels->begin(), labels->end(), [&label_ids](const auto &label_id) {
-//       return std::find(label_ids.begin(), label_ids.end(), label_id) != label_ids.end();
-//     }));
-//     // check properties
-//     auto props = vertex_acc.Properties(memgraph::storage::View::OLD);
-//     ASSERT_FALSE(props.HasError());
-//     auto prop_name = vertex_acc.GetProperty(memgraph::storage::View::OLD, dba.NameToProperty("name"));
-//     auto prop_memory = vertex_acc.GetProperty(memgraph::storage::View::OLD, dba.NameToProperty("memory"));
-//     auto prop_price = vertex_acc.GetProperty(memgraph::storage::View::OLD, dba.NameToProperty("price"));
-//     auto prop_unexisting = vertex_acc.GetProperty(memgraph::storage::View::OLD, dba.NameToProperty("random"));
-//     ASSERT_TRUE(prop_name->IsString());
-//     ASSERT_EQ(prop_name->ValueString(), "disk");
-//     ASSERT_TRUE(prop_memory->IsString());
-//     ASSERT_EQ(prop_memory->ValueString(), "1TB");
-//     ASSERT_TRUE(prop_price->IsDouble());
-//     ASSERT_DOUBLE_EQ(prop_price->ValueDouble(), 1000.21);
-//     ASSERT_TRUE(prop_unexisting->IsNull());
-//   }
-// }