From 3b336e3e0b98a62476aa49764f6d3d81112b6deb Mon Sep 17 00:00:00 2001 From: antonio2368 Date: Wed, 30 Jun 2021 12:31:30 +0200 Subject: [PATCH] Add CREATE SNAPSHOT query (#182) --- src/auth/models.cpp | 4 +-- src/auth/models.hpp | 14 +++++----- src/glue/auth.cpp | 4 +-- src/query/exceptions.hpp | 6 +++++ src/query/frontend/ast/ast.lcp | 12 +++++++-- src/query/frontend/ast/ast_visitor.hpp | 4 ++- .../frontend/ast/cypher_main_visitor.cpp | 7 ++++- .../frontend/ast/cypher_main_visitor.hpp | 5 ++++ .../opencypher/grammar/MemgraphCypher.g4 | 5 +++- .../opencypher/grammar/MemgraphCypherLexer.g4 | 2 +- .../frontend/semantic/required_privileges.cpp | 4 ++- src/query/interpreter.cpp | 26 +++++++++++++++++++ src/storage/v2/storage.cpp | 26 +++++++++++++++---- src/storage/v2/storage.hpp | 7 +++-- tests/unit/cypher_main_visitor.cpp | 9 +++++-- tests/unit/query_required_privileges.cpp | 8 +++++- 16 files changed, 115 insertions(+), 28 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index cc34ca410..6d4f44a91 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -41,8 +41,8 @@ std::string PermissionToString(Permission permission) { return "DUMP"; case Permission::REPLICATION: return "REPLICATION"; - case Permission::LOCK_PATH: - return "LOCK_PATH"; + case Permission::DURABILITY: + return "DURABILITY"; case Permission::READ_FILE: return "READ_FILE"; case Permission::FREE_MEMORY: diff --git a/src/auth/models.hpp b/src/auth/models.hpp index 56cf897d2..3dc542d00 100644 --- a/src/auth/models.hpp +++ b/src/auth/models.hpp @@ -22,7 +22,7 @@ enum class Permission : uint64_t { CONSTRAINT = 1U << 8U, DUMP = 1U << 9U, REPLICATION = 1U << 10U, - LOCK_PATH = 1U << 11U, + DURABILITY = 1U << 11U, READ_FILE = 1U << 12U, FREE_MEMORY = 1U << 13U, TRIGGER = 1U << 14U, @@ -32,12 +32,12 @@ enum class Permission : uint64_t { // clang-format on // Constant list of all available permissions. -const std::vector kPermissionsAll = {Permission::MATCH, Permission::CREATE, Permission::MERGE, - Permission::DELETE, Permission::SET, Permission::REMOVE, - Permission::INDEX, Permission::STATS, Permission::CONSTRAINT, - Permission::DUMP, Permission::AUTH, Permission::REPLICATION, - Permission::LOCK_PATH, Permission::READ_FILE, Permission::FREE_MEMORY, - Permission::TRIGGER, Permission::CONFIG}; +const std::vector kPermissionsAll = {Permission::MATCH, Permission::CREATE, Permission::MERGE, + Permission::DELETE, Permission::SET, Permission::REMOVE, + Permission::INDEX, Permission::STATS, Permission::CONSTRAINT, + Permission::DUMP, Permission::AUTH, Permission::REPLICATION, + Permission::DURABILITY, Permission::READ_FILE, Permission::FREE_MEMORY, + Permission::TRIGGER, Permission::CONFIG}; // Function that converts a permission to its string representation. std::string PermissionToString(Permission permission); diff --git a/src/glue/auth.cpp b/src/glue/auth.cpp index fe55c6dde..b8a9dee7e 100644 --- a/src/glue/auth.cpp +++ b/src/glue/auth.cpp @@ -26,8 +26,8 @@ auth::Permission PrivilegeToPermission(query::AuthQuery::Privilege privilege) { return auth::Permission::DUMP; case query::AuthQuery::Privilege::REPLICATION: return auth::Permission::REPLICATION; - case query::AuthQuery::Privilege::LOCK_PATH: - return auth::Permission::LOCK_PATH; + case query::AuthQuery::Privilege::DURABILITY: + return auth::Permission::DURABILITY; case query::AuthQuery::Privilege::READ_FILE: return auth::Permission::READ_FILE; case query::AuthQuery::Privilege::FREE_MEMORY: diff --git a/src/query/exceptions.hpp b/src/query/exceptions.hpp index 6c757e91b..161d29112 100644 --- a/src/query/exceptions.hpp +++ b/src/query/exceptions.hpp @@ -181,4 +181,10 @@ class IsolationLevelModificationInMulticommandTxException : public QueryExceptio IsolationLevelModificationInMulticommandTxException() : QueryException("Isolation level cannot be modified in multicommand transactions.") {} }; + +class CreateSnapshotInMulticommandTxException final : public QueryException { + public: + CreateSnapshotInMulticommandTxException() + : QueryException("Snapshot cannot be created in multicommand transactions.") {} +}; } // namespace query diff --git a/src/query/frontend/ast/ast.lcp b/src/query/frontend/ast/ast.lcp index 81b6038e5..dd8515116 100644 --- a/src/query/frontend/ast/ast.lcp +++ b/src/query/frontend/ast/ast.lcp @@ -2193,7 +2193,7 @@ cpp<# (:serialize)) (lcp:define-enum privilege (create delete match merge set remove index stats auth constraint - dump replication lock_path read_file free_memory trigger config) + dump replication durability read_file free_memory trigger config) (:serialize)) #>cpp AuthQuery() = default; @@ -2231,7 +2231,7 @@ const std::vector kPrivilegesAll = { AuthQuery::Privilege::CONSTRAINT, AuthQuery::Privilege::DUMP, AuthQuery::Privilege::REPLICATION, AuthQuery::Privilege::READ_FILE, - AuthQuery::Privilege::LOCK_PATH, + AuthQuery::Privilege::DURABILITY, AuthQuery::Privilege::FREE_MEMORY, AuthQuery::Privilege::TRIGGER, AuthQuery::Privilege::CONFIG}; cpp<# @@ -2456,4 +2456,12 @@ cpp<# (:serialize (:slk)) (:clone)) +(lcp:define-class create-snapshot-query (query) () + (:public + #>cpp + DEFVISITABLE(QueryVisitor); + cpp<#) + (:serialize (:slk)) + (:clone)) + (lcp:pop-namespace) ;; namespace query diff --git a/src/query/frontend/ast/ast_visitor.hpp b/src/query/frontend/ast/ast_visitor.hpp index 9d55f6b12..937c5121d 100644 --- a/src/query/frontend/ast/ast_visitor.hpp +++ b/src/query/frontend/ast/ast_visitor.hpp @@ -78,6 +78,7 @@ class LoadCsv; class FreeMemoryQuery; class TriggerQuery; class IsolationLevelQuery; +class CreateSnapshotQuery; using TreeCompositeVisitor = ::utils::CompositeVisitor< SingleQuery, CypherUnion, NamedExpression, OrOperator, XorOperator, AndOperator, NotOperator, AdditionOperator, @@ -111,6 +112,7 @@ class ExpressionVisitor template class QueryVisitor : public ::utils::Visitor {}; + FreeMemoryQuery, TriggerQuery, IsolationLevelQuery, CreateSnapshotQuery> { +}; } // namespace query diff --git a/src/query/frontend/ast/cypher_main_visitor.cpp b/src/query/frontend/ast/cypher_main_visitor.cpp index 58c98bce2..724910be5 100644 --- a/src/query/frontend/ast/cypher_main_visitor.cpp +++ b/src/query/frontend/ast/cypher_main_visitor.cpp @@ -442,6 +442,11 @@ antlrcpp::Any CypherMainVisitor::visitIsolationLevelQuery(MemgraphCypher::Isolat return isolation_level_query; } +antlrcpp::Any CypherMainVisitor::visitCreateSnapshotQuery(MemgraphCypher::CreateSnapshotQueryContext *ctx) { + query_ = storage_->Create(); + return query_; +} + antlrcpp::Any CypherMainVisitor::visitCypherUnion(MemgraphCypher::CypherUnionContext *ctx) { bool distinct = !ctx->ALL(); auto *cypher_union = storage_->Create(distinct); @@ -870,11 +875,11 @@ antlrcpp::Any CypherMainVisitor::visitPrivilege(MemgraphCypher::PrivilegeContext if (ctx->CONSTRAINT()) return AuthQuery::Privilege::CONSTRAINT; if (ctx->DUMP()) return AuthQuery::Privilege::DUMP; if (ctx->REPLICATION()) return AuthQuery::Privilege::REPLICATION; - if (ctx->LOCK_PATH()) return AuthQuery::Privilege::LOCK_PATH; if (ctx->READ_FILE()) return AuthQuery::Privilege::READ_FILE; if (ctx->FREE_MEMORY()) return AuthQuery::Privilege::FREE_MEMORY; if (ctx->TRIGGER()) return AuthQuery::Privilege::TRIGGER; if (ctx->CONFIG()) return AuthQuery::Privilege::CONFIG; + if (ctx->DURABILITY()) return AuthQuery::Privilege::DURABILITY; LOG_FATAL("Should not get here - unknown privilege!"); } diff --git a/src/query/frontend/ast/cypher_main_visitor.hpp b/src/query/frontend/ast/cypher_main_visitor.hpp index 13825f33d..903071e73 100644 --- a/src/query/frontend/ast/cypher_main_visitor.hpp +++ b/src/query/frontend/ast/cypher_main_visitor.hpp @@ -243,6 +243,11 @@ class CypherMainVisitor : public antlropencypher::MemgraphCypherBaseVisitor { */ antlrcpp::Any visitIsolationLevelQuery(MemgraphCypher::IsolationLevelQueryContext *ctx) override; + /** + * @return CreateSnapshotQuery* + */ + antlrcpp::Any visitCreateSnapshotQuery(MemgraphCypher::CreateSnapshotQueryContext *ctx) override; + /** * @return CypherUnion* */ diff --git a/src/query/frontend/opencypher/grammar/MemgraphCypher.g4 b/src/query/frontend/opencypher/grammar/MemgraphCypher.g4 index e90157503..03ac4c426 100644 --- a/src/query/frontend/opencypher/grammar/MemgraphCypher.g4 +++ b/src/query/frontend/opencypher/grammar/MemgraphCypher.g4 @@ -86,6 +86,7 @@ query : cypherQuery | freeMemoryQuery | triggerQuery | isolationLevelQuery + | createSnapshotQuery ; authQuery : createRole @@ -183,11 +184,11 @@ privilege : CREATE | CONSTRAINT | DUMP | REPLICATION - | LOCK_PATH | READ_FILE | FREE_MEMORY | TRIGGER | CONFIG + | DURABILITY ; privilegeList : privilege ( ',' privilege )* ; @@ -241,3 +242,5 @@ isolationLevel : SNAPSHOT ISOLATION | READ COMMITTED | READ UNCOMMITTED ; isolationLevelScope : GLOBAL | SESSION | NEXT ; isolationLevelQuery : SET isolationLevelScope TRANSACTION ISOLATION LEVEL isolationLevel ; + +createSnapshotQuery : CREATE SNAPSHOT ; diff --git a/src/query/frontend/opencypher/grammar/MemgraphCypherLexer.g4 b/src/query/frontend/opencypher/grammar/MemgraphCypherLexer.g4 index 37d8cafc1..76e37675b 100644 --- a/src/query/frontend/opencypher/grammar/MemgraphCypherLexer.g4 +++ b/src/query/frontend/opencypher/grammar/MemgraphCypherLexer.g4 @@ -30,6 +30,7 @@ DENY : D E N Y ; DIRECTORY : D I R E C T O R Y ; DROP : D R O P ; DUMP : D U M P ; +DURABILITY : D U R A B I L I T Y ; EXECUTE : E X E C U T E ; FOR : F O R ; FREE : F R E E ; @@ -45,7 +46,6 @@ ISOLATION : I S O L A T I O N ; LEVEL : L E V E L ; LOAD : L O A D ; LOCK : L O C K ; -LOCK_PATH : L O C K UNDERSCORE P A T H ; MAIN : M A I N ; MODE : M O D E ; NEXT : N E X T ; diff --git a/src/query/frontend/semantic/required_privileges.cpp b/src/query/frontend/semantic/required_privileges.cpp index 49e78c4bf..698f396ef 100644 --- a/src/query/frontend/semantic/required_privileges.cpp +++ b/src/query/frontend/semantic/required_privileges.cpp @@ -49,7 +49,7 @@ class PrivilegeExtractor : public QueryVisitor, public HierarchicalTreeVis void Visit(DumpQuery &dump_query) override { AddPrivilege(AuthQuery::Privilege::DUMP); } - void Visit(LockPathQuery &lock_path_query) override { AddPrivilege(AuthQuery::Privilege::LOCK_PATH); } + void Visit(LockPathQuery &lock_path_query) override { AddPrivilege(AuthQuery::Privilege::DURABILITY); } void Visit(FreeMemoryQuery &free_memory_query) override { AddPrivilege(AuthQuery::Privilege::FREE_MEMORY); } @@ -59,6 +59,8 @@ class PrivilegeExtractor : public QueryVisitor, public HierarchicalTreeVis void Visit(IsolationLevelQuery &isolation_level_query) override { AddPrivilege(AuthQuery::Privilege::CONFIG); } + void Visit(CreateSnapshotQuery &create_snapshot_query) override { AddPrivilege(AuthQuery::Privilege::DURABILITY); } + bool PreVisit(Create & /*unused*/) override { AddPrivilege(AuthQuery::Privilege::CREATE); return false; diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp index 963c19fc8..17ca83879 100644 --- a/src/query/interpreter.cpp +++ b/src/query/interpreter.cpp @@ -6,6 +6,7 @@ #include "glue/communication.hpp" #include "query/constants.hpp" #include "query/context.hpp" +#include "query/cypher_query_interpreter.hpp" #include "query/db_accessor.hpp" #include "query/dump.hpp" #include "query/exceptions.hpp" @@ -1207,6 +1208,28 @@ PreparedQuery PrepareIsolationLevelQuery(ParsedQuery parsed_query, const bool in RWType::NONE}; } +PreparedQuery PrepareCreateSnapshotQuery(ParsedQuery parsed_query, bool in_explicit_transaction, + InterpreterContext *interpreter_context) { + if (in_explicit_transaction) { + throw CreateSnapshotInMulticommandTxException(); + } + + return PreparedQuery{ + {}, + std::move(parsed_query.required_privileges), + [interpreter_context](AnyStream *stream, std::optional n) -> std::optional { + if (auto maybe_error = interpreter_context->db->CreateSnapshot(); maybe_error.HasError()) { + switch (maybe_error.GetError()) { + case storage::Storage::CreateSnapshotError::DisabledForReplica: + throw utils::BasicException( + "Failed to create a snapshot. Replica instances are not allowed to create them."); + } + } + return QueryHandlerResult::COMMIT; + }, + RWType::NONE}; +} + PreparedQuery PrepareInfoQuery(ParsedQuery parsed_query, bool in_explicit_transaction, std::map *summary, InterpreterContext *interpreter_context, storage::Storage *db, utils::MemoryResource *execution_memory) { @@ -1552,6 +1575,9 @@ Interpreter::PrepareResult Interpreter::Prepare(const std::string &query_string, } else if (utils::Downcast(parsed_query.query)) { prepared_query = PrepareIsolationLevelQuery(std::move(parsed_query), in_explicit_transaction_, interpreter_context_, this); + } else if (utils::Downcast(parsed_query.query)) { + prepared_query = + PrepareCreateSnapshotQuery(std::move(parsed_query), in_explicit_transaction_, interpreter_context_); } else { LOG_FATAL("Should not get here -- unknown query type!"); } diff --git a/src/storage/v2/storage.cpp b/src/storage/v2/storage.cpp index fae4c710b..75a62ed11 100644 --- a/src/storage/v2/storage.cpp +++ b/src/storage/v2/storage.cpp @@ -361,7 +361,15 @@ Storage::Storage(Config config) } } if (config_.durability.snapshot_wal_mode != Config::Durability::SnapshotWalMode::DISABLED) { - snapshot_runner_.Run("Snapshot", config_.durability.snapshot_interval, [this] { this->CreateSnapshot(); }); + snapshot_runner_.Run("Snapshot", config_.durability.snapshot_interval, [this] { + if (auto maybe_error = this->CreateSnapshot(); maybe_error.HasError()) { + switch (maybe_error.GetError()) { + case CreateSnapshotError::DisabledForReplica: + spdlog::warn("Snapshots are disabled for replicas!"); + break; + } + } + }); } if (config_.gc.type == Config::Gc::Type::PERIODIC) { gc_runner_.Run("Storage GC", config_.gc.interval, [this] { this->CollectGarbage(); }); @@ -391,7 +399,13 @@ Storage::~Storage() { snapshot_runner_.Stop(); } if (config_.durability.snapshot_on_exit) { - CreateSnapshot(); + if (auto maybe_error = this->CreateSnapshot(); maybe_error.HasError()) { + switch (maybe_error.GetError()) { + case CreateSnapshotError::DisabledForReplica: + spdlog::warn("Snapshots are disabled for replicas!"); + break; + } + } } } @@ -1727,12 +1741,13 @@ void Storage::AppendToWal(durability::StorageGlobalOperation operation, LabelId FinalizeWalFile(); } -void Storage::CreateSnapshot() { +utils::BasicResult Storage::CreateSnapshot() { if (replication_role_.load() != ReplicationRole::MAIN) { - spdlog::warn("Snapshots are disabled for replicas!"); - return; + return CreateSnapshotError::DisabledForReplica; } + std::lock_guard snapshot_guard(snapshot_lock_); + // Take master RW lock (for reading). std::shared_lock storage_guard(main_lock_); @@ -1747,6 +1762,7 @@ void Storage::CreateSnapshot() { // Finalize snapshot transaction. commit_log_->MarkFinished(transaction.start_timestamp); + return {}; } bool Storage::LockPath() { diff --git a/src/storage/v2/storage.hpp b/src/storage/v2/storage.hpp index fcb13c349..e24c40b5c 100644 --- a/src/storage/v2/storage.hpp +++ b/src/storage/v2/storage.hpp @@ -428,6 +428,10 @@ class Storage final { void SetIsolationLevel(IsolationLevel isolation_level); + enum class CreateSnapshotError : uint8_t { DisabledForReplica }; + + utils::BasicResult CreateSnapshot(); + private: Transaction CreateTransaction(IsolationLevel isolation_level); @@ -452,8 +456,6 @@ class Storage final { void AppendToWal(durability::StorageGlobalOperation operation, LabelId label, const std::set &properties, uint64_t final_commit_timestamp); - void CreateSnapshot(); - uint64_t CommitTimestamp(std::optional desired_commit_timestamp = {}); // Main storage lock. @@ -518,6 +520,7 @@ class Storage final { utils::OutputFile lock_file_handle_; utils::Scheduler snapshot_runner_; + utils::SpinLock snapshot_lock_; // UUID used to distinguish snapshots and to link snapshots to WALs std::string uuid_; diff --git a/tests/unit/cypher_main_visitor.cpp b/tests/unit/cypher_main_visitor.cpp index fd673c3f4..f02b1cc16 100644 --- a/tests/unit/cypher_main_visitor.cpp +++ b/tests/unit/cypher_main_visitor.cpp @@ -2057,8 +2057,8 @@ TEST_P(CypherMainVisitorTest, GrantPrivilege) { {AuthQuery::Privilege::DUMP}); check_auth_query(&ast_generator, "GRANT REPLICATION TO user", AuthQuery::Action::GRANT_PRIVILEGE, "", "", "user", {}, {AuthQuery::Privilege::REPLICATION}); - check_auth_query(&ast_generator, "GRANT LOCK_PATH TO user", AuthQuery::Action::GRANT_PRIVILEGE, "", "", "user", {}, - {AuthQuery::Privilege::LOCK_PATH}); + check_auth_query(&ast_generator, "GRANT DURABILITY TO user", AuthQuery::Action::GRANT_PRIVILEGE, "", "", "user", {}, + {AuthQuery::Privilege::DURABILITY}); check_auth_query(&ast_generator, "GRANT READ_FILE TO user", AuthQuery::Action::GRANT_PRIVILEGE, "", "", "user", {}, {AuthQuery::Privilege::READ_FILE}); check_auth_query(&ast_generator, "GRANT FREE_MEMORY TO user", AuthQuery::Action::GRANT_PRIVILEGE, "", "", "user", {}, @@ -3198,4 +3198,9 @@ TEST_P(CypherMainVisitorTest, SetIsolationLevelQuery) { } } } + +TEST_P(CypherMainVisitorTest, CreateSnapshotQuery) { + auto &ast_generator = *GetParam(); + ASSERT_TRUE(dynamic_cast(ast_generator.ParseQuery("CREATE SNAPSHOT"))); +} } // namespace diff --git a/tests/unit/query_required_privileges.cpp b/tests/unit/query_required_privileges.cpp index 8ce38ee55..2a18d0b82 100644 --- a/tests/unit/query_required_privileges.cpp +++ b/tests/unit/query_required_privileges.cpp @@ -1,6 +1,7 @@ #include #include +#include "query/frontend/ast/ast.hpp" #include "query/frontend/ast/ast_visitor.hpp" #include "query/frontend/semantic/required_privileges.hpp" #include "storage/v2/id_types.hpp" @@ -142,7 +143,7 @@ TEST_F(TestPrivilegeExtractor, ReadFile) { TEST_F(TestPrivilegeExtractor, LockPathQuery) { auto *query = storage.Create(); - EXPECT_THAT(GetRequiredPrivileges(query), UnorderedElementsAre(AuthQuery::Privilege::LOCK_PATH)); + EXPECT_THAT(GetRequiredPrivileges(query), UnorderedElementsAre(AuthQuery::Privilege::DURABILITY)); } TEST_F(TestPrivilegeExtractor, FreeMemoryQuery) { @@ -159,3 +160,8 @@ TEST_F(TestPrivilegeExtractor, SetIsolationLevelQuery) { auto *query = storage.Create(); EXPECT_THAT(GetRequiredPrivileges(query), UnorderedElementsAre(AuthQuery::Privilege::CONFIG)); } + +TEST_F(TestPrivilegeExtractor, CreateSnapshotQuery) { + auto *query = storage.Create(); + EXPECT_THAT(GetRequiredPrivileges(query), UnorderedElementsAre(AuthQuery::Privilege::DURABILITY)); +}