Prepare storage v2 config for more options

Reviewers: teon.banek

Reviewed By: teon.banek

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2382

src/storage/v2/storage.cpp

@@ -290,15 +290,15 @@ bool VerticesIterable::Iterator::operator==(const Iterator &other) const {
   }
 }
 
-Storage::Storage(StorageGcConfig gc_config) : gc_config_(gc_config) {
+Storage::Storage(Config config) : config_(config) {
-  if (gc_config.type == StorageGcConfig::Type::PERIODIC) {
+  if (config_.gc_type == Config::GcType::PERIODIC) {
-    gc_runner_.Run("Storage GC", gc_config.interval,
+    gc_runner_.Run("Storage GC", config_.gc_interval,
                    [this] { this->CollectGarbage(); });
   }
 }
 
 Storage::~Storage() {
-  if (gc_config_.type == StorageGcConfig::Type::PERIODIC) {
+  if (config_.gc_type == Config::GcType::PERIODIC) {
     gc_runner_.Stop();
   }
 }
@@ -644,9 +644,6 @@ Storage::Accessor::Commit() {
     storage_->commit_log_.MarkFinished(commit_timestamp);
   }
   is_transaction_active_ = false;
-  if (storage_->gc_config_.type == StorageGcConfig::Type::ON_FINISH) {
-    storage_->CollectGarbage();
-  }
 
   return {};
 }
@@ -842,9 +839,6 @@ void Storage::Accessor::Abort() {
 
   storage_->commit_log_.MarkFinished(transaction_.start_timestamp);
   is_transaction_active_ = false;
-  if (storage_->gc_config_.type == StorageGcConfig::Type::ON_FINISH) {
-    storage_->CollectGarbage();
-  }
 }
 
 const std::string &Storage::LabelToName(LabelId label) const {

src/storage/v2/storage.hpp

@@ -29,24 +29,14 @@ namespace storage {
 const uint64_t kTimestampInitialId = 0;
 const uint64_t kTransactionInitialId = 1ULL << 63U;
 
-/// Pass this class to the \ref Storage constructor to set the behavior of the
+/// Pass this class to the \ref Storage constructor to change the behavior of
-/// garbage control.
+/// the storage. This class also defines the default behavior.
-///
+struct Config {
-/// There are three options:
+  enum class GcType { NONE, PERIODIC };
-//    1. NONE - No GC at all, only useful for benchmarking.
-//    2. PERIODIC - A separate thread performs GC periodically with given
-//                  interval (this is the default, with 1 second interval).
-//    3. ON_FINISH - Whenever a transaction commits or aborts, GC is performed
-//                   on the same thread.
-struct StorageGcConfig {
-  enum class Type { NONE, PERIODIC, ON_FINISH };
-  Type type;
-  std::chrono::milliseconds interval;
-};
 
-inline static constexpr StorageGcConfig DefaultGcConfig = {
+  GcType gc_type{GcType::PERIODIC};
-    .type = StorageGcConfig::Type::PERIODIC,
+  std::chrono::milliseconds gc_interval{std::chrono::milliseconds(1000)};
-    .interval = std::chrono::milliseconds(1000)};
+};
 
 /// Iterable for iterating through all vertices of a Storage.
 ///
@@ -168,7 +158,7 @@ class Storage final {
  public:
   /// @throw std::system_error
   /// @throw std::bad_alloc
-  explicit Storage(StorageGcConfig gc_config = DefaultGcConfig);
+  explicit Storage(Config config = Config());
 
   ~Storage();
 
@@ -400,7 +390,7 @@ class Storage final {
   utils::Synchronized<std::list<Transaction>, utils::SpinLock>
       committed_transactions_;
 
-  StorageGcConfig gc_config_;
+  Config config_;
   utils::Scheduler gc_runner_;
   std::mutex gc_lock_;
 

tests/benchmark/storage_v2_gc.cpp

@@ -15,16 +15,14 @@ DEFINE_int32(num_threads, 4, "number of threads");
 DEFINE_int32(num_vertices, kNumVertices, "number of vertices");
 DEFINE_int32(num_iterations, kNumIterations, "number of iterations");
 
-std::pair<std::string, storage::StorageGcConfig> TestConfigurations[] = {
+std::pair<std::string, storage::Config> TestConfigurations[] = {
-    {"NoGc",
+    {"NoGc", storage::Config{.gc_type = storage::Config::GcType::NONE}},
-     storage::StorageGcConfig{.type = storage::StorageGcConfig::Type::NONE}},
     {"100msPeriodicGc",
-     storage::StorageGcConfig{.type = storage::StorageGcConfig::Type::PERIODIC,
+     storage::Config{.gc_type = storage::Config::GcType::PERIODIC,
-                              .interval = std::chrono::milliseconds(100)}},
+                     .gc_interval = std::chrono::milliseconds(100)}},
     {"1000msPeriodicGc",
-
+     storage::Config{.gc_type = storage::Config::GcType::PERIODIC,
-     storage::StorageGcConfig{.type = storage::StorageGcConfig::Type::PERIODIC,
+                     .gc_interval = std::chrono::milliseconds(1000)}}};
-                              .interval = std::chrono::milliseconds(1000)}}};
 
 void UpdateLabelFunc(int thread_id, storage::Storage *storage,
                      const std::vector<storage::Gid> &vertices,

tests/unit/storage_v2_gc.cpp

@@ -14,8 +14,8 @@ using testing::UnorderedElementsAre;
 // NOLINTNEXTLINE(hicpp-special-member-functions)
 TEST(StorageV2Gc, Sanity) {
   storage::Storage storage(
-      storage::StorageGcConfig{.type = storage::StorageGcConfig::Type::PERIODIC,
+      storage::Config{.gc_type = storage::Config::GcType::PERIODIC,
-                               .interval = std::chrono::milliseconds(100)});
+                      .gc_interval = std::chrono::milliseconds(100)});
 
   std::vector<storage::Gid> vertices;
 
@@ -164,8 +164,8 @@ TEST(StorageV2Gc, Sanity) {
 // NOLINTNEXTLINE(hicpp-special-member-functions)
 TEST(StorageV2Gc, Indices) {
   storage::Storage storage(
-      storage::StorageGcConfig{.type = storage::StorageGcConfig::Type::PERIODIC,
+      storage::Config{.gc_type = storage::Config::GcType::PERIODIC,
-                               .interval = std::chrono::milliseconds(100)});
+                      .gc_interval = std::chrono::milliseconds(100)});
 
   {
     auto acc0 = storage.Access();