From 4e5fe37dd6036bbedad940037972c66c687616e9 Mon Sep 17 00:00:00 2001 From: Matej Ferencevic Date: Tue, 9 Oct 2018 11:09:10 +0200 Subject: [PATCH] Remove virtual and pimpl from single node Summary: This diff removes: `SingleNodeRecoveryTransactions`, `TypemapPack` It also removes virtual and/or pimpl from: `SingleNodeCounters`, `StorageGcSingleNode`, `SingleNodeConcurrentIdMapper`, accessors (revert D1510), transaction engine, `GraphDbAccessor`, `GraphDb` Reviewers: msantl, teon.banek Reviewed By: msantl, teon.banek Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D1639 --- src/CMakeLists.txt | 2 +- src/database/{ => distributed}/counters.hpp | 0 .../distributed/distributed_counters.hpp | 2 +- .../distributed/distributed_graph_db.cpp | 2 +- src/database/distributed/graph_db.hpp | 6 +- ...{single_node_counters.hpp => counters.hpp} | 7 +- src/database/single_node/graph_db.cpp | 343 +++--------------- src/database/single_node/graph_db.hpp | 96 ++--- .../single_node/graph_db_accessor.hpp | 29 +- src/durability/single_node/recovery.cpp | 30 ++ src/durability/single_node/recovery.hpp | 21 +- src/memgraph.cpp | 2 +- src/mvcc/distributed/record.hpp | 2 +- src/mvcc/single_node/record.hpp | 2 +- .../concurrent_id_mapper.hpp | 0 .../concurrent_id_mapper_single_node.hpp | 2 +- .../concurrent_id_mapper_worker.hpp | 2 +- src/storage/distributed/garbage_collector.hpp | 2 +- src/storage/distributed/storage_gc.hpp | 2 +- ...ngle_node.hpp => concurrent_id_mapper.hpp} | 11 +- src/storage/single_node/edge_accessor.cpp | 4 +- src/storage/single_node/garbage_collector.hpp | 2 +- src/storage/single_node/record_accessor.cpp | 26 +- src/storage/single_node/record_accessor.hpp | 70 +--- src/storage/single_node/storage_gc.hpp | 12 +- .../single_node/storage_gc_single_node.hpp | 22 -- src/storage/single_node/vertex_accessor.cpp | 28 +- src/storage/single_node/vertex_accessor.hpp | 12 - src/transactions/distributed/engine.hpp | 111 ++++++ .../distributed/engine_distributed.hpp | 2 +- .../distributed/engine_single_node.hpp | 2 +- src/transactions/engine.hpp | 114 +----- .../{engine_single_node.cpp => engine.cpp} | 35 +- src/transactions/single_node/engine.hpp | 71 ++++ .../single_node/engine_single_node.hpp | 56 --- tests/benchmark/expansion.cpp | 2 +- tests/benchmark/mvcc.cpp | 4 +- tests/benchmark/query/planner.cpp | 6 +- tests/benchmark/tx_engine.cpp | 4 +- tests/feature_benchmark/kafka/benchmark.cpp | 2 +- tests/manual/query_planner.cpp | 2 +- tests/manual/repl.cpp | 2 +- tests/manual/single_query.cpp | 2 +- tests/property_based/random_graph.cpp | 2 +- tests/unit/bfs_single_node.cpp | 2 +- tests/unit/bolt_encoder.cpp | 2 +- .../unit/concurrent_id_mapper_single_node.cpp | 6 +- tests/unit/cypher_main_visitor.cpp | 2 +- tests/unit/database_key_index.cpp | 18 +- tests/unit/database_label_property_index.cpp | 14 +- tests/unit/database_transaction_timeout.cpp | 2 +- tests/unit/durability.cpp | 62 ++-- tests/unit/graph_db.cpp | 2 +- tests/unit/graph_db_accessor.cpp | 26 +- tests/unit/graph_db_accessor_index_api.cpp | 4 +- tests/unit/interpreter.cpp | 2 +- tests/unit/mvcc.cpp | 8 +- tests/unit/mvcc_find_update_common.hpp | 4 +- tests/unit/mvcc_gc.cpp | 6 +- tests/unit/mvcc_gc_common.hpp | 4 +- tests/unit/query_common.hpp | 2 +- tests/unit/query_cost_estimator.cpp | 2 +- tests/unit/query_expression_evaluator.cpp | 2 +- .../unit/query_plan_accumulate_aggregate.cpp | 20 +- tests/unit/query_plan_bag_semantics.cpp | 12 +- .../query_plan_create_set_remove_delete.cpp | 62 ++-- tests/unit/query_plan_edge_cases.cpp | 2 +- tests/unit/query_plan_match_filter_return.cpp | 62 ++-- tests/unit/query_semantic.cpp | 2 +- tests/unit/query_variable_start_planner.cpp | 18 +- tests/unit/record_edge_vertex_accessor.cpp | 24 +- tests/unit/state_delta.cpp | 16 +- tests/unit/transaction_engine_single_node.cpp | 12 +- tests/unit/typed_value.cpp | 2 +- tools/tests/mg_recovery_check.cpp | 4 +- 75 files changed, 661 insertions(+), 901 deletions(-) rename src/database/{ => distributed}/counters.hpp (100%) rename src/database/single_node/{single_node_counters.hpp => counters.hpp} (77%) rename src/storage/{common => distributed}/concurrent_id_mapper.hpp (100%) rename src/storage/single_node/{concurrent_id_mapper_single_node.hpp => concurrent_id_mapper.hpp} (86%) delete mode 100644 src/storage/single_node/storage_gc_single_node.hpp create mode 100644 src/transactions/distributed/engine.hpp rename src/transactions/single_node/{engine_single_node.cpp => engine.cpp} (72%) create mode 100644 src/transactions/single_node/engine.hpp delete mode 100644 src/transactions/single_node/engine_single_node.hpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0de998382..df7584a9e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -46,7 +46,7 @@ set(mg_single_node_sources storage/common/property_value_store.cpp storage/single_node/record_accessor.cpp storage/single_node/vertex_accessor.cpp - transactions/single_node/engine_single_node.cpp + transactions/single_node/engine.cpp memgraph_init.cpp ) diff --git a/src/database/counters.hpp b/src/database/distributed/counters.hpp similarity index 100% rename from src/database/counters.hpp rename to src/database/distributed/counters.hpp diff --git a/src/database/distributed/distributed_counters.hpp b/src/database/distributed/distributed_counters.hpp index c83a83c5e..7ac12a6be 100644 --- a/src/database/distributed/distributed_counters.hpp +++ b/src/database/distributed/distributed_counters.hpp @@ -6,7 +6,7 @@ #include #include "data_structures/concurrent/concurrent_map.hpp" -#include "database/counters.hpp" +#include "database/distributed/counters.hpp" namespace communication::rpc { class Server; diff --git a/src/database/distributed/distributed_graph_db.cpp b/src/database/distributed/distributed_graph_db.cpp index f964c0b98..b33ba2d2a 100644 --- a/src/database/distributed/distributed_graph_db.cpp +++ b/src/database/distributed/distributed_graph_db.cpp @@ -25,7 +25,7 @@ #include "durability/distributed/snapshooter.hpp" // TODO: Why do we depend on query here? #include "query/exceptions.hpp" -#include "storage/common/concurrent_id_mapper.hpp" +#include "storage/distributed/concurrent_id_mapper.hpp" #include "storage/distributed/concurrent_id_mapper_master.hpp" #include "storage/distributed/concurrent_id_mapper_worker.hpp" #include "storage/distributed/storage_gc_master.hpp" diff --git a/src/database/distributed/graph_db.hpp b/src/database/distributed/graph_db.hpp index a9d54ba01..6c3e8b575 100644 --- a/src/database/distributed/graph_db.hpp +++ b/src/database/distributed/graph_db.hpp @@ -5,16 +5,16 @@ #include #include -#include "database/counters.hpp" +#include "database/distributed/counters.hpp" #include "durability/distributed/recovery.hpp" #include "durability/distributed/wal.hpp" #include "io/network/endpoint.hpp" -#include "storage/common/concurrent_id_mapper.hpp" #include "storage/common/types.hpp" +#include "storage/distributed/concurrent_id_mapper.hpp" #include "storage/distributed/storage.hpp" #include "storage/distributed/storage_gc.hpp" #include "storage/distributed/vertex_accessor.hpp" -#include "transactions/engine.hpp" +#include "transactions/distributed/engine.hpp" #include "utils/scheduler.hpp" namespace database { diff --git a/src/database/single_node/single_node_counters.hpp b/src/database/single_node/counters.hpp similarity index 77% rename from src/database/single_node/single_node_counters.hpp rename to src/database/single_node/counters.hpp index 4e801868e..5cd681f2b 100644 --- a/src/database/single_node/single_node_counters.hpp +++ b/src/database/single_node/counters.hpp @@ -6,20 +6,19 @@ #include #include "data_structures/concurrent/concurrent_map.hpp" -#include "database/counters.hpp" namespace database { /// Implementation for the single-node memgraph -class SingleNodeCounters : public Counters { +class Counters { public: - int64_t Get(const std::string &name) override { + int64_t Get(const std::string &name) { return counters_.access() .emplace(name, std::make_tuple(name), std::make_tuple(0)) .first->second.fetch_add(1); } - void Set(const std::string &name, int64_t value) override { + void Set(const std::string &name, int64_t value) { auto name_counter_pair = counters_.access().emplace( name, std::make_tuple(name), std::make_tuple(value)); if (!name_counter_pair.second) name_counter_pair.first->second.store(value); diff --git a/src/database/single_node/graph_db.cpp b/src/database/single_node/graph_db.cpp index bda854ab3..1fc079656 100644 --- a/src/database/single_node/graph_db.cpp +++ b/src/database/single_node/graph_db.cpp @@ -4,220 +4,26 @@ #include +#include "database/single_node/counters.hpp" #include "database/single_node/graph_db_accessor.hpp" -#include "database/single_node/single_node_counters.hpp" #include "durability/paths.hpp" #include "durability/single_node/recovery.hpp" #include "durability/single_node/snapshooter.hpp" -#include "storage/single_node/concurrent_id_mapper_single_node.hpp" -#include "storage/single_node/storage_gc_single_node.hpp" -#include "transactions/single_node/engine_single_node.hpp" +#include "storage/single_node/concurrent_id_mapper.hpp" +#include "storage/single_node/storage_gc.hpp" +#include "transactions/single_node/engine.hpp" #include "utils/file.hpp" namespace database { -namespace { - -////////////////////////////////////////////////////////////////////// -// RecordAccessor and GraphDbAccessor implementations -////////////////////////////////////////////////////////////////////// - -template -class SingleNodeRecordAccessor final { - public: - typename RecordAccessor::AddressT GlobalAddress( - const RecordAccessor &record_accessor) { - // TODO: This is still coupled to distributed storage, albeit loosely. - int worker_id = 0; - CHECK(record_accessor.is_local()); - return storage::Address>(record_accessor.gid(), - worker_id); - } - - void SetOldNew(const RecordAccessor &record_accessor, TRecord **old, - TRecord **newr) { - auto &dba = record_accessor.db_accessor(); - const auto &address = record_accessor.address(); - CHECK(record_accessor.is_local()); - address.local()->find_set_old_new(dba.transaction(), old, newr); - } - - TRecord *FindNew(const RecordAccessor &record_accessor) { - const auto &address = record_accessor.address(); - auto &dba = record_accessor.db_accessor(); - CHECK(address.is_local()); - return address.local()->update(dba.transaction()); - } - - void ProcessDelta(const RecordAccessor &record_accessor, - const database::StateDelta &delta) { - CHECK(record_accessor.is_local()); - record_accessor.db_accessor().wal().Emplace(delta); - } - - int64_t CypherId(const RecordAccessor &record_accessor) { - return record_accessor.address().local()->cypher_id(); - } -}; - -class VertexAccessorImpl final : public ::VertexAccessor::Impl { - SingleNodeRecordAccessor accessor_; - - public: - typename RecordAccessor::AddressT GlobalAddress( - const RecordAccessor &ra) override { - return accessor_.GlobalAddress(ra); - } - - void SetOldNew(const RecordAccessor &ra, Vertex **old_record, - Vertex **new_record) override { - return accessor_.SetOldNew(ra, old_record, new_record); - } - - Vertex *FindNew(const RecordAccessor &ra) override { - return accessor_.FindNew(ra); - } - - void ProcessDelta(const RecordAccessor &ra, - const database::StateDelta &delta) override { - return accessor_.ProcessDelta(ra, delta); - } - - void AddLabel(const VertexAccessor &va, - const storage::Label &label) override { - CHECK(va.is_local()); - auto &dba = va.db_accessor(); - auto delta = StateDelta::AddLabel(dba.transaction_id(), va.gid(), label, - dba.LabelName(label)); - Vertex &vertex = va.update(); - // not a duplicate label, add it - if (!utils::Contains(vertex.labels_, label)) { - vertex.labels_.emplace_back(label); - dba.wal().Emplace(delta); - dba.UpdateLabelIndices(label, va, &vertex); - } - } - - void RemoveLabel(const VertexAccessor &va, - const storage::Label &label) override { - CHECK(va.is_local()); - auto &dba = va.db_accessor(); - auto delta = StateDelta::RemoveLabel(dba.transaction_id(), va.gid(), label, - dba.LabelName(label)); - Vertex &vertex = va.update(); - if (utils::Contains(vertex.labels_, label)) { - auto &labels = vertex.labels_; - auto found = std::find(labels.begin(), labels.end(), delta.label); - std::swap(*found, labels.back()); - labels.pop_back(); - dba.wal().Emplace(delta); - } - } - - int64_t CypherId(const RecordAccessor &ra) override { - return accessor_.CypherId(ra); - } -}; - -class EdgeAccessorImpl final : public ::RecordAccessor::Impl { - SingleNodeRecordAccessor accessor_; - - public: - typename RecordAccessor::AddressT GlobalAddress( - const RecordAccessor &ra) override { - return accessor_.GlobalAddress(ra); - } - - void SetOldNew(const RecordAccessor &ra, Edge **old_record, - Edge **new_record) override { - return accessor_.SetOldNew(ra, old_record, new_record); - } - - Edge *FindNew(const RecordAccessor &ra) override { - return accessor_.FindNew(ra); - } - - void ProcessDelta(const RecordAccessor &ra, - const database::StateDelta &delta) override { - return accessor_.ProcessDelta(ra, delta); - } - - int64_t CypherId(const RecordAccessor &ra) override { - return accessor_.CypherId(ra); - } -}; - -class SingleNodeAccessor : public GraphDbAccessor { - // Shared implementations of record accessors. - static VertexAccessorImpl vertex_accessor_; - static EdgeAccessorImpl edge_accessor_; - - public: - explicit SingleNodeAccessor(GraphDb &db) : GraphDbAccessor(db) {} - SingleNodeAccessor(GraphDb &db, tx::TransactionId tx_id) - : GraphDbAccessor(db, tx_id) {} - - ::VertexAccessor::Impl *GetVertexImpl() override { return &vertex_accessor_; } - - ::RecordAccessor::Impl *GetEdgeImpl() override { - return &edge_accessor_; - } -}; - -VertexAccessorImpl SingleNodeAccessor::vertex_accessor_; -EdgeAccessorImpl SingleNodeAccessor::edge_accessor_; - -} // namespace - -////////////////////////////////////////////////////////////////////// -// SingleNode GraphDb implementation -////////////////////////////////////////////////////////////////////// - -template