Up db info test limit

src/memory/global_memory_control.cpp

@@ -122,11 +122,11 @@ static bool my_commit(extent_hooks_t *extent_hooks, void *addr, size_t size, siz
 
   [[maybe_unused]] auto blocker = memgraph::utils::MemoryTracker::OutOfMemoryExceptionBlocker{};
   if (GetQueriesMemoryControl().IsThreadTracked()) [[unlikely]] {
-    [[maybe_unused]] bool ok = GetQueriesMemoryControl().TrackAllocOnCurrentThread(length);
+    bool ok = GetQueriesMemoryControl().TrackAllocOnCurrentThread(length);
     DMG_ASSERT(ok);
   }
 
-  [[maybe_unused]] auto ok = memgraph::utils::total_memory_tracker.Alloc(static_cast<int64_t>(length));
+  auto ok = memgraph::utils::total_memory_tracker.Alloc(static_cast<int64_t>(length));
   DMG_ASSERT(ok);
 
   return false;

src/mg_import_csv.cpp

@@ -416,7 +416,7 @@ memgraph::storage::PropertyValue StringToValue(const std::string &str, const std
 std::string GetIdSpace(const std::string &type) {
   // The format of this field is as follows:
   // [START_|END_]ID[(<id_space>)]
-  static std::regex format(R"(^(START_|END_)?ID(\(([^\(\)]+)\))?$)", std::regex::extended);
+  std::regex format(R"(^(START_|END_)?ID(\(([^\(\)]+)\))?$)", std::regex::extended);
   std::smatch res;
   if (!std::regex_match(type, res, format))
     throw LoadException(

src/query/procedure/mg_procedure_impl.cpp

@@ -3798,7 +3798,7 @@ void PrintFuncSignature(const mgp_func &func, std::ostream &stream) {
 
 bool IsValidIdentifierName(const char *name) {
   if (!name) return false;
-  static std::regex regex("[_[:alpha:]][_[:alnum:]]*");
+  std::regex regex("[_[:alpha:]][_[:alnum:]]*");
   return std::regex_match(name, regex);
 }
 

src/storage/v2/delta.hpp

@@ -123,26 +123,6 @@ inline bool operator==(const PreviousPtr::Pointer &a, const PreviousPtr::Pointer
 
 inline bool operator!=(const PreviousPtr::Pointer &a, const PreviousPtr::Pointer &b) { return !(a == b); }
 
-struct opt_str {
-  opt_str(std::optional<std::string> const &other) : str_{other ? new_cstr(*other) : nullptr} {}
-
-  ~opt_str() { delete[] str_; }
-
-  auto as_opt_str() const -> std::optional<std::string> {
-    if (!str_) return std::nullopt;
-    return std::optional<std::string>{std::in_place, str_};
-  }
-
- private:
-  static auto new_cstr(std::string const &str) -> char const * {
-    auto *mem = new char[str.length() + 1];
-    strcpy(mem, str.c_str());
-    return mem;
-  }
-
-  char const *str_ = nullptr;
-};
-
 struct Delta {
   enum class Action : std::uint8_t {
     /// Use for Vertex and Edge
@@ -180,7 +160,7 @@ struct Delta {
   // Because of this object was created in past txs, we create timestamp by ourselves inside instead of having it from
   // current tx. This timestamp we got from RocksDB timestamp stored in key.
   Delta(DeleteDeserializedObjectTag /*tag*/, uint64_t ts, std::optional<std::string> old_disk_key)
-      : timestamp(new std::atomic<uint64_t>(ts)), command_id(0), old_disk_key{.value = old_disk_key} {}
+      : timestamp(new std::atomic<uint64_t>(ts)), command_id(0), old_disk_key{.value = std::move(old_disk_key)} {}
 
   Delta(DeleteObjectTag /*tag*/, std::atomic<uint64_t> *timestamp, uint64_t command_id)
       : timestamp(timestamp), command_id(command_id), action(Action::DELETE_OBJECT) {}
@@ -242,7 +222,7 @@ struct Delta {
       case Action::REMOVE_OUT_EDGE:
         break;
       case Action::DELETE_DESERIALIZED_OBJECT:
-        std::destroy_at(&old_disk_key.value);
+        old_disk_key.value.reset();
         delete timestamp;
         timestamp = nullptr;
         break;
@@ -262,7 +242,7 @@ struct Delta {
     Action action;
     struct {
       Action action = Action::DELETE_DESERIALIZED_OBJECT;
-      opt_str value;
+      std::optional<std::string> value;
     } old_disk_key;
     struct {
       Action action;

src/storage/v2/disk/storage.hpp

@@ -310,7 +310,7 @@ class DiskStorage final : public Storage {
   StorageInfo GetBaseInfo() override;
   StorageInfo GetInfo(memgraph::replication_coordination_glue::ReplicationRole replication_role) override;
 
-  void FreeMemory(std::unique_lock<utils::ResourceLock> /*lock*/, bool /*periodic*/) override {}
+  void FreeMemory(std::unique_lock<utils::ResourceLock> /*lock*/) override {}
 
   void PrepareForNewEpoch() override { throw utils::BasicException("Disk storage mode does not support replication."); }
 

src/storage/v2/indices/indices.cpp

@@ -32,13 +32,10 @@ void Indices::AbortEntries(LabelId label, std::span<std::pair<PropertyValue, Ver
       ->AbortEntries(label, vertices, exact_start_timestamp);
 }
 
-void Indices::RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token,
-                                    std::optional<utils::BloomFilter<Vertex *>> filter) const {
-  auto const *filter_ptr = filter ? &*filter : nullptr;
-  static_cast<InMemoryLabelIndex *>(label_index_.get())
-      ->RemoveObsoleteEntries(oldest_active_start_timestamp, token, filter_ptr);
+void Indices::RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token) const {
+  static_cast<InMemoryLabelIndex *>(label_index_.get())->RemoveObsoleteEntries(oldest_active_start_timestamp, token);
   static_cast<InMemoryLabelPropertyIndex *>(label_property_index_.get())
-      ->RemoveObsoleteEntries(oldest_active_start_timestamp, std::move(token), filter_ptr);
+      ->RemoveObsoleteEntries(oldest_active_start_timestamp, std::move(token));
 }
 
 void Indices::UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transaction &tx) const {

src/storage/v2/indices/indices.hpp

@@ -33,8 +33,7 @@ struct Indices {
   /// This function should be called from garbage collection to clean-up the
   /// index.
   /// TODO: unused in disk indices
-  void RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token,
-                             std::optional<utils::BloomFilter<Vertex *>> filter) const;
+  void RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token) const;
 
   /// Surgical removal of entries that was inserted this transaction
   /// TODO: unused in disk indices

src/storage/v2/inmemory/label_index.cpp

@@ -80,34 +80,31 @@ std::vector<LabelId> InMemoryLabelIndex::ListIndices() const {
   return ret;
 }
 
-void InMemoryLabelIndex::RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token,
-                                               utils::BloomFilter<Vertex *> const *filter) {
+void InMemoryLabelIndex::RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token) {
   auto maybe_stop = utils::ResettableCounter<2048>();
 
-  for (auto &[label_id, index] : index_) {
+  for (auto &label_storage : index_) {
     // before starting index, check if stop_requested
     if (token.stop_requested()) return;
 
-    auto index_acc = index.access();
-    auto it = index_acc.begin();
-    auto end_it = index_acc.end();
-    if (it == end_it) continue;
-    while (true) {
+    auto vertices_acc = label_storage.second.access();
+    for (auto it = vertices_acc.begin(); it != vertices_acc.end();) {
       // Hot loop, don't check stop_requested every time
       if (maybe_stop() && token.stop_requested()) return;
 
       auto next_it = it;
       ++next_it;
 
-      bool has_next = next_it != end_it;
-      if (it->timestamp < oldest_active_start_timestamp) {
-        bool redundant_duplicate = has_next && it->vertex == next_it->vertex;
-        if (redundant_duplicate || ((!filter || filter->maybe_contains(it->vertex)) &&
-                                    !AnyVersionHasLabel(*it->vertex, label_id, oldest_active_start_timestamp))) {
-          index_acc.remove(*it);
-        }
+      if (it->timestamp >= oldest_active_start_timestamp) {
+        it = next_it;
+        continue;
       }
-      if (!has_next) break;
+
+      if ((next_it != vertices_acc.end() && it->vertex == next_it->vertex) ||
+          !AnyVersionHasLabel(*it->vertex, label_storage.first, oldest_active_start_timestamp)) {
+        vertices_acc.remove(*it);
+      }
+
       it = next_it;
     }
   }

src/storage/v2/inmemory/label_index.hpp

@@ -54,8 +54,7 @@ class InMemoryLabelIndex : public storage::LabelIndex {
 
   std::vector<LabelId> ListIndices() const override;
 
-  void RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token,
-                             utils::BloomFilter<Vertex *> const *filter);
+  void RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token);
 
   /// Surgical removal of entries that was inserted this transaction
   void AbortEntries(LabelId labelId, std::span<Vertex *const> vertices, uint64_t exact_start_timestamp);

src/storage/v2/inmemory/label_property_index.cpp

@@ -140,37 +140,31 @@ std::vector<std::pair<LabelId, PropertyId>> InMemoryLabelPropertyIndex::ListIndi
   return ret;
 }
 
-void InMemoryLabelPropertyIndex::RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token,
-                                                       utils::BloomFilter<Vertex *> const *filter) {
+void InMemoryLabelPropertyIndex::RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token) {
   auto maybe_stop = utils::ResettableCounter<2048>();
 
   for (auto &[label_property, index] : index_) {
     // before starting index, check if stop_requested
     if (token.stop_requested()) return;
 
-    auto [label_id, prop_id] = label_property;
-
     auto index_acc = index.access();
-    auto it = index_acc.begin();
-    auto end_it = index_acc.end();
-    if (it == end_it) continue;
-    while (true) {
+    for (auto it = index_acc.begin(); it != index_acc.end();) {
       // Hot loop, don't check stop_requested every time
       if (maybe_stop() && token.stop_requested()) return;
 
       auto next_it = it;
       ++next_it;
 
-      bool has_next = next_it != end_it;
-      if (it->timestamp < oldest_active_start_timestamp) {
-        bool redundant_duplicate = has_next && it->vertex == next_it->vertex && it->value == next_it->value;
-        if (redundant_duplicate ||
-            ((!filter || filter->maybe_contains(it->vertex)) &&
-             !AnyVersionHasLabelProperty(*it->vertex, label_id, prop_id, it->value, oldest_active_start_timestamp))) {
-          index_acc.remove(*it);
-        }
+      if (it->timestamp >= oldest_active_start_timestamp) {
+        it = next_it;
+        continue;
+      }
+
+      if ((next_it != index_acc.end() && it->vertex == next_it->vertex && it->value == next_it->value) ||
+          !AnyVersionHasLabelProperty(*it->vertex, label_property.first, label_property.second, it->value,
+                                      oldest_active_start_timestamp)) {
+        index_acc.remove(*it);
       }
-      if (!has_next) break;
       it = next_it;
     }
   }

src/storage/v2/inmemory/label_property_index.hpp

@@ -60,8 +60,7 @@ class InMemoryLabelPropertyIndex : public storage::LabelPropertyIndex {
 
   std::vector<std::pair<LabelId, PropertyId>> ListIndices() const override;
 
-  void RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token,
-                             utils::BloomFilter<Vertex *> const *filter);
+  void RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp, std::stop_token token);
 
   void AbortEntries(PropertyId property, std::span<std::pair<PropertyValue, Vertex *> const> vertices,
                     uint64_t exact_start_timestamp);

src/storage/v2/inmemory/storage.cpp

@@ -143,7 +143,9 @@ InMemoryStorage::InMemoryStorage(Config config)
 
   if (config_.gc.type == Config::Gc::Type::PERIODIC) {
     // TODO: move out of storage have one global gc_runner_
-    gc_runner_.Run("Storage GC", config_.gc.interval, [this] { this->FreeMemory({}, true); });
+    gc_runner_.Run("Storage GC", config_.gc.interval, [this] {
+      this->FreeMemory(std::unique_lock<utils::ResourceLock>{main_lock_, std::defer_lock});
+    });
   }
   if (timestamp_ == kTimestampInitialId) {
     commit_log_.emplace();
@@ -889,7 +891,6 @@ void InMemoryStorage::InMemoryAccessor::FastDiscardOfDeltas(uint64_t oldest_acti
   auto *mem_storage = static_cast<InMemoryStorage *>(storage_);
   std::list<Gid> current_deleted_edges;
   std::list<Gid> current_deleted_vertices;
-  auto index_invalidator = utils::BloomFilter<Vertex *>{};
 
   auto const unlink_remove_clear = [&](std::deque<Delta> &deltas) {
     for (auto &delta : deltas) {
@@ -939,26 +940,22 @@ void InMemoryStorage::InMemoryAccessor::FastDiscardOfDeltas(uint64_t oldest_acti
   // 1.b.1) unlink, gathering the removals
   for (auto &gc_deltas : linked_undo_buffers) {
     unlink_remove_clear(gc_deltas.deltas_);
-    index_invalidator.merge(std::move(gc_deltas.index_invalidator));
   }
   // 1.b.2) clear the list of deltas deques
   linked_undo_buffers.clear();
 
   // STEP 2) this transactions deltas also mininal unlinking + remove + clear
   unlink_remove_clear(transaction_.deltas);
-  index_invalidator.merge(std::move(transaction_.index_invalidator));
 
   // STEP 3) skip_list removals
-  if (!index_invalidator.empty()) {
+  if (!current_deleted_vertices.empty()) {
     // 3.a) clear from indexes first
     std::stop_source dummy;
-    mem_storage->indices_.RemoveObsoleteEntries(oldest_active_timestamp, dummy.get_token(), index_invalidator);
+    mem_storage->indices_.RemoveObsoleteEntries(oldest_active_timestamp, dummy.get_token());
     auto *mem_unique_constraints =
         static_cast<InMemoryUniqueConstraints *>(mem_storage->constraints_.unique_constraints_.get());
     mem_unique_constraints->RemoveObsoleteEntries(oldest_active_timestamp, dummy.get_token());
-  }
 
-  if (!current_deleted_vertices.empty()) {
     // 3.b) remove from veretex skip_list
     auto vertex_acc = mem_storage->vertices_.access();
     for (auto gid : current_deleted_vertices) {
@@ -1183,7 +1180,7 @@ void InMemoryStorage::InMemoryAccessor::Abort() {
         engine_guard.unlock();
 
         garbage_undo_buffers.emplace_back(mark_timestamp, std::move(transaction_.deltas),
-                                          std::move(transaction_.commit_timestamp), utils::BloomFilter<Vertex *>{});
+                                          std::move(transaction_.commit_timestamp));
       });
 
       /// We MUST unlink (aka. remove) entries in indexes and constraints
@@ -1232,8 +1229,8 @@ void InMemoryStorage::InMemoryAccessor::FinalizeTransaction() {
       // Only hand over delta to be GC'ed if there was any deltas
       mem_storage->committed_transactions_.WithLock([&](auto &committed_transactions) {
         // using mark of 0 as GC will assign a mark_timestamp after unlinking
-        committed_transactions.emplace_back(0, std::move(transaction_.deltas), std::move(transaction_.commit_timestamp),
-                                            std::move(transaction_.index_invalidator));
+        committed_transactions.emplace_back(0, std::move(transaction_.deltas),
+                                            std::move(transaction_.commit_timestamp));
       });
     }
     commit_timestamp_.reset();
@@ -1428,27 +1425,28 @@ void InMemoryStorage::SetStorageMode(StorageMode new_storage_mode) {
     }
 
     storage_mode_ = new_storage_mode;
-    FreeMemory(std::move(main_guard), false);
+    FreeMemory(std::move(main_guard));
   }
 }
 
-template <bool aggressive = true>
-void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_guard, bool periodic) {
+template <bool force>
+void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_guard) {
   // NOTE: You do not need to consider cleanup of deleted object that occurred in
   // different storage modes within the same CollectGarbage call. This is because
   // SetStorageMode will ensure CollectGarbage is called before any new transactions
   // with the new storage mode can start.
 
   // SetStorageMode will pass its unique_lock of main_lock_. We will use that lock,
-  // as reacquiring the lock would cause deadlock. Otherwise, we need to get our own
+  // as reacquiring the lock would cause  deadlock. Otherwise, we need to get our own
   // lock.
   if (!main_guard.owns_lock()) {
-    if constexpr (aggressive) {
-      // We tried to be aggressive but we do not already have main lock continue as not aggressive
-      // Perf note: Do not try to get unique lock if it was not already passed in. GC maybe expensive,
-      // do not assume it is fast, unique lock will blocks all new storage transactions.
-      CollectGarbage<false>({}, periodic);
-      return;
+    if constexpr (force) {
+      // We take the unique lock on the main storage lock, so we can forcefully clean
+      // everything we can
+      if (!main_lock_.try_lock()) {
+        CollectGarbage<false>();
+        return;
+      }
     } else {
       // Because the garbage collector iterates through the indices and constraints
       // to clean them up, it must take the main lock for reading to make sure that
@@ -1460,24 +1458,17 @@ void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_
   }
 
   utils::OnScopeExit lock_releaser{[&] {
-    if (main_guard.owns_lock()) {
-      main_guard.unlock();
+    if (!main_guard.owns_lock()) {
+      if constexpr (force) {
+        main_lock_.unlock();
+      } else {
+        main_lock_.unlock_shared();
+      }
     } else {
-      main_lock_.unlock_shared();
+      main_guard.unlock();
     }
   }};
 
-  // Only one gc run at a time
-  std::unique_lock<std::mutex> gc_guard(gc_lock_, std::try_to_lock);
-  if (!gc_guard.owns_lock()) {
-    return;
-  }
-
-  // Diagnostic trace
-  spdlog::trace("Storage GC on '{}' started [{}]", name(), periodic ? "periodic" : "forced");
-  auto trace_on_exit = utils::OnScopeExit{
-      [&] { spdlog::trace("Storage GC on '{}' finished [{}]", name(), periodic ? "periodic" : "forced"); }};
-
   // Garbage collection must be performed in two phases. In the first phase,
   // deltas that won't be applied by any transaction anymore are unlinked from
   // the version chains. They cannot be deleted immediately, because there
@@ -1485,29 +1476,27 @@ void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_
   // chain traversal. They are instead marked for deletion and will be deleted
   // in the second GC phase in this GC iteration or some of the following
   // ones.
+  std::unique_lock<std::mutex> gc_guard(gc_lock_, std::try_to_lock);
+  if (!gc_guard.owns_lock()) {
+    return;
+  }
 
   uint64_t oldest_active_start_timestamp = commit_log_->OldestActive();
 
-  {
-    std::unique_lock<utils::SpinLock> guard(engine_lock_);
-    uint64_t mark_timestamp = timestamp_;  // a timestamp no active transaction can currently have
-
-    // Deltas from previous GC runs or from aborts can be cleaned up here
-    garbage_undo_buffers_.WithLock([&](auto &garbage_undo_buffers) {
-      guard.unlock();
-      if (aggressive or mark_timestamp == oldest_active_start_timestamp) {
-        // We know no transaction is active, it is safe to simply delete all the garbage undos
-        // Nothing can be reading them
-        garbage_undo_buffers.clear();
-      } else {
-        // garbage_undo_buffers is ordered, pop until we can't
-        while (!garbage_undo_buffers.empty() &&
-               garbage_undo_buffers.front().mark_timestamp_ <= oldest_active_start_timestamp) {
-          garbage_undo_buffers.pop_front();
-        }
+  // Deltas from previous GC runs or from aborts can be cleaned up here
+  garbage_undo_buffers_.WithLock([&](auto &garbage_undo_buffers) {
+    if constexpr (force) {
+      // if force is set to true we can simply delete all the leftover undos because
+      // no transaction is active
+      garbage_undo_buffers.clear();
+    } else {
+      // garbage_undo_buffers is ordered, pop until we can't
+      while (!garbage_undo_buffers.empty() &&
+             garbage_undo_buffers.front().mark_timestamp_ <= oldest_active_start_timestamp) {
+        garbage_undo_buffers.pop_front();
       }
-    });
-  }
+    }
+  });
 
   // We don't move undo buffers of unlinked transactions to garbage_undo_buffers
   // list immediately, because we would have to repeatedly take
@@ -1528,7 +1517,12 @@ void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_
   committed_transactions_.WithLock(
       [&](auto &committed_transactions) { committed_transactions.swap(linked_undo_buffers); });
 
-  auto index_invalidator = utils::BloomFilter<Vertex *>{};
+  // Flag that will be used to determine whether the Index GC should be run. It
+  // should be run when there were any items that were cleaned up (there were
+  // updates between this run of the GC and the previous run of the GC). This
+  // eliminates high CPU usage when the GC doesn't have to clean up anything.
+  bool run_index_cleanup = !linked_undo_buffers.empty() || !garbage_undo_buffers_->empty() || need_full_scan_vertices ||
+                           need_full_scan_edges;
 
   auto const end_linked_undo_buffers = linked_undo_buffers.end();
   for (auto linked_entry = linked_undo_buffers.begin(); linked_entry != end_linked_undo_buffers;) {
@@ -1670,8 +1664,7 @@ void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_
 
     // Now unlinked, move to unlinked_undo_buffers
     auto const to_move = linked_entry;
-    ++linked_entry;                                                  // advanced to next before we move the list node
-    index_invalidator.merge(std::move(to_move->index_invalidator));  // track potential invalidations
+    ++linked_entry;  // advanced to next before we move the list node
     unlinked_undo_buffers.splice(unlinked_undo_buffers.end(), linked_undo_buffers, to_move);
   }
 
@@ -1682,26 +1675,16 @@ void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_
     });
   }
 
-  // Flag that will be used to determine whether the Index GC should be run. It
-  // should be run when there were any items that were cleaned up (there were
-  // updates between this run of the GC and the previous run of the GC). This
-  // eliminates high CPU usage when the GC doesn't have to clean up anything.
-  bool force_index_cleanup = need_full_scan_vertices || need_full_scan_edges;
-
   // After unlinking deltas from vertices, we refresh the indices. That way
   // we're sure that none of the vertices from `current_deleted_vertices`
   // appears in an index, and we can safely remove the from the main storage
   // after the last currently active transaction is finished.
-  if (!index_invalidator.empty() || force_index_cleanup) {
+  if (run_index_cleanup) {
     // This operation is very expensive as it traverses through all of the items
     // in every index every time.
     auto token = stop_source.get_token();
     if (!token.stop_requested()) {
-      auto filter = std::optional<utils::BloomFilter<Vertex *>>{};
-      if (!force_index_cleanup) {
-        filter = std::move(index_invalidator);
-      }
-      indices_.RemoveObsoleteEntries(oldest_active_start_timestamp, token, std::move(filter));
+      indices_.RemoveObsoleteEntries(oldest_active_start_timestamp, token);
       auto *mem_unique_constraints = static_cast<InMemoryUniqueConstraints *>(constraints_.unique_constraints_.get());
       mem_unique_constraints->RemoveObsoleteEntries(oldest_active_start_timestamp, std::move(token));
     }
@@ -1711,8 +1694,7 @@ void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_
     std::unique_lock<utils::SpinLock> guard(engine_lock_);
     uint64_t mark_timestamp = timestamp_;  // a timestamp no active transaction can currently have
 
-    if (aggressive or mark_timestamp == oldest_active_start_timestamp) {
-      guard.unlock();
+    if (force or mark_timestamp == oldest_active_start_timestamp) {
       // if lucky, there are no active transactions, hence nothing looking at the deltas
       // remove them now
       unlinked_undo_buffers.clear();
@@ -1774,8 +1756,8 @@ void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_
 }
 
 // tell the linker he can find the CollectGarbage definitions here
-template void InMemoryStorage::CollectGarbage<true>(std::unique_lock<utils::ResourceLock> main_guard, bool periodic);
-template void InMemoryStorage::CollectGarbage<false>(std::unique_lock<utils::ResourceLock> main_guard, bool periodic);
+template void InMemoryStorage::CollectGarbage<true>(std::unique_lock<utils::ResourceLock>);
+template void InMemoryStorage::CollectGarbage<false>(std::unique_lock<utils::ResourceLock>);
 
 StorageInfo InMemoryStorage::GetBaseInfo() {
   StorageInfo info{};
@@ -2126,35 +2108,50 @@ void InMemoryStorage::AppendToWalDataDefinition(durability::StorageMetadataOpera
 
 utils::BasicResult<InMemoryStorage::CreateSnapshotError> InMemoryStorage::CreateSnapshot(
     memgraph::replication_coordination_glue::ReplicationRole replication_role) {
-  using memgraph::replication_coordination_glue::ReplicationRole;
-  if (replication_role == ReplicationRole::REPLICA) {
+  if (replication_role == memgraph::replication_coordination_glue::ReplicationRole::REPLICA) {
     return InMemoryStorage::CreateSnapshotError::DisabledForReplica;
   }
+  auto const &epoch = repl_storage_state_.epoch_;
+  auto snapshot_creator = [this, &epoch]() {
+    utils::Timer timer;
+    auto transaction = CreateTransaction(IsolationLevel::SNAPSHOT_ISOLATION, storage_mode_,
+                                         memgraph::replication_coordination_glue::ReplicationRole::MAIN);
+    durability::CreateSnapshot(this, &transaction, recovery_.snapshot_directory_, recovery_.wal_directory_, &vertices_,
+                               &edges_, uuid_, epoch, repl_storage_state_.history, &file_retainer_);
+    // Finalize snapshot transaction.
+    commit_log_->MarkFinished(transaction.start_timestamp);
+
+    memgraph::metrics::Measure(memgraph::metrics::SnapshotCreationLatency_us,
+                               std::chrono::duration_cast<std::chrono::microseconds>(timer.Elapsed()).count());
+  };
 
   std::lock_guard snapshot_guard(snapshot_lock_);
 
-  auto accessor = std::invoke([&]() {
-    if (storage_mode_ == StorageMode::IN_MEMORY_ANALYTICAL) {
-      // For analytical no other txn can be in play
-      return UniqueAccess(ReplicationRole::MAIN, IsolationLevel::SNAPSHOT_ISOLATION);
+  auto should_try_shared{true};
+  auto max_num_tries{10};
+  while (max_num_tries) {
+    if (should_try_shared) {
+      std::shared_lock storage_guard(main_lock_);
+      if (storage_mode_ == memgraph::storage::StorageMode::IN_MEMORY_TRANSACTIONAL) {
+        snapshot_creator();
+        return {};
+      }
     } else {
-      return Access(ReplicationRole::MAIN, IsolationLevel::SNAPSHOT_ISOLATION);
+      std::unique_lock main_guard{main_lock_};
+      if (storage_mode_ == memgraph::storage::StorageMode::IN_MEMORY_ANALYTICAL) {
+        snapshot_creator();
+        return {};
+      }
     }
-  });
+    should_try_shared = !should_try_shared;
+    max_num_tries--;
+  }
 
-  utils::Timer timer;
-  Transaction *transaction = accessor->GetTransaction();
-  auto const &epoch = repl_storage_state_.epoch_;
-  durability::CreateSnapshot(this, transaction, recovery_.snapshot_directory_, recovery_.wal_directory_, &vertices_,
-                             &edges_, uuid_, epoch, repl_storage_state_.history, &file_retainer_);
-
-  memgraph::metrics::Measure(memgraph::metrics::SnapshotCreationLatency_us,
-                             std::chrono::duration_cast<std::chrono::microseconds>(timer.Elapsed()).count());
-  return {};
+  return CreateSnapshotError::ReachedMaxNumTries;
 }
 
-void InMemoryStorage::FreeMemory(std::unique_lock<utils::ResourceLock> main_guard, bool periodic) {
-  CollectGarbage(std::move(main_guard), periodic);
+void InMemoryStorage::FreeMemory(std::unique_lock<utils::ResourceLock> main_guard) {
+  CollectGarbage<true>(std::move(main_guard));
 
   static_cast<InMemoryLabelIndex *>(indices_.label_index_.get())->RunGC();
   static_cast<InMemoryLabelPropertyIndex *>(indices_.label_property_index_.get())->RunGC();

src/storage/v2/inmemory/storage.hpp

@@ -332,7 +332,7 @@ class InMemoryStorage final : public Storage {
   std::unique_ptr<Accessor> UniqueAccess(memgraph::replication_coordination_glue::ReplicationRole replication_role,
                                          std::optional<IsolationLevel> override_isolation_level) override;
 
-  void FreeMemory(std::unique_lock<utils::ResourceLock> main_guard, bool periodic) override;
+  void FreeMemory(std::unique_lock<utils::ResourceLock> main_guard) override;
 
   utils::FileRetainer::FileLockerAccessor::ret_type IsPathLocked();
   utils::FileRetainer::FileLockerAccessor::ret_type LockPath();
@@ -363,7 +363,7 @@ class InMemoryStorage final : public Storage {
   /// @throw std::system_error
   /// @throw std::bad_alloc
   template <bool force>
-  void CollectGarbage(std::unique_lock<utils::ResourceLock> main_guard, bool periodic);
+  void CollectGarbage(std::unique_lock<utils::ResourceLock> main_guard = {});
 
   bool InitializeWalFile(memgraph::replication::ReplicationEpoch &epoch);
   void FinalizeWalFile();
@@ -432,12 +432,8 @@ class InMemoryStorage final : public Storage {
   std::mutex gc_lock_;
 
   struct GCDeltas {
-    GCDeltas(uint64_t mark_timestamp, std::deque<Delta> deltas, std::unique_ptr<std::atomic<uint64_t>> commit_timestamp,
-             utils::BloomFilter<Vertex *> indexInvalidator)
-        : mark_timestamp_{mark_timestamp},
-          deltas_{std::move(deltas)},
-          commit_timestamp_{std::move(commit_timestamp)},
-          index_invalidator(std::move(indexInvalidator)) {}
+    GCDeltas(uint64_t mark_timestamp, std::deque<Delta> deltas, std::unique_ptr<std::atomic<uint64_t>> commit_timestamp)
+        : mark_timestamp_{mark_timestamp}, deltas_{std::move(deltas)}, commit_timestamp_{std::move(commit_timestamp)} {}
 
     GCDeltas(GCDeltas &&) = default;
     GCDeltas &operator=(GCDeltas &&) = default;
@@ -445,8 +441,6 @@ class InMemoryStorage final : public Storage {
     uint64_t mark_timestamp_{};                                  //!< a timestamp no active transaction currently has
     std::deque<Delta> deltas_;                                   //!< the deltas that need cleaning
     std::unique_ptr<std::atomic<uint64_t>> commit_timestamp_{};  //!< the timestamp the deltas are pointing at
-    utils::BloomFilter<Vertex *>
-        index_invalidator{};  //!< bloom filter of maybe Vertex * that invalidated one or more indexes
   };
 
   // Ownership of linked deltas is transferred to committed_transactions_ once transaction is commited

src/storage/v2/property_store.cpp

@@ -1051,14 +1051,6 @@ struct SpecificPropertyAndBufferInfo {
   uint64_t all_size;
 };
 
-// Struct used to return info about the property position
-struct SpecificPropertyAndBufferInfoMinimal {
-  uint64_t property_begin;
-  uint64_t property_end;
-
-  auto property_size() const { return property_end - property_begin; }
-};
-
 // Function used to find the position where the property should be in the data
 // buffer. It keeps the properties in the buffer sorted by `PropertyId` and
 // returns the positions in the buffer where the seeked property starts and
@@ -1091,27 +1083,6 @@ SpecificPropertyAndBufferInfo FindSpecificPropertyAndBufferInfo(Reader *reader,
   return {property_begin, property_end, property_end - property_begin, all_begin, all_end, all_end - all_begin};
 }
 
-// Like FindSpecificPropertyAndBufferInfo, but will early exit. No need to find the "all" information
-SpecificPropertyAndBufferInfoMinimal FindSpecificPropertyAndBufferInfoMinimal(Reader *reader, PropertyId property) {
-  uint64_t property_begin = reader->GetPosition();
-  while (true) {
-    switch (HasExpectedProperty(reader, property)) {
-      case ExpectedPropertyStatus::MISSING_DATA:
-        [[fallthrough]];
-      case ExpectedPropertyStatus::GREATER: {
-        return {0, 0};
-      }
-      case ExpectedPropertyStatus::EQUAL: {
-        return {property_begin, reader->GetPosition()};
-      }
-      case ExpectedPropertyStatus::SMALLER: {
-        property_begin = reader->GetPosition();
-        break;
-      }
-    }
-  }
-}
-
 // All data buffers will be allocated to a power of 8 size.
 uint64_t ToPowerOf8(uint64_t size) {
   uint64_t mod = size % 8;
@@ -1283,12 +1254,11 @@ bool PropertyStore::IsPropertyEqual(PropertyId property, const PropertyValue &va
   BufferInfo buffer_info = GetBufferInfo(buffer_);
   Reader reader(buffer_info.data, buffer_info.size);
 
-  auto info = FindSpecificPropertyAndBufferInfoMinimal(&reader, property);
-  auto property_size = info.property_size();
-  if (property_size == 0) return value.IsNull();
-  Reader prop_reader(buffer_info.data + info.property_begin, property_size);
+  auto info = FindSpecificPropertyAndBufferInfo(&reader, property);
+  if (info.property_size == 0) return value.IsNull();
+  Reader prop_reader(buffer_info.data + info.property_begin, info.property_size);
   if (!CompareExpectedProperty(&prop_reader, property, value)) return false;
-  return prop_reader.GetPosition() == property_size;
+  return prop_reader.GetPosition() == info.property_size;
 }
 
 std::map<PropertyId, PropertyValue> PropertyStore::Properties() const {

src/storage/v2/storage.cpp

@@ -275,10 +275,6 @@ Storage::Accessor::DetachDelete(std::vector<VertexAccessor *> nodes, std::vector
 
   auto deleted_vertices = maybe_deleted_vertices.GetValue();
 
-  for (auto const &vertex : deleted_vertices) {
-    transaction_.index_invalidator.insert(vertex.vertex_);
-  }
-
   return std::make_optional<ReturnType>(std::move(deleted_vertices), std::move(deleted_edges));
 }
 

src/storage/v2/storage.hpp

@@ -284,8 +284,6 @@ class Storage {
     virtual UniqueConstraints::DeletionStatus DropUniqueConstraint(LabelId label,
                                                                    const std::set<PropertyId> &properties) = 0;
 
-    auto GetTransaction() -> Transaction * { return std::addressof(transaction_); }
-
    protected:
     Storage *storage_;
     std::shared_lock<utils::ResourceLock> storage_guard_;
@@ -338,15 +336,9 @@ class Storage {
 
   StorageMode GetStorageMode() const noexcept;
 
-  virtual void FreeMemory(std::unique_lock<utils::ResourceLock> main_guard, bool periodic) = 0;
+  virtual void FreeMemory(std::unique_lock<utils::ResourceLock> main_guard) = 0;
 
-  void FreeMemory() {
-    if (storage_mode_ == StorageMode::IN_MEMORY_ANALYTICAL) {
-      FreeMemory(std::unique_lock{main_lock_}, false);
-    } else {
-      FreeMemory({}, false);
-    }
-  }
+  void FreeMemory() { FreeMemory({}); }
 
   virtual std::unique_ptr<Accessor> Access(memgraph::replication_coordination_glue::ReplicationRole replication_role,
                                            std::optional<IsolationLevel> override_isolation_level) = 0;

src/storage/v2/transaction.hpp

@@ -15,7 +15,6 @@
 #include <limits>
 #include <memory>
 
-#include "utils/bloom_filter.hpp"
 #include "utils/memory.hpp"
 #include "utils/skip_list.hpp"
 
@@ -90,7 +89,6 @@ struct Transaction {
   uint64_t command_id{};
 
   std::deque<Delta> deltas;
-  utils::BloomFilter<Vertex *> index_invalidator;
   utils::pmr::list<MetadataDelta> md_deltas;
   bool must_abort{};
   IsolationLevel isolation_level{};

src/storage/v2/vertex_accessor.cpp

@@ -123,7 +123,6 @@ Result<bool> VertexAccessor::AddLabel(LabelId label) {
   transaction_->constraint_verification_info.AddedLabel(vertex_);
   storage_->indices_.UpdateOnAddLabel(label, vertex_, *transaction_);
   transaction_->manyDeltasCache.Invalidate(vertex_, label);
-  transaction_->index_invalidator.insert(vertex_);
 
   return true;
 }
@@ -152,7 +151,6 @@ Result<bool> VertexAccessor::RemoveLabel(LabelId label) {
   storage_->constraints_.unique_constraints_->UpdateOnRemoveLabel(label, *vertex_, transaction_->start_timestamp);
   storage_->indices_.UpdateOnRemoveLabel(label, vertex_, *transaction_);
   transaction_->manyDeltasCache.Invalidate(vertex_, label);
-  transaction_->index_invalidator.insert(vertex_);
 
   return true;
 }
@@ -285,7 +283,6 @@ Result<PropertyValue> VertexAccessor::SetProperty(PropertyId property, const Pro
   }
   storage_->indices_.UpdateOnSetProperty(property, value, vertex_, *transaction_);
   transaction_->manyDeltasCache.Invalidate(vertex_, property);
-  transaction_->index_invalidator.insert(vertex_);
 
   return std::move(current_value);
 }
@@ -317,7 +314,6 @@ Result<bool> VertexAccessor::InitProperties(const std::map<storage::PropertyId,
           } else {
             transaction->constraint_verification_info.RemovedProperty(vertex);
           }
-          transaction->index_invalidator.insert(vertex);
         }
         result = true;
       }};
@@ -356,7 +352,6 @@ Result<std::vector<std::tuple<PropertyId, PropertyValue, PropertyValue>>> Vertex
           } else {
             transaction->constraint_verification_info.RemovedProperty(vertex);
           }
-          transaction->index_invalidator.insert(vertex);
         }
       }};
   std::invoke(atomic_memory_block);
@@ -389,7 +384,6 @@ Result<std::map<PropertyId, PropertyValue>> VertexAccessor::ClearProperties() {
           transaction->manyDeltasCache.Invalidate(vertex, property);
         }
         vertex->properties.ClearProperties();
-        transaction->index_invalidator.insert(vertex);
       }};
   std::invoke(atomic_memory_block);
 

src/utils/bloom_filter.hpp

@@ -1,45 +0,0 @@
-// Copyright 2024 Memgraph Ltd.
-//
-// Use of this software is governed by the Business Source License
-// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
-// License, and you may not use this file except in compliance with the Business Source License.
-//
-// As of the Change Date specified in that file, in accordance with
-// the Business Source License, use of this software will be governed
-// by the Apache License, Version 2.0, included in the file
-// licenses/APL.txt.
-
-#pragma once
-
-#include "absl/container/flat_hash_set.h"
-
-namespace memgraph::utils {
-
-template <typename T>
-struct BloomFilter {
-  void insert(T const &value) {
-    constexpr auto hasher = absl::Hash<T>{};
-    auto hash_1 = hasher(value);
-    auto hash_2 = hasher(value + 1987);
-    store.insert(hash_1);
-    store.insert(hash_2);
-  }
-
-  bool maybe_contains(T const &value) const {
-    constexpr auto hasher = absl::Hash<T>{};
-    auto hash_1 = hasher(value);
-    if (!store.contains(hash_1)) return false;
-    auto hash_2 = hasher(value + 1987);
-    return store.contains(hash_2);
-  }
-
-  void merge(BloomFilter &&other) { store.merge(std::move(other.store)); }
-
-  bool empty() const { return store.empty(); }
-
- private:
-  // Deliberate truncate to uint32_t
-  absl::flat_hash_set<uint32_t> store;  // TODO: replace with roaring bitmap?
-};
-
-}  // namespace memgraph::utils

src/utils/disk_utils.hpp

@@ -21,7 +21,7 @@ inline std::optional<std::string> GetOldDiskKeyOrNull(storage::Delta *head) {
     head = head->next;
   }
   if (head->action == storage::Delta::Action::DELETE_DESERIALIZED_OBJECT) {
-    return head->old_disk_key.value.as_opt_str();
+    return head->old_disk_key.value;
   }
   return std::nullopt;
 }

src/utils/resource_lock.hpp

@@ -66,17 +66,6 @@ struct ResourceLock {
     }
     return false;
   }
-
-  template <typename Rep, typename Period>
-  bool try_lock_shared_for(std::chrono::duration<Rep, Period> const &time) {
-    auto lock = std::unique_lock{mtx};
-    // block until available
-    if (!cv.wait_for(lock, time, [this] { return state != UNIQUE; })) return false;
-    state = SHARED;
-    ++count;
-    return true;
-  }
-
   void unlock() {
     auto lock = std::unique_lock{mtx};
     state = UNLOCKED;

src/utils/string.hpp

@@ -242,7 +242,7 @@ std::vector<TString, TAllocator> *Split(std::vector<TString, TAllocator> *out, c
   if (src.empty()) return out;
   // TODO: Investigate how much regex allocate and perhaps replace with custom
   // solution doing no allocations.
-  static std::regex not_whitespace("[^\\s]+");
+  std::regex not_whitespace("[^\\s]+");
   auto matches_begin = std::cregex_iterator(src.data(), src.data() + src.size(), not_whitespace);
   auto matches_end = std::cregex_iterator();
   out->reserve(std::distance(matches_begin, matches_end));

tests/unit/database_get_info.cpp

@@ -197,8 +197,8 @@ TYPED_TEST(InfoTest, InfoCheck) {
   ASSERT_EQ(info.storage_info.vertex_count, 5);
   ASSERT_EQ(info.storage_info.edge_count, 2);
   ASSERT_EQ(info.storage_info.average_degree, 0.8);
-  ASSERT_GT(info.storage_info.memory_res, 10'000'000);  // 250MB < > 10MB
-  ASSERT_LT(info.storage_info.memory_res, 250'000'000);
+  ASSERT_GT(info.storage_info.memory_res, 10'000'000);  // 300MB < > 10MB
+  ASSERT_LT(info.storage_info.memory_res, 300'000'000);
   ASSERT_GT(info.storage_info.disk_usage, 100);  // 1MB < > 100B
   ASSERT_LT(info.storage_info.disk_usage, 1000'000);
   ASSERT_EQ(info.storage_info.label_indices, 1);