From 5e5f4ffc5d2de94792c8e3ef362b8a11e1154685 Mon Sep 17 00:00:00 2001 From: andrejtonev <29177572+andrejtonev@users.noreply.github.com> Date: Mon, 11 Sep 2023 17:30:54 +0200 Subject: [PATCH] Add more runtime configurable settings (#1183) server name, query timeout settings, log.level, log.to_stderr --- src/CMakeLists.txt | 9 +- src/communication/bolt/v1/session.hpp | 8 + src/communication/bolt/v1/states/init.hpp | 2 +- src/communication/v2/session.hpp | 2 + src/flags/CMakeLists.txt | 9 + src/flags/all.hpp | 1 + src/flags/bolt.cpp | 4 + src/flags/bolt.hpp | 2 + src/flags/general.cpp | 8 +- src/flags/log_level.cpp | 33 ++- src/flags/log_level.hpp | 11 + src/flags/run_time_configurable.cpp | 111 ++++++++++ src/flags/run_time_configurable.hpp | 26 +++ src/glue/SessionHL.cpp | 14 +- src/glue/auth_global.hpp | 16 ++ src/glue/auth_handler.hpp | 3 +- src/memgraph.cpp | 3 +- src/query/CMakeLists.txt | 13 +- src/query/config.hpp | 4 +- src/query/interpreter.cpp | 7 +- src/storage/v2/CMakeLists.txt | 1 - src/storage/v2/config.hpp | 1 - src/storage/v2/isolation_level.cpp | 34 --- src/storage/v2/isolation_level.hpp | 19 +- src/storage/v2/replication/replication.hpp | 2 + src/storage/v2/storage.hpp | 1 + src/utils/settings.cpp | 23 +- src/utils/settings.hpp | 8 +- tests/drivers/python/v5_8/run.sh | 1 + tests/drivers/python/v5_8/server_name.py | 52 +++++ tests/drivers/python/v5_8/transactions.py | 61 +++++- tests/drivers/run.sh | 8 +- tests/drivers/run_v1.sh | 84 ++++++++ tests/e2e/configuration/default_config.py | 4 +- tests/integration/CMakeLists.txt | 5 +- .../run_time_settings/CMakeLists.txt | 16 ++ .../run_time_settings/executor.cpp | 53 +++++ .../run_time_settings/flag_tester.cpp | 69 ++++++ tests/integration/run_time_settings/runner.py | 202 ++++++++++++++++++ .../integration/run_time_settings/tester.cpp | 106 +++++++++ tests/unit/auth_handler.cpp | 3 +- tests/unit/interpreter.cpp | 7 +- 42 files changed, 949 insertions(+), 97 deletions(-) create mode 100644 src/flags/CMakeLists.txt create mode 100644 src/flags/run_time_configurable.cpp create mode 100644 src/flags/run_time_configurable.hpp create mode 100644 src/glue/auth_global.hpp delete mode 100644 src/storage/v2/isolation_level.cpp create mode 100644 tests/drivers/python/v5_8/server_name.py create mode 100755 tests/drivers/run_v1.sh create mode 100644 tests/integration/run_time_settings/CMakeLists.txt create mode 100644 tests/integration/run_time_settings/executor.cpp create mode 100644 tests/integration/run_time_settings/flag_tester.cpp create mode 100755 tests/integration/run_time_settings/runner.py create mode 100644 tests/integration/run_time_settings/tester.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c3483779d..fa35ccfbc 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -18,6 +18,7 @@ add_subdirectory(rpc) add_subdirectory(license) add_subdirectory(auth) add_subdirectory(audit) +add_subdirectory(flags) string(TOLOWER ${CMAKE_BUILD_TYPE} lower_build_type) @@ -31,19 +32,13 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR}) # ---------------------------------------------------------------------------- set(mg_single_node_v2_sources memgraph.cpp - flags/isolation_level.cpp - flags/memory_limit.cpp - flags/log_level.cpp - flags/general.cpp - flags/audit.cpp - flags/bolt.cpp ) # memgraph main executable add_executable(memgraph ${mg_single_node_v2_sources}) target_include_directories(memgraph PUBLIC ${CMAKE_SOURCE_DIR}/include) target_link_libraries(memgraph stdc++fs Threads::Threads - mg-telemetry mg-communication mg-memory mg-utils mg-license mg-settings mg-glue) + mg-telemetry mg-communication mg-memory mg-utils mg-license mg-settings mg-glue mg-flags) # NOTE: `include/mg_procedure.syms` describes a pattern match for symbols which # should be dynamically exported, so that `dlopen` can correctly link the diff --git a/src/communication/bolt/v1/session.hpp b/src/communication/bolt/v1/session.hpp index bc968dab8..ab594722c 100644 --- a/src/communication/bolt/v1/session.hpp +++ b/src/communication/bolt/v1/session.hpp @@ -159,6 +159,7 @@ class Session : public dbms::SessionInterface { break; case State::Idle: case State::Result: + at_least_one_run_ = true; state_ = StateExecutingRun(*this, state_); break; case State::Error: @@ -180,6 +181,12 @@ class Session : public dbms::SessionInterface { } } + void HandleError() { + if (!at_least_one_run_) { + spdlog::info("Sudden connection loss. Make sure the client supports Memgraph."); + } + } + // TODO: Rethink if there is a way to hide some members. At the momement all of them are public. TInputStream &input_stream_; TOutputStream &output_stream_; @@ -192,6 +199,7 @@ class Session : public dbms::SessionInterface { bool handshake_done_{false}; State state_{State::Handshake}; + bool at_least_one_run_{false}; struct Version { uint8_t major; diff --git a/src/communication/bolt/v1/states/init.hpp b/src/communication/bolt/v1/states/init.hpp index 955223467..2645d3987 100644 --- a/src/communication/bolt/v1/states/init.hpp +++ b/src/communication/bolt/v1/states/init.hpp @@ -174,7 +174,7 @@ State SendSuccessMessage(TSession &session) { // we send a hardcoded value for now. std::map metadata{{"connection_id", "bolt-1"}}; if (auto server_name = session.GetServerNameForInit(); server_name) { - metadata.insert({"server", *server_name}); + metadata.insert({"server", std::move(*server_name)}); } bool success_sent = session.encoder_.MessageSuccess(metadata); if (!success_sent) { diff --git a/src/communication/v2/session.hpp b/src/communication/v2/session.hpp index f069546fd..f74740197 100644 --- a/src/communication/v2/session.hpp +++ b/src/communication/v2/session.hpp @@ -413,6 +413,8 @@ class Session final : public std::enable_shared_from_thisWritten(bytes_transferred); diff --git a/src/flags/CMakeLists.txt b/src/flags/CMakeLists.txt new file mode 100644 index 000000000..66a82fbdf --- /dev/null +++ b/src/flags/CMakeLists.txt @@ -0,0 +1,9 @@ +add_library(mg-flags STATIC audit.cpp + bolt.cpp + general.cpp + isolation_level.cpp + log_level.cpp + memory_limit.cpp + run_time_configurable.cpp) +target_include_directories(mg-flags PUBLIC ${CMAKE_SOURCE_DIR}/include) +target_link_libraries(mg-flags PUBLIC spdlog::spdlog mg-settings mg-utils) diff --git a/src/flags/all.hpp b/src/flags/all.hpp index 6a0309133..1410d3d92 100644 --- a/src/flags/all.hpp +++ b/src/flags/all.hpp @@ -16,3 +16,4 @@ #include "flags/isolation_level.hpp" #include "flags/log_level.hpp" #include "flags/memory_limit.hpp" +#include "flags/run_time_configurable.hpp" diff --git a/src/flags/bolt.cpp b/src/flags/bolt.cpp index 9401acf3d..eabc38e03 100644 --- a/src/flags/bolt.cpp +++ b/src/flags/bolt.cpp @@ -36,3 +36,7 @@ DEFINE_VALIDATED_int32(bolt_session_inactivity_timeout, 1800, DEFINE_string(bolt_cert_file, "", "Certificate file which should be used for the Bolt server."); // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) DEFINE_string(bolt_key_file, "", "Key file which should be used for the Bolt server."); +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +DEFINE_string(bolt_server_name_for_init, "", + "Server name which the database should send to the client in the " + "Bolt INIT message."); diff --git a/src/flags/bolt.hpp b/src/flags/bolt.hpp index f03b75e99..0556be8bc 100644 --- a/src/flags/bolt.hpp +++ b/src/flags/bolt.hpp @@ -25,3 +25,5 @@ DECLARE_int32(bolt_session_inactivity_timeout); DECLARE_string(bolt_cert_file); // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) DECLARE_string(bolt_key_file); +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +DECLARE_string(bolt_server_name_for_init); diff --git a/src/flags/general.cpp b/src/flags/general.cpp index 30e8dd4ad..01c7a4f12 100644 --- a/src/flags/general.cpp +++ b/src/flags/general.cpp @@ -11,9 +11,13 @@ #include "general.hpp" +#include "glue/auth_global.hpp" +#include "storage/v2/config.hpp" +#include "utils/file.hpp" #include "utils/flag_validation.hpp" +#include "utils/string.hpp" -#include "glue/auth_handler.hpp" +#include // Short help flag. // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) @@ -142,7 +146,7 @@ DEFINE_string(pulsar_service_url, "", "Default URL used while connecting to Puls // Query flags. // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -DEFINE_double(query_execution_timeout_sec, 600, +DEFINE_double(query_execution_timeout_sec, -1, "Maximum allowed query execution time. Queries exceeding this " "limit will be aborted. Value of 0 means no limit."); diff --git a/src/flags/log_level.cpp b/src/flags/log_level.cpp index b292410d4..1a4998b13 100644 --- a/src/flags/log_level.cpp +++ b/src/flags/log_level.cpp @@ -37,7 +37,10 @@ inline constexpr std::array log_level_mappings{ const std::string log_level_help_string = fmt::format("Minimum log level. Allowed values: {}", memgraph::utils::GetAllowedEnumValuesString(log_level_mappings)); -DEFINE_VALIDATED_string(log_level, "WARNING", log_level_help_string.c_str(), { +DEFINE_VALIDATED_string(log_level, "WARNING", log_level_help_string.c_str(), + { return memgraph::flags::ValidLogLevel(value); }); + +bool memgraph::flags::ValidLogLevel(std::string_view value) { if (const auto result = memgraph::utils::IsValidEnumValueString(value, log_level_mappings); result.HasError()) { const auto error = result.GetError(); switch (error) { @@ -55,10 +58,14 @@ DEFINE_VALIDATED_string(log_level, "WARNING", log_level_help_string.c_str(), { } return true; -}); +} + +std::optional memgraph::flags::LogLevelToEnum(std::string_view value) { + return memgraph::utils::StringToEnum(value, log_level_mappings); +} spdlog::level::level_enum ParseLogLevel() { - const auto log_level = memgraph::utils::StringToEnum(FLAGS_log_level, log_level_mappings); + const auto log_level = memgraph::flags::LogLevelToEnum(FLAGS_log_level); MG_ASSERT(log_level, "Invalid log level"); return *log_level; } @@ -70,14 +77,19 @@ void CreateLoggerFromSink(const auto &sinks, const auto log_level) { logger->set_level(log_level); logger->flush_on(spdlog::level::trace); spdlog::set_default_logger(std::move(logger)); + // Enable stderr sink + if (FLAGS_also_log_to_stderr) { + memgraph::flags::LogToStderr(log_level); + } } void memgraph::flags::InitializeLogger() { std::vector sinks; - if (FLAGS_also_log_to_stderr) { - sinks.emplace_back(std::make_shared()); - } + // Force the stderr logger to be at the front of the sinks vector + // Will be used to disable/enable it at run-time by settings its log level + sinks.emplace_back(std::make_shared()); + sinks.back()->set_level(spdlog::level::off); if (!FLAGS_log_file.empty()) { // get local time @@ -93,9 +105,18 @@ void memgraph::flags::InitializeLogger() { CreateLoggerFromSink(sinks, ParseLogLevel()); } +// TODO: Make sure this is used in a safe way void memgraph::flags::AddLoggerSink(spdlog::sink_ptr new_sink) { auto default_logger = spdlog::default_logger(); auto sinks = default_logger->sinks(); sinks.push_back(new_sink); CreateLoggerFromSink(sinks, default_logger->level()); } + +// Thread-safe because the level enum is an atomic +// NOTE: default_logger is not thread-safe and shouldn't be changed during application lifetime +void memgraph::flags::LogToStderr(spdlog::level::level_enum log_level) { + auto default_logger = spdlog::default_logger(); + auto sink = default_logger->sinks().front(); + sink->set_level(log_level); +} diff --git a/src/flags/log_level.hpp b/src/flags/log_level.hpp index 18d9df81b..1f1824c86 100644 --- a/src/flags/log_level.hpp +++ b/src/flags/log_level.hpp @@ -11,8 +11,19 @@ #pragma once #include +#include +#include "gflags/gflags.h" + +DECLARE_string(log_level); +DECLARE_bool(also_log_to_stderr); namespace memgraph::flags { + +bool ValidLogLevel(std::string_view value); +std::optional LogLevelToEnum(std::string_view value); + void InitializeLogger(); void AddLoggerSink(spdlog::sink_ptr new_sink); +void LogToStderr(spdlog::level::level_enum log_level); + } // namespace memgraph::flags diff --git a/src/flags/run_time_configurable.cpp b/src/flags/run_time_configurable.cpp new file mode 100644 index 000000000..93cf6f3ce --- /dev/null +++ b/src/flags/run_time_configurable.cpp @@ -0,0 +1,111 @@ +// Copyright 2023 Memgraph Ltd. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +#include "flags/run_time_configurable.hpp" +#include +#include "flags/bolt.hpp" +#include "flags/general.hpp" +#include "flags/log_level.hpp" +#include "spdlog/cfg/helpers-inl.h" +#include "spdlog/spdlog.h" +#include "utils/exceptions.hpp" +#include "utils/settings.hpp" +#include "utils/string.hpp" + +namespace { +// Bolt server name +constexpr auto kServerNameSettingKey = "server.name"; +constexpr auto kDefaultServerName = "Neo4j/v5.11.0 compatible graph database server - Memgraph"; +// Query timeout +constexpr auto kQueryTxSettingKey = "query.timeout"; +constexpr auto kDefaultQueryTx = "600"; // seconds +// Log level +// No default value because it is not persistent +constexpr auto kLogLevelSettingKey = "log.level"; +// Log to stderr +// No default value because it is not persistent +constexpr auto kLogToStderrSettingKey = "log.to_stderr"; +} // namespace + +namespace memgraph::flags::run_time { +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +memgraph::utils::Synchronized bolt_server_name_; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +std::atomic execution_timeout_sec_; + +void Initialize() { + // Register bolt server name settings + memgraph::utils::global_settings.RegisterSetting(kServerNameSettingKey, kDefaultServerName, [&] { + const auto server_name = memgraph::utils::global_settings.GetValue(kServerNameSettingKey); + MG_ASSERT(server_name, "Bolt server name is missing from the settings"); + *(bolt_server_name_.Lock()) = *server_name; + }); + // Update value from read settings + const auto &name = memgraph::utils::global_settings.GetValue(kServerNameSettingKey); + MG_ASSERT(name, "Failed to read server name from settings."); + *(bolt_server_name_.Lock()) = *name; + // Override server name if passed via command line argument + if (!FLAGS_bolt_server_name_for_init.empty()) { + memgraph::utils::global_settings.SetValue(kServerNameSettingKey, FLAGS_bolt_server_name_for_init); + } + + // Register query timeout + memgraph::utils::global_settings.RegisterSetting(kQueryTxSettingKey, kDefaultQueryTx, [&] { + const auto query_tx = memgraph::utils::global_settings.GetValue(kQueryTxSettingKey); + MG_ASSERT(query_tx, "Query timeout is missing from the settings"); + execution_timeout_sec_ = std::stod(*query_tx); + }); + // Update value from read settings + const auto &tx = memgraph::utils::global_settings.GetValue(kQueryTxSettingKey); + MG_ASSERT(tx, "Failed to read query timeout from settings."); + execution_timeout_sec_ = std::stod(*tx); + // Override query timeout if passed via command line argument + if (FLAGS_query_execution_timeout_sec != -1) { + memgraph::utils::global_settings.SetValue(kQueryTxSettingKey, std::to_string(FLAGS_query_execution_timeout_sec)); + } + + // Register log level + auto get_global_log_level = []() { + const auto log_level = memgraph::utils::global_settings.GetValue(kLogLevelSettingKey); + MG_ASSERT(log_level, "Log level is missing from the settings"); + const auto ll_enum = memgraph::flags::LogLevelToEnum(*log_level); + if (!ll_enum) { + throw utils::BasicException("Unsupported log level {}", *log_level); + } + return *ll_enum; + }; + memgraph::utils::global_settings.RegisterSetting( + kLogLevelSettingKey, FLAGS_log_level, [&] { spdlog::set_level(get_global_log_level()); }, + memgraph::flags::ValidLogLevel); + // Always override log level with command line argument + memgraph::utils::global_settings.SetValue(kLogLevelSettingKey, FLAGS_log_level); + + // Register logging to stderr + auto bool_to_str = [](bool in) { return in ? "true" : "false"; }; + const std::string log_to_stderr_s = bool_to_str(FLAGS_also_log_to_stderr); + memgraph::utils::global_settings.RegisterSetting( + kLogToStderrSettingKey, log_to_stderr_s, + [&] { + const auto enable = memgraph::utils::global_settings.GetValue(kLogToStderrSettingKey); + if (enable == "true") { + LogToStderr(get_global_log_level()); + } else { + LogToStderr(spdlog::level::off); + } + }, + [](std::string_view in) { + const auto lc = memgraph::utils::ToLowerCase(in); + return lc == "false" || lc == "true"; + }); + // Always override log to stderr with command line argument + memgraph::utils::global_settings.SetValue(kLogToStderrSettingKey, log_to_stderr_s); +} +} // namespace memgraph::flags::run_time diff --git a/src/flags/run_time_configurable.hpp b/src/flags/run_time_configurable.hpp new file mode 100644 index 000000000..86c28721b --- /dev/null +++ b/src/flags/run_time_configurable.hpp @@ -0,0 +1,26 @@ +// Copyright 2023 Memgraph Ltd. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. +#pragma once + +#include "utils/spin_lock.hpp" +#include "utils/synchronized.hpp" + +namespace memgraph::flags::run_time { + +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +extern utils::Synchronized bolt_server_name_; + +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +extern std::atomic execution_timeout_sec_; + +void Initialize(); + +} // namespace memgraph::flags::run_time diff --git a/src/glue/SessionHL.cpp b/src/glue/SessionHL.cpp index 4983e02a8..2387f7e34 100644 --- a/src/glue/SessionHL.cpp +++ b/src/glue/SessionHL.cpp @@ -10,12 +10,15 @@ // licenses/APL.txt. #include "glue/SessionHL.hpp" +#include #include "audit/log.hpp" +#include "flags/run_time_configurable.hpp" #include "glue/auth_checker.hpp" #include "glue/communication.hpp" #include "license/license.hpp" #include "query/discard_value_stream.hpp" +#include "utils/spin_lock.hpp" #include "gflags/gflags.h" @@ -23,11 +26,6 @@ namespace memgraph::metrics { extern const Event ActiveBoltSessions; } // namespace memgraph::metrics -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -DEFINE_string(bolt_server_name_for_init, "", - "Server name which the database should send to the client in the " - "Bolt INIT message."); - auto ToQueryExtras(const memgraph::communication::bolt::Value &extra) -> memgraph::query::QueryExtras { auto const &as_map = extra.ValueMap(); @@ -151,9 +149,10 @@ memgraph::dbms::SetForResult SessionHL::OnChange(const std::string &db_name) { std::string SessionHL::GetDatabaseName() const { return interpreter_context_->db->id(); } std::optional SessionHL::GetServerNameForInit() { - if (FLAGS_bolt_server_name_for_init.empty()) return std::nullopt; - return FLAGS_bolt_server_name_for_init; + auto locked_name = flags::run_time::bolt_server_name_.Lock(); + return locked_name->empty() ? std::nullopt : std::make_optional(*locked_name); } + bool SessionHL::Authenticate(const std::string &username, const std::string &password) { auto locked_auth = auth_->Lock(); if (!locked_auth->HasUsers()) { @@ -304,6 +303,7 @@ SessionHL::SessionHL( #endif endpoint_(endpoint), run_id_(current_.run_id()) { + // Metrics update memgraph::metrics::IncrementCounter(memgraph::metrics::ActiveBoltSessions); } diff --git a/src/glue/auth_global.hpp b/src/glue/auth_global.hpp new file mode 100644 index 000000000..4675b6978 --- /dev/null +++ b/src/glue/auth_global.hpp @@ -0,0 +1,16 @@ +// Copyright 2023 Memgraph Ltd. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +#pragma once + +namespace memgraph::glue { +inline constexpr std::string_view kDefaultUserRoleRegex = "[a-zA-Z0-9_.+-@]+"; +} // namespace memgraph::glue diff --git a/src/glue/auth_handler.hpp b/src/glue/auth_handler.hpp index 716095add..b2f278a47 100644 --- a/src/glue/auth_handler.hpp +++ b/src/glue/auth_handler.hpp @@ -14,6 +14,7 @@ #include #include "auth/auth.hpp" +#include "auth_global.hpp" #include "glue/auth.hpp" #include "license/license.hpp" #include "query/interpreter.hpp" @@ -21,8 +22,6 @@ namespace memgraph::glue { -inline constexpr std::string_view kDefaultUserRoleRegex = "[a-zA-Z0-9_.+-@]+"; - class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { memgraph::utils::Synchronized *auth_; std::string name_regex_string_; diff --git a/src/memgraph.cpp b/src/memgraph.cpp index 55ba2a921..6a5010d21 100644 --- a/src/memgraph.cpp +++ b/src/memgraph.cpp @@ -9,6 +9,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +#include "flags/run_time_configurable.hpp" #ifndef MG_ENTERPRISE #include "dbms/session_context_handler.hpp" #endif @@ -209,6 +210,7 @@ int main(int argc, char **argv) { // register all runtime settings memgraph::license::RegisterLicenseSettings(memgraph::license::global_license_checker, memgraph::utils::global_settings); + memgraph::flags::run_time::Initialize(); memgraph::license::global_license_checker.CheckEnvLicense(); if (!FLAGS_organization_name.empty() && !FLAGS_license_key.empty()) { @@ -291,7 +293,6 @@ int main(int argc, char **argv) { // Default interpreter configuration memgraph::query::InterpreterConfig interp_config{ .query = {.allow_load_csv = FLAGS_allow_load_csv}, - .execution_timeout_sec = FLAGS_query_execution_timeout_sec, .replication_replica_check_frequency = std::chrono::seconds(FLAGS_replication_replica_check_frequency_sec), .default_kafka_bootstrap_servers = FLAGS_kafka_bootstrap_servers, .default_pulsar_service_url = FLAGS_pulsar_service_url, diff --git a/src/query/CMakeLists.txt b/src/query/CMakeLists.txt index b78c5b74d..c1370cc0b 100644 --- a/src/query/CMakeLists.txt +++ b/src/query/CMakeLists.txt @@ -40,7 +40,18 @@ set(mg_query_sources add_library(mg-query STATIC ${mg_query_sources}) target_include_directories(mg-query PUBLIC ${CMAKE_SOURCE_DIR}/include) -target_link_libraries(mg-query PUBLIC dl cppitertools Python3::Python mg-integrations-pulsar mg-integrations-kafka mg-storage-v2 mg-license mg-utils mg-kvstore mg-memory mg::csv) +target_link_libraries(mg-query PUBLIC dl + cppitertools + Python3::Python + mg-integrations-pulsar + mg-integrations-kafka + mg-storage-v2 + mg-license + mg-utils + mg-kvstore + mg-memory + mg::csv + mg-flags) if(NOT "${MG_PYTHON_PATH}" STREQUAL "") set(Python3_ROOT_DIR "${MG_PYTHON_PATH}") endif() diff --git a/src/query/config.hpp b/src/query/config.hpp index d3d6c1d8d..59a34e263 100644 --- a/src/query/config.hpp +++ b/src/query/config.hpp @@ -1,4 +1,4 @@ -// Copyright 2022 Memgraph Ltd. +// Copyright 2023 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -19,8 +19,6 @@ struct InterpreterConfig { bool allow_load_csv{true}; } query; - // The default execution timeout is 10 minutes. - double execution_timeout_sec{600.0}; // The same as \ref memgraph::storage::replication::ReplicationClientConfig std::chrono::seconds replication_replica_check_frequency{1}; diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp index e051c9fd1..0b70536bd 100644 --- a/src/query/interpreter.cpp +++ b/src/query/interpreter.cpp @@ -35,6 +35,7 @@ #include "csv/parsing.hpp" #include "dbms/global.hpp" #include "dbms/session_context_handler.hpp" +#include "flags/run_time_configurable.hpp" #include "glue/communication.hpp" #include "license/license.hpp" #include "memory/memory_control.hpp" @@ -1377,7 +1378,7 @@ Interpreter::Interpreter(InterpreterContext *interpreter_context) : interpreter_ auto DetermineTxTimeout(std::optional tx_timeout_ms, InterpreterConfig const &config) -> TxTimeout { using double_seconds = std::chrono::duration; - auto const global_tx_timeout = double_seconds{config.execution_timeout_sec}; + auto const global_tx_timeout = double_seconds{flags::run_time::execution_timeout_sec_}; auto const valid_global_tx_timeout = global_tx_timeout > double_seconds{0}; if (tx_timeout_ms) { @@ -3798,7 +3799,7 @@ void RunTriggersIndividually(const utils::SkipList &triggers, Interpret auto trigger_context = original_trigger_context; trigger_context.AdaptForAccessor(&db_accessor); try { - trigger.Execute(&db_accessor, &execution_memory, interpreter_context->config.execution_timeout_sec, + trigger.Execute(&db_accessor, &execution_memory, flags::run_time::execution_timeout_sec_, &interpreter_context->is_shutting_down, transaction_status, trigger_context, interpreter_context->auth_checker); } catch (const utils::BasicException &exception) { @@ -3905,7 +3906,7 @@ void Interpreter::Commit() { utils::MonotonicBufferResource execution_memory{kExecutionMemoryBlockSize}; AdvanceCommand(); try { - trigger.Execute(&*execution_db_accessor_, &execution_memory, interpreter_context_->config.execution_timeout_sec, + trigger.Execute(&*execution_db_accessor_, &execution_memory, flags::run_time::execution_timeout_sec_, &interpreter_context_->is_shutting_down, &transaction_status_, *trigger_context, interpreter_context_->auth_checker); } catch (const utils::BasicException &e) { diff --git a/src/storage/v2/CMakeLists.txt b/src/storage/v2/CMakeLists.txt index 3cd3a04d7..7c9c567ed 100644 --- a/src/storage/v2/CMakeLists.txt +++ b/src/storage/v2/CMakeLists.txt @@ -32,7 +32,6 @@ add_library(mg-storage-v2 STATIC disk/label_property_index.cpp disk/unique_constraints.cpp storage_mode.cpp - isolation_level.cpp replication/replication_client.cpp replication/replication_server.cpp replication/serialization.cpp diff --git a/src/storage/v2/config.hpp b/src/storage/v2/config.hpp index 8a07c144d..bdb2e586b 100644 --- a/src/storage/v2/config.hpp +++ b/src/storage/v2/config.hpp @@ -15,7 +15,6 @@ #include #include #include "storage/v2/isolation_level.hpp" -#include "storage/v2/transaction.hpp" #include "utils/exceptions.hpp" namespace memgraph::storage { diff --git a/src/storage/v2/isolation_level.cpp b/src/storage/v2/isolation_level.cpp deleted file mode 100644 index d9c09d2f3..000000000 --- a/src/storage/v2/isolation_level.cpp +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2023 Memgraph Ltd. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source -// License, and you may not use this file except in compliance with the Business Source License. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -#include "isolation_level.hpp" - -namespace memgraph::storage { - -std::string_view IsolationLevelToString(IsolationLevel isolation_level) { - switch (isolation_level) { - case IsolationLevel::READ_COMMITTED: - return "READ_COMMITTED"; - case IsolationLevel::READ_UNCOMMITTED: - return "READ_UNCOMMITTED"; - case IsolationLevel::SNAPSHOT_ISOLATION: - return "SNAPSHOT_ISOLATION"; - } -} - -std::string_view IsolationLevelToString(std::optional isolation_level) { - if (isolation_level) { - return IsolationLevelToString(*isolation_level); - } - return ""; -} - -} // namespace memgraph::storage diff --git a/src/storage/v2/isolation_level.hpp b/src/storage/v2/isolation_level.hpp index b83d14f4e..3026ac131 100644 --- a/src/storage/v2/isolation_level.hpp +++ b/src/storage/v2/isolation_level.hpp @@ -19,7 +19,22 @@ namespace memgraph::storage { enum class IsolationLevel : std::uint8_t { SNAPSHOT_ISOLATION, READ_COMMITTED, READ_UNCOMMITTED }; -std::string_view IsolationLevelToString(IsolationLevel isolation_level); -std::string_view IsolationLevelToString(std::optional isolation_level); +static inline std::string_view IsolationLevelToString(IsolationLevel isolation_level) { + switch (isolation_level) { + case IsolationLevel::READ_COMMITTED: + return "READ_COMMITTED"; + case IsolationLevel::READ_UNCOMMITTED: + return "READ_UNCOMMITTED"; + case IsolationLevel::SNAPSHOT_ISOLATION: + return "SNAPSHOT_ISOLATION"; + } +} + +static inline std::string_view IsolationLevelToString(std::optional isolation_level) { + if (isolation_level) { + return IsolationLevelToString(*isolation_level); + } + return ""; +} } // namespace memgraph::storage diff --git a/src/storage/v2/replication/replication.hpp b/src/storage/v2/replication/replication.hpp index 71c4d2b2b..18306542f 100644 --- a/src/storage/v2/replication/replication.hpp +++ b/src/storage/v2/replication/replication.hpp @@ -12,7 +12,9 @@ #pragma once #include "kvstore/kvstore.hpp" +#include "storage/v2/delta.hpp" #include "storage/v2/durability/storage_global_operation.hpp" +#include "storage/v2/transaction.hpp" #include "utils/result.hpp" /// REPLICATION /// diff --git a/src/storage/v2/storage.hpp b/src/storage/v2/storage.hpp index 8f97ebb58..7d73388c7 100644 --- a/src/storage/v2/storage.hpp +++ b/src/storage/v2/storage.hpp @@ -31,6 +31,7 @@ #include "storage/v2/replication/replication_server.hpp" #include "storage/v2/storage_error.hpp" #include "storage/v2/storage_mode.hpp" +#include "storage/v2/transaction.hpp" #include "storage/v2/vertices_iterable.hpp" #include "utils/event_counter.hpp" #include "utils/event_histogram.hpp" diff --git a/src/utils/settings.cpp b/src/utils/settings.cpp index 8d1ed9d69..330b43f48 100644 --- a/src/utils/settings.cpp +++ b/src/utils/settings.cpp @@ -1,4 +1,4 @@ -// Copyright 2022 Memgraph Ltd. +// Copyright 2023 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -29,18 +29,25 @@ void Settings::Finalize() { on_change_callbacks_.clear(); } -void Settings::RegisterSetting(std::string name, const std::string &default_value, OnChangeCallback callback) { +void Settings::RegisterSetting(std::string name, const std::string &default_value, OnChangeCallback callback, + Validation validation) { std::lock_guard settings_guard{settings_lock_}; MG_ASSERT(storage_); + MG_ASSERT(validation(default_value), "\"{}\"'s default value does not satisfy the validation condition.", name); if (const auto maybe_value = storage_->Get(name); maybe_value) { SPDLOG_INFO("The setting with name {} already exists!", name); } else { MG_ASSERT(storage_->Put(name, default_value), "Failed to register a setting"); } - - const auto [it, inserted] = on_change_callbacks_.emplace(std::move(name), callback); - MG_ASSERT(inserted, "Settings storage is out of sync"); + { + const auto [_, inserted] = on_change_callbacks_.emplace(name, callback); + MG_ASSERT(inserted, "Settings storage is out of sync"); + } + { + const auto [_, inserted] = validations_.emplace(std::move(name), validation); + MG_ASSERT(inserted, "Settings storage is out of sync"); + } } std::optional Settings::GetValue(const std::string &setting_name) const { @@ -59,6 +66,12 @@ bool Settings::SetValue(const std::string &setting_name, const std::string &new_ return std::nullopt; } + const auto val = validations_.find(setting_name); + MG_ASSERT(val != validations_.end(), "Settings storage is out of sync"); + if (!val->second(new_value)) { + throw utils::BasicException("'{}' not valid for '{}'", new_value, setting_name); + } + MG_ASSERT(storage_->Put(setting_name, new_value), "Failed to modify the setting"); const auto it = on_change_callbacks_.find(setting_name); diff --git a/src/utils/settings.hpp b/src/utils/settings.hpp index ed8889b01..19d92a0f2 100644 --- a/src/utils/settings.hpp +++ b/src/utils/settings.hpp @@ -1,4 +1,4 @@ -// Copyright 2022 Memgraph Ltd. +// Copyright 2023 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -22,12 +22,15 @@ namespace memgraph::utils { struct Settings { using OnChangeCallback = std::function; + using Validation = std::function; void Initialize(std::filesystem::path storage_path); // RocksDB depends on statically allocated objects so we need to delete it before the static destruction kicks in void Finalize(); - void RegisterSetting(std::string name, const std::string &default_value, OnChangeCallback callback); + void RegisterSetting( + std::string name, const std::string &default_value, OnChangeCallback callback, + Validation validation = [](auto) { return true; }); std::optional GetValue(const std::string &setting_name) const; bool SetValue(const std::string &setting_name, const std::string &new_value); std::vector> AllSettings() const; @@ -35,6 +38,7 @@ struct Settings { private: mutable utils::RWLock settings_lock_{RWLock::Priority::WRITE}; std::unordered_map on_change_callbacks_; + std::unordered_map validations_; std::optional storage_; }; diff --git a/tests/drivers/python/v5_8/run.sh b/tests/drivers/python/v5_8/run.sh index a22c5cb68..0231f4e0d 100755 --- a/tests/drivers/python/v5_8/run.sh +++ b/tests/drivers/python/v5_8/run.sh @@ -25,3 +25,4 @@ python3 docs_how_to_query.py || exit 1 python3 max_query_length.py || exit 1 python3 transactions.py || exit 1 python3 path.py || exit 1 +python3 server_name.py || exit 1 diff --git a/tests/drivers/python/v5_8/server_name.py b/tests/drivers/python/v5_8/server_name.py new file mode 100644 index 000000000..67b60e9f6 --- /dev/null +++ b/tests/drivers/python/v5_8/server_name.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +# Copyright 2021 Memgraph Ltd. +# +# Use of this software is governed by the Business Source License +# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +# License, and you may not use this file except in compliance with the Business Source License. +# +# As of the Change Date specified in that file, in accordance with +# the Business Source License, use of this software will be governed +# by the Apache License, Version 2.0, included in the file +# licenses/APL.txt. + +from neo4j import GraphDatabase, basic_auth +from neo4j.exceptions import ClientError, TransientError + + +def get_server_name(tx): + res = tx.run("SHOW DATABASE SETTINGS").values() + for setting in res: + if setting[0] == "server.name": + return setting[1] + assert False, "No setting named server.name" + + +def set_server_name(tx, name): + tx.run("SET DATABASE SETTING 'server.name' TO '{}'".format(name)).consume() + + +# Connect, check name, set a new name and recheck +with GraphDatabase.driver("bolt://localhost:7687", auth=None, encrypted=False) as driver: + with driver.session() as session: + default_name = get_server_name(session) + assert driver.get_server_info().agent == default_name, "Wrong server name! Expected {} and got {}".format( + default_name, driver.get_server_info().agent + ) + + with driver.session() as session: + set_server_name(session, "Neo4j/1.1 compatible database") + + +with GraphDatabase.driver("bolt://localhost:7687", auth=None, encrypted=False) as driver: + assert ( + driver.get_server_info().agent == "Neo4j/1.1 compatible database" + ), 'Wrong server name! Expected "Neo4j/1.1 compatible database" and got {}'.format(driver.get_server_info().agent) + + with driver.session() as session: + set_server_name(session, default_name) + + +print("All ok!") diff --git a/tests/drivers/python/v5_8/transactions.py b/tests/drivers/python/v5_8/transactions.py index 73f1d9d75..58f05eeb1 100644 --- a/tests/drivers/python/v5_8/transactions.py +++ b/tests/drivers/python/v5_8/transactions.py @@ -12,6 +12,8 @@ # by the Apache License, Version 2.0, included in the file # licenses/APL.txt. +import time + from neo4j import GraphDatabase, basic_auth from neo4j.exceptions import ClientError, TransientError @@ -35,6 +37,44 @@ def tx_too_long(tx): tx.run("MATCH (a), (b), (c), (d), (e), (f) RETURN COUNT(*) AS cnt") +def assert_timeout(set_timeout, measure_timeout): + print(measure_timeout) + print(set_timeout) + assert ( + measure_timeout >= set_timeout and measure_timeout < set_timeout * 1.2 + ), "Wrong timeout; expected {}s and measured {}s".format(set_timeout, measure_timeout) + + +def get_timeout(tx): + res = tx.run("SHOW DATABASE SETTINGS").values() + for setting in res: + if setting[0] == "query.timeout": + return float(setting[1]) + assert False, "No setting named query.timeout" + + +def set_timeout(tx, timeout): + tx.run("SET DATABASE SETTING 'query.timeout' TO '{}'".format(timeout)).consume() + + +def test_timeout(driver, set_timeout): + # Query that will run for a very long time, transient error expected. + timed_out = False + try: + with driver.session() as session: + start_time = time.time() + session.run("MATCH (a), (b), (c), (d), (e), (f) RETURN COUNT(*) AS cnt").consume() + except TransientError: + end_time = time.time() + assert_timeout(set_timeout, end_time - start_time) + timed_out = True + + if timed_out: + print("The query timed out as was expected.") + else: + raise Exception("The query should have timed out, but it didn't!") + + with GraphDatabase.driver("bolt://localhost:7687", auth=None, encrypted=False) as driver: def add_person(f, name, name2): @@ -53,17 +93,16 @@ with GraphDatabase.driver("bolt://localhost:7687", auth=None, encrypted=False) a with driver.session() as session: session.run("UNWIND range(1, 100000) AS x CREATE ()").consume() - # Query that will run for a very long time, transient error expected. - timed_out = False - try: - with driver.session() as session: - session.run("MATCH (a), (b), (c), (d), (e), (f) RETURN COUNT(*) AS cnt").consume() - except TransientError: - timed_out = True + # Test changing the timeout at run-time + with driver.session() as session: + default_timeout = get_timeout(session) + test_timeout(driver, default_timeout) - if timed_out: - print("The query timed out as was expected.") - else: - raise Exception("The query should have timed out, but it didn't!") + with driver.session() as session: + set_timeout(session, 1) + test_timeout(driver, 1) + + with driver.session() as session: + set_timeout(session, default_timeout) print("All ok!") diff --git a/tests/drivers/run.sh b/tests/drivers/run.sh index b5b26c16b..e6a12a89e 100755 --- a/tests/drivers/run.sh +++ b/tests/drivers/run.sh @@ -1,5 +1,9 @@ #!/bin/bash +# Old v1 tests +run_v1.sh + +# New tests pushd () { command pushd "$@" > /dev/null; } popd () { command popd "$@" > /dev/null; } @@ -30,7 +34,6 @@ $binary_dir/memgraph \ --query-execution-timeout-sec=5 \ --bolt-session-inactivity-timeout=10 \ --bolt-cert-file="" \ - --bolt-server-name-for-init="Neo4j/1.1" \ --log-file=$tmpdir/logs/memgarph.log \ --also-log-to-stderr \ --log-level ERROR & @@ -45,7 +48,8 @@ for i in *; do echo "Running: $i" # run all versions for v in *; do - if [ ! -d $v ]; then continue; fi + #skip v1 (needs different server name) + if [[ "$v" == "v1" || ! -d "$v" ]]; then continue; fi pushd $v echo "Running version: $v" ./run.sh diff --git a/tests/drivers/run_v1.sh b/tests/drivers/run_v1.sh new file mode 100755 index 000000000..d90d45891 --- /dev/null +++ b/tests/drivers/run_v1.sh @@ -0,0 +1,84 @@ +#!/bin/bash + +pushd () { command pushd "$@" > /dev/null; } +popd () { command popd "$@" > /dev/null; } + +function wait_for_server { + port=$1 + while ! nc -z -w 1 127.0.0.1 $port; do + sleep 0.1 + done + sleep 1 +} + +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +cd "$DIR" + +# Create a temporary directory. +tmpdir=/tmp/memgraph_drivers +if [ -d $tmpdir ]; then + rm -rf $tmpdir +fi +mkdir -p $tmpdir + +# Find memgraph binaries. +binary_dir="$DIR/../../build" + +# Start memgraph. +$binary_dir/memgraph \ + --data-directory=$tmpdir \ + --query-execution-timeout-sec=5 \ + --bolt-session-inactivity-timeout=10 \ + --bolt-server-name-for-init="Neo4j/1.1" \ + --bolt-cert-file="" \ + --log-file=$tmpdir/logs/memgarph.log \ + --also-log-to-stderr \ + --log-level ERROR & +pid=$! +wait_for_server 7687 + +# Run all available tests +code_test=0 +for i in *; do + if [ ! -d $i ]; then continue; fi + pushd $i + echo "Running: $i" + # run all versions + for v in *; do + # Run only v1 + if [[ "$v" != "v1" || ! -d "$v" ]]; then continue; fi + pushd $v + echo "Running version: $v" + ./run.sh + code_test=$? + if [ $code_test -ne 0 ]; then + echo "FAILED: $i" + break + fi + popd + done; + echo + popd +done + +# Stop memgraph. +kill $pid +wait $pid +code_mg=$? + +# Temporary directory cleanup. +if [ -d $tmpdir ]; then + rm -rf $tmpdir +fi + +# Check memgraph exit code. +if [ $code_mg -ne 0 ]; then + echo "The memgraph process didn't terminate properly!" + exit $code_mg +fi + +# Check test exit code. +if [ $code_test -ne 0 ]; then + echo "One of the tests failed!" + exit $code_test +fi diff --git a/tests/e2e/configuration/default_config.py b/tests/e2e/configuration/default_config.py index 093bd0dca..ba5e11c6e 100644 --- a/tests/e2e/configuration/default_config.py +++ b/tests/e2e/configuration/default_config.py @@ -117,8 +117,8 @@ startup_config_dict = { "password_encryption_algorithm": ("bcrypt", "bcrypt", "The password encryption algorithm used for authentication."), "pulsar_service_url": ("", "", "Default URL used while connecting to Pulsar brokers."), "query_execution_timeout_sec": ( - "600", - "600", + "-1", + "-1", "Maximum allowed query execution time. Queries exceeding this limit will be aborted. Value of 0 means no limit.", ), "query_modules_directory": ( diff --git a/tests/integration/CMakeLists.txt b/tests/integration/CMakeLists.txt index 7c93ad585..73d98ce6a 100644 --- a/tests/integration/CMakeLists.txt +++ b/tests/integration/CMakeLists.txt @@ -31,5 +31,8 @@ add_subdirectory(env_variable_check) #flag check binaries add_subdirectory(flag_check) -#flag check binaries +#storage mode binaries add_subdirectory(storage_mode) + +#run time settings binaries +add_subdirectory(run_time_settings) diff --git a/tests/integration/run_time_settings/CMakeLists.txt b/tests/integration/run_time_settings/CMakeLists.txt new file mode 100644 index 000000000..b04229cf4 --- /dev/null +++ b/tests/integration/run_time_settings/CMakeLists.txt @@ -0,0 +1,16 @@ +set(target_name memgraph__integration__executor) +set(tester_target_name ${target_name}__tester) +set(flag_tester_target_name ${target_name}__flag_tester) +set(executor_target_name ${target_name}__executor) + +add_executable(${tester_target_name} tester.cpp) +set_target_properties(${tester_target_name} PROPERTIES OUTPUT_NAME tester) +target_link_libraries(${tester_target_name} mg-communication) + +add_executable(${flag_tester_target_name} flag_tester.cpp) +set_target_properties(${flag_tester_target_name} PROPERTIES OUTPUT_NAME flag_tester) +target_link_libraries(${flag_tester_target_name} mg-communication) + +add_executable(${executor_target_name} executor.cpp) +set_target_properties(${executor_target_name} PROPERTIES OUTPUT_NAME executor) +target_link_libraries(${executor_target_name} mg-communication) diff --git a/tests/integration/run_time_settings/executor.cpp b/tests/integration/run_time_settings/executor.cpp new file mode 100644 index 000000000..90f761e79 --- /dev/null +++ b/tests/integration/run_time_settings/executor.cpp @@ -0,0 +1,53 @@ +// Copyright 2023 Memgraph Ltd. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +#include +#include + +#include "communication/bolt/client.hpp" +#include "io/network/endpoint.hpp" +#include "io/network/utils.hpp" +#include "utils/logging.hpp" + +DEFINE_string(address, "127.0.0.1", "Server address"); +DEFINE_int32(port, 7687, "Server port"); +DEFINE_string(username, "admin", "Username for the database"); +DEFINE_string(password, "admin", "Password for the database"); +DEFINE_bool(use_ssl, false, "Set to true to connect with SSL to the server."); + +/** + * Verifies that user 'user' has privileges that are given as positional + * arguments. + */ +int main(int argc, char **argv) { + gflags::ParseCommandLineFlags(&argc, &argv, true); + + memgraph::communication::SSLInit sslInit; + + memgraph::io::network::Endpoint endpoint(memgraph::io::network::ResolveHostname(FLAGS_address), FLAGS_port); + + memgraph::communication::ClientContext context(FLAGS_use_ssl); + memgraph::communication::bolt::Client client(context); + + client.Connect(endpoint, FLAGS_username, FLAGS_password); + + try { + std::string query(argv[1]); + auto ret = client.Execute(query, {}); + } catch (const memgraph::communication::bolt::ClientQueryException &e) { + LOG_FATAL( + "The query shouldn't have failed but it failed with an " + "error message '{}', {}", + e.what(), argv[0]); + } + + return 0; +} diff --git a/tests/integration/run_time_settings/flag_tester.cpp b/tests/integration/run_time_settings/flag_tester.cpp new file mode 100644 index 000000000..6031b26c8 --- /dev/null +++ b/tests/integration/run_time_settings/flag_tester.cpp @@ -0,0 +1,69 @@ +// Copyright 2023 Memgraph Ltd. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +#include + +#include "communication/bolt/client.hpp" +#include "communication/bolt/v1/value.hpp" +#include "io/network/endpoint.hpp" +#include "io/network/utils.hpp" +#include "utils/logging.hpp" + +DEFINE_string(address, "127.0.0.1", "Server address"); +DEFINE_int32(port, 7687, "Server port"); +DEFINE_string(field, "", "Expected settings field to check"); +DEFINE_string(value, "", "Expected string result from field"); + +/** + * Executes queries passed as positional arguments and verifies whether they + * succeeded, failed, failed with a specific error message or executed without a + * specific error occurring. + */ +int main(int argc, char **argv) { + gflags::ParseCommandLineFlags(&argc, &argv, true); + + memgraph::communication::SSLInit sslInit; + + memgraph::io::network::Endpoint endpoint(memgraph::io::network::ResolveHostname(FLAGS_address), FLAGS_port); + + memgraph::communication::ClientContext context(false); + memgraph::communication::bolt::Client client(context); + + try { + client.Connect(endpoint, "", ""); + } catch (const memgraph::utils::BasicException &e) { + LOG_FATAL(""); + } + + const auto &res = client.Execute("SHOW DATABASE SETTINGS", {}); + MG_ASSERT(res.fields[0] == "setting_name", "Expected \"setting_name\" field in the query result."); + MG_ASSERT(res.fields[1] == "setting_value", "Expected \"setting_value\" field in the query result."); + + unsigned i = 0; + for (const auto &record : res.records) { + const auto &settings_name = record[0].ValueString(); + if (settings_name == FLAGS_field) { + const auto &settings_value = record[1].ValueString(); + // First try to encode the flags as float; if that fails just compare the raw strings + try { + MG_ASSERT(std::stof(settings_value) == std::stof(FLAGS_value), + "Failed when checking \"{}\"; expected \"{}\", found \"{}\"!", FLAGS_field, FLAGS_value, + settings_value); + } catch (const std::invalid_argument &) { + MG_ASSERT(settings_value == FLAGS_value, "Failed when checking \"{}\"; expected \"{}\", found \"{}\"!", + FLAGS_field, FLAGS_value, settings_value); + } + return 0; + } + } + + LOG_FATAL("No setting named \"{}\" found!", FLAGS_field); +} diff --git a/tests/integration/run_time_settings/runner.py b/tests/integration/run_time_settings/runner.py new file mode 100755 index 000000000..98ee032f2 --- /dev/null +++ b/tests/integration/run_time_settings/runner.py @@ -0,0 +1,202 @@ +#!/usr/bin/python3 -u + +# Copyright 2022 Memgraph Ltd. +# +# Use of this software is governed by the Business Source License +# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +# License, and you may not use this file except in compliance with the Business Source License. +# +# As of the Change Date specified in that file, in accordance with +# the Business Source License, use of this software will be governed +# by the Apache License, Version 2.0, included in the file +# licenses/APL.txt. + +import argparse +import atexit +import fcntl +import os +import subprocess +import sys +import tempfile +import time +from typing import List + +SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) +PROJECT_DIR = os.path.normpath(os.path.join(SCRIPT_DIR, "..", "..", "..")) + + +def wait_for_server(port: int, delay: float = 0.1) -> float: + cmd = ["nc", "-z", "-w", "1", "127.0.0.1", str(port)] + while subprocess.call(cmd) != 0: + time.sleep(0.01) + time.sleep(delay) + + +def execute_tester( + binary, + queries, + should_fail=False, + failure_message="", + username="", + password="", + check_failure=True, + connection_should_fail=False, +): + args = [binary, "--username", username, "--password", password] + if should_fail: + args.append("--should-fail") + if failure_message: + args.extend(["--failure-message", failure_message]) + if check_failure: + args.append("--check-failure") + if connection_should_fail: + args.append("--connection-should-fail") + args.extend(queries) + subprocess.run(args).check_returncode() + + +def execute_query(binary: str, queries: List[str], username: str = "", password: str = "") -> None: + args = [binary, "--username", username, "--password", password] + args.extend(queries) + subprocess.run(args).check_returncode() + + +def make_non_blocking(fd): + flags = fcntl.fcntl(fd, fcntl.F_GETFL) + fcntl.fcntl(fd, fcntl.F_SETFL, flags | os.O_NONBLOCK) + + +def start_memgraph(memgraph_args: List[any]) -> subprocess: + memgraph = subprocess.Popen( + list(map(str, memgraph_args)), stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True + ) + time.sleep(0.1) + assert memgraph.poll() is None, "Memgraph process died prematurely!" + wait_for_server(7687) + # Make the stdout and stderr pipes non-blocking + make_non_blocking(memgraph.stdout.fileno()) + make_non_blocking(memgraph.stderr.fileno()) + return memgraph + + +def check_flag(tester_binary: str, flag: str, value: str) -> None: + args = [tester_binary, "--field", flag, "--value", value] + subprocess.run(args).check_returncode() + + +def cleanup(memgraph: subprocess): + if memgraph.poll() is None: + memgraph.terminate() + assert memgraph.wait() == 0, "Memgraph process didn't exit cleanly!" + + +def run_test(tester_binary: str, memgraph_args: List[str], server_name: str, query_tx: str): + memgraph = start_memgraph(memgraph_args) + atexit.register(cleanup, memgraph) + check_flag(tester_binary, "server.name", server_name) + check_flag(tester_binary, "query.timeout", query_tx) + cleanup(memgraph) + atexit.unregister(cleanup) + + +def run_test_w_query(tester_binary: str, memgraph_args: List[str], executor_binary: str): + memgraph = start_memgraph(memgraph_args) + atexit.register(cleanup, memgraph) + execute_query(executor_binary, ["SET DATABASE SETTING 'server.name' TO 'New Name';"]) + execute_query(executor_binary, ["SET DATABASE SETTING 'query.timeout' TO '123';"]) + check_flag(tester_binary, "server.name", "New Name") + check_flag(tester_binary, "query.timeout", "123") + cleanup(memgraph) + atexit.unregister(cleanup) + + +def consume(stream): + res = [] + while True: + line = stream.readline() + if not line: + break + res.append(line.strip()) + return res + + +def run_log_test(tester_binary: str, memgraph_args: List[str], executor_binary: str): + # Test if command line parameters work + memgraph = start_memgraph(memgraph_args + ["--log-level", "TRACE", "--also-log-to-stderr"]) + atexit.register(cleanup, memgraph) + std_err = consume(memgraph.stderr) + assert len(std_err) > 5, "Failed to log to stderr" + # Test if run-time setting log.to_stderr works + execute_query(executor_binary, ["SET DATABASE SETTING 'log.to_stderr' TO 'false';"]) + consume(memgraph.stderr) + execute_query(executor_binary, ["SET DATABASE SETTING 'query.timeout' TO '123';"]) + std_err = consume(memgraph.stderr) + assert len(std_err) == 0, "Still writing to stderr even after disabling it" + # Test if run-time setting log.level works + execute_query(executor_binary, ["SET DATABASE SETTING 'log.to_stderr' TO 'true';"]) + execute_query(executor_binary, ["SET DATABASE SETTING 'log.level' TO 'CRITICAL';"]) + consume(memgraph.stderr) + execute_query(executor_binary, ["SET DATABASE SETTING 'query.timeout' TO '123';"]) + std_err = consume(memgraph.stderr) + assert len(std_err) == 0, "Log level not updated" + # Tets that unsupported values cause an exception + execute_tester( + tester_binary, + ["SET DATABASE SETTING 'log.to_stderr' TO 'something'"], + should_fail=True, + failure_message="'something' not valid for 'log.to_stderr'", + ) + execute_tester( + tester_binary, + ["SET DATABASE SETTING 'log.level' TO 'something'"], + should_fail=True, + failure_message="'something' not valid for 'log.level'", + ) + cleanup(memgraph) + atexit.unregister(cleanup) + + +def execute_test(memgraph_binary: str, tester_binary: str, flag_tester_binary: str, executor_binary: str) -> None: + storage_directory = tempfile.TemporaryDirectory() + memgraph_args = [memgraph_binary, "--data-directory", storage_directory.name] + + print("\033[1;36m~~ Starting run-time settings check test ~~\033[0m") + + print("\033[1;34m~~ server.name and query.timeout ~~\033[0m") + # Check default flags + run_test(flag_tester_binary, memgraph_args, "Neo4j/v5.11.0 compatible graph database server - Memgraph", "600") + + # Check changing flags via command-line arguments + run_test( + flag_tester_binary, + memgraph_args + ["--bolt-server-name-for-init", "Memgraph", "--query-execution-timeout-sec", "1000"], + "Memgraph", + "1000", + ) + + # Check changing flags via query + run_test_w_query(flag_tester_binary, memgraph_args, executor_binary) + + print("\033[1;34m~~ log.level and log.to_stderr ~~\033[0m") + # Check log settings + run_log_test(tester_binary, memgraph_args, executor_binary) + + print("\033[1;36m~~ Finished run-time settings check test ~~\033[0m") + + +if __name__ == "__main__": + memgraph_binary = os.path.join(PROJECT_DIR, "build", "memgraph") + tester_binary = os.path.join(PROJECT_DIR, "build", "tests", "integration", "run_time_settings", "tester") + flag_tester_binary = os.path.join(PROJECT_DIR, "build", "tests", "integration", "run_time_settings", "flag_tester") + executor_binary = os.path.join(PROJECT_DIR, "build", "tests", "integration", "run_time_settings", "executor") + + parser = argparse.ArgumentParser() + parser.add_argument("--memgraph", default=memgraph_binary) + parser.add_argument("--tester", default=tester_binary) + parser.add_argument("--flag_tester", default=flag_tester_binary) + parser.add_argument("--executor", default=executor_binary) + args = parser.parse_args() + + execute_test(args.memgraph, args.tester, args.flag_tester, args.executor) + + sys.exit(0) diff --git a/tests/integration/run_time_settings/tester.cpp b/tests/integration/run_time_settings/tester.cpp new file mode 100644 index 000000000..0bd3295ab --- /dev/null +++ b/tests/integration/run_time_settings/tester.cpp @@ -0,0 +1,106 @@ +// Copyright 2023 Memgraph Ltd. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +// License, and you may not use this file except in compliance with the Business Source License. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +#include + +#include + +#include "communication/bolt/client.hpp" +#include "io/network/endpoint.hpp" +#include "io/network/utils.hpp" + +DEFINE_string(address, "127.0.0.1", "Server address"); +DEFINE_int32(port, 7687, "Server port"); +DEFINE_string(username, "", "Username for the database"); +DEFINE_string(password, "", "Password for the database"); +DEFINE_bool(use_ssl, false, "Set to true to connect with SSL to the server."); + +DEFINE_bool(check_failure, false, "Set to true to enable failure checking."); +DEFINE_bool(should_fail, false, "Set to true to expect a failure."); +DEFINE_bool(connection_should_fail, false, "Set to true to expect a connection failure."); +DEFINE_string(failure_message, "", "Set to the expected failure message."); + +/** + * Executes queries passed as positional arguments and verifies whether they + * succeeded, failed, failed with a specific error message or executed without a + * specific error occurring. + */ +int main(int argc, char **argv) { + gflags::ParseCommandLineFlags(&argc, &argv, true); + + memgraph::communication::SSLInit sslInit; + + memgraph::io::network::Endpoint endpoint(memgraph::io::network::ResolveHostname(FLAGS_address), FLAGS_port); + + memgraph::communication::ClientContext context(FLAGS_use_ssl); + memgraph::communication::bolt::Client client(context); + + std::regex re(FLAGS_failure_message); + + try { + client.Connect(endpoint, FLAGS_username, FLAGS_password); + } catch (const memgraph::communication::bolt::ClientFatalException &e) { + if (FLAGS_connection_should_fail) { + if (!FLAGS_failure_message.empty() && !std::regex_match(e.what(), re)) { + LOG_FATAL( + "The connection should have failed with an error message of '{}'' but " + "instead it failed with '{}'", + FLAGS_failure_message, e.what()); + } + return 0; + } else { + LOG_FATAL( + "The connection shoudn't have failed but it failed with an " + "error message '{}'", + e.what()); + } + } + + for (int i = 1; i < argc; ++i) { + std::string query(argv[i]); + try { + client.Execute(query, {}); + } catch (const memgraph::communication::bolt::ClientQueryException &e) { + if (!FLAGS_check_failure) { + if (!FLAGS_failure_message.empty() && std::regex_match(e.what(), re)) { + LOG_FATAL( + "The query should have succeeded or failed with an error " + "message that isn't equal to '{}' but it failed with that error " + "message", + FLAGS_failure_message); + } + continue; + } + if (FLAGS_should_fail) { + if (!FLAGS_failure_message.empty() && !std::regex_match(e.what(), re)) { + LOG_FATAL( + "The query should have failed with an error message of '{}'' but " + "instead it failed with '{}'", + FLAGS_failure_message, e.what()); + } + return 0; + } else { + LOG_FATAL( + "The query shoudn't have failed but it failed with an " + "error message '{}'", + e.what()); + } + } + if (!FLAGS_check_failure) continue; + if (FLAGS_should_fail) { + LOG_FATAL( + "The query should have failed but instead it executed " + "successfully!"); + } + } + + return 0; +} diff --git a/tests/unit/auth_handler.cpp b/tests/unit/auth_handler.cpp index 961c6722b..e3352220f 100644 --- a/tests/unit/auth_handler.cpp +++ b/tests/unit/auth_handler.cpp @@ -1,4 +1,4 @@ -// Copyright 2022 Memgraph Ltd. +// Copyright 2023 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -15,6 +15,7 @@ #include "auth/auth.hpp" #include "auth/models.hpp" +#include "glue/auth_global.hpp" #include "glue/auth_handler.hpp" #include "query/typed_value.hpp" #include "utils/file.hpp" diff --git a/tests/unit/interpreter.cpp b/tests/unit/interpreter.cpp index 8db925609..1115fd3be 100644 --- a/tests/unit/interpreter.cpp +++ b/tests/unit/interpreter.cpp @@ -17,6 +17,7 @@ #include "communication/result_stream_faker.hpp" #include "csv/parsing.hpp" #include "disk_test_utils.hpp" +#include "flags/run_time_configurable.hpp" #include "glue/communication.hpp" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -56,8 +57,10 @@ class InterpreterTest : public ::testing::Test { InterpreterTest() : data_directory(std::filesystem::temp_directory_path() / "MG_tests_unit_interpreter"), - interpreter_context(std::make_unique(disk_test_utils::GenerateOnDiskConfig(testSuite)), - {.execution_timeout_sec = 600}, data_directory) {} + interpreter_context(std::make_unique(disk_test_utils::GenerateOnDiskConfig(testSuite)), {}, + data_directory) { + memgraph::flags::run_time::execution_timeout_sec_ = 600.0; + } std::filesystem::path data_directory; memgraph::query::InterpreterContext interpreter_context;