From 55d8b98aebec18f0cd2397a014091ede452e548f Mon Sep 17 00:00:00 2001 From: Matej Ferencevic Date: Fri, 13 Sep 2019 11:18:17 +0200 Subject: [PATCH] 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 | 14 ++++---------- src/storage/v2/storage.hpp | 28 +++++++++------------------- tests/benchmark/storage_v2_gc.cpp | 14 ++++++-------- tests/unit/storage_v2_gc.cpp | 8 ++++---- 4 files changed, 23 insertions(+), 41 deletions(-) diff --git a/src/storage/v2/storage.cpp b/src/storage/v2/storage.cpp index 1882c2f34..4a90fce72 100644 --- a/src/storage/v2/storage.cpp +++ b/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) { - if (gc_config.type == StorageGcConfig::Type::PERIODIC) { - gc_runner_.Run("Storage GC", gc_config.interval, +Storage::Storage(Config config) : config_(config) { + if (config_.gc_type == Config::GcType::PERIODIC) { + 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 { diff --git a/src/storage/v2/storage.hpp b/src/storage/v2/storage.hpp index 8ffd1f843..151b330ce 100644 --- a/src/storage/v2/storage.hpp +++ b/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 -/// garbage control. -/// -/// There are three options: -// 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; -}; +/// Pass this class to the \ref Storage constructor to change the behavior of +/// the storage. This class also defines the default behavior. +struct Config { + enum class GcType { NONE, PERIODIC }; -inline static constexpr StorageGcConfig DefaultGcConfig = { - .type = StorageGcConfig::Type::PERIODIC, - .interval = std::chrono::milliseconds(1000)}; + GcType gc_type{GcType::PERIODIC}; + std::chrono::milliseconds gc_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, utils::SpinLock> committed_transactions_; - StorageGcConfig gc_config_; + Config config_; utils::Scheduler gc_runner_; std::mutex gc_lock_; diff --git a/tests/benchmark/storage_v2_gc.cpp b/tests/benchmark/storage_v2_gc.cpp index b52fc0e36..525cd7150 100644 --- a/tests/benchmark/storage_v2_gc.cpp +++ b/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 TestConfigurations[] = { - {"NoGc", - storage::StorageGcConfig{.type = storage::StorageGcConfig::Type::NONE}}, +std::pair TestConfigurations[] = { + {"NoGc", storage::Config{.gc_type = storage::Config::GcType::NONE}}, {"100msPeriodicGc", - storage::StorageGcConfig{.type = storage::StorageGcConfig::Type::PERIODIC, - .interval = std::chrono::milliseconds(100)}}, + storage::Config{.gc_type = storage::Config::GcType::PERIODIC, + .gc_interval = std::chrono::milliseconds(100)}}, {"1000msPeriodicGc", - - storage::StorageGcConfig{.type = storage::StorageGcConfig::Type::PERIODIC, - .interval = std::chrono::milliseconds(1000)}}}; + storage::Config{.gc_type = storage::Config::GcType::PERIODIC, + .gc_interval = std::chrono::milliseconds(1000)}}}; void UpdateLabelFunc(int thread_id, storage::Storage *storage, const std::vector &vertices, diff --git a/tests/unit/storage_v2_gc.cpp b/tests/unit/storage_v2_gc.cpp index 85c453091..e3e4167fb 100644 --- a/tests/unit/storage_v2_gc.cpp +++ b/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, - .interval = std::chrono::milliseconds(100)}); + storage::Config{.gc_type = storage::Config::GcType::PERIODIC, + .gc_interval = std::chrono::milliseconds(100)}); std::vector 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, - .interval = std::chrono::milliseconds(100)}); + storage::Config{.gc_type = storage::Config::GcType::PERIODIC, + .gc_interval = std::chrono::milliseconds(100)}); { auto acc0 = storage.Access();