From af56ab6ea84a0b96fd556b78f859c10be4695715 Mon Sep 17 00:00:00 2001 From: Andi <andi8647@gmail.com> Date: Mon, 23 Oct 2023 06:02:56 +0200 Subject: [PATCH] Forbid changing isolation level for disk and analytical (#1367) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marko Budiselić <marko.budiselic@memgraph.com> --- src/query/exceptions.hpp | 6 ++++++ src/query/interpreter.cpp | 23 ++++++++++++----------- src/storage/v2/storage.cpp | 4 ---- tests/e2e/graphql/graphql_server.py | 1 + 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/query/exceptions.hpp b/src/query/exceptions.hpp index 7a86b1126..a0998d035 100644 --- a/src/query/exceptions.hpp +++ b/src/query/exceptions.hpp @@ -307,6 +307,12 @@ class IsolationLevelModificationInAnalyticsException : public QueryException { SPECIALIZE_GET_EXCEPTION_NAME(IsolationLevelModificationInAnalyticsException) }; +class IsolationLevelModificationInDiskTransactionalException : public QueryException { + public: + IsolationLevelModificationInDiskTransactionalException() + : QueryException("Snapshot isolation level is the only supported isolation level for disk storage.") {} +}; + class StorageModeModificationInMulticommandTxException : public QueryException { public: StorageModeModificationInMulticommandTxException() diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp index d433fc68c..9cb34fbc4 100644 --- a/src/query/interpreter.cpp +++ b/src/query/interpreter.cpp @@ -2613,25 +2613,26 @@ PreparedQuery PrepareIsolationLevelQuery(ParsedQuery parsed_query, const bool in MG_ASSERT(isolation_level_query); const auto isolation_level = ToStorageIsolationLevel(isolation_level_query->isolation_level_); + MG_ASSERT(current_db.db_acc_, "Storage Isolation Level query expects a current DB"); + storage::Storage *storage = current_db.db_acc_->get()->storage(); + if (storage->GetStorageMode() == storage::StorageMode::IN_MEMORY_ANALYTICAL) { + throw IsolationLevelModificationInAnalyticsException(); + } + if (storage->GetStorageMode() == storage::StorageMode::ON_DISK_TRANSACTIONAL && + isolation_level != storage::IsolationLevel::SNAPSHOT_ISOLATION) { + throw IsolationLevelModificationInDiskTransactionalException(); + } std::function<void()> callback; switch (isolation_level_query->isolation_level_scope_) { - case IsolationLevelQuery::IsolationLevelScope::GLOBAL: // TODO:.....not GLOBAL is it?! - { - MG_ASSERT(current_db.db_acc_, "Storage Isolation Level query expects a current DB"); - storage::Storage *storage = current_db.db_acc_->get()->storage(); + case IsolationLevelQuery::IsolationLevelScope::GLOBAL: { callback = [storage, isolation_level] { - if (auto maybe_error = storage->SetIsolationLevel(isolation_level); maybe_error.HasError()) { - switch (maybe_error.GetError()) { - case storage::Storage::SetIsolationLevelError::DisabledForAnalyticalMode: - throw IsolationLevelModificationInAnalyticsException(); - break; - } + if (auto res = storage->SetIsolationLevel(isolation_level); res.HasError()) { + throw utils::BasicException("Failed setting global isolation level"); } }; break; } - case IsolationLevelQuery::IsolationLevelScope::SESSION: { callback = [interpreter, isolation_level] { interpreter->SetSessionIsolationLevel(isolation_level); }; break; diff --git a/src/storage/v2/storage.cpp b/src/storage/v2/storage.cpp index 81c86c2ae..040edf5fe 100644 --- a/src/storage/v2/storage.cpp +++ b/src/storage/v2/storage.cpp @@ -111,10 +111,6 @@ IsolationLevel Storage::GetIsolationLevel() const noexcept { return isolation_le utils::BasicResult<Storage::SetIsolationLevelError> Storage::SetIsolationLevel(IsolationLevel isolation_level) { std::unique_lock main_guard{main_lock_}; - if (storage_mode_ == storage::StorageMode::IN_MEMORY_ANALYTICAL) { - return Storage::SetIsolationLevelError::DisabledForAnalyticalMode; - } - isolation_level_ = isolation_level; return {}; } diff --git a/tests/e2e/graphql/graphql_server.py b/tests/e2e/graphql/graphql_server.py index 74a5e197a..03406362f 100644 --- a/tests/e2e/graphql/graphql_server.py +++ b/tests/e2e/graphql/graphql_server.py @@ -21,6 +21,7 @@ class GraphQLServer: self.__wait_process_to_init(7687) self.__wait_process_to_init(4000) atexit.register(self.__shut_down) + print(f"GraphQLServer started") def send_query(self, query: str, timeout=5.0) -> requests.Response: try: