From 26e31ca06faead6d00e718c68563ae800dc7129d Mon Sep 17 00:00:00 2001 From: andrejtonev <29177572+andrejtonev@users.noreply.github.com> Date: Mon, 23 Oct 2023 10:18:07 +0200 Subject: [PATCH] Fix SHOW CONFIG to show the run-time flag status (#1278) --- src/flags/bolt.cpp | 4 - src/flags/bolt.hpp | 2 +- src/flags/general.cpp | 5 - src/flags/general.hpp | 2 +- src/flags/log_level.cpp | 28 +-- src/flags/log_level.hpp | 7 +- src/flags/run_time_configurable.cpp | 198 ++++++++++++------ src/flags/run_time_configurable.hpp | 24 ++- src/glue/CMakeLists.txt | 2 +- src/glue/SessionHL.cpp | 4 +- src/query/interpreter.cpp | 10 +- tests/e2e/configuration/default_config.py | 8 +- .../run_time_settings/CMakeLists.txt | 5 + .../run_time_settings/config_checker.cpp | 70 +++++++ tests/integration/run_time_settings/runner.py | 32 ++- tests/unit/interpreter.cpp | 2 +- 16 files changed, 294 insertions(+), 109 deletions(-) create mode 100644 tests/integration/run_time_settings/config_checker.cpp diff --git a/src/flags/bolt.cpp b/src/flags/bolt.cpp index eabc38e03..9401acf3d 100644 --- a/src/flags/bolt.cpp +++ b/src/flags/bolt.cpp @@ -36,7 +36,3 @@ 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 0556be8bc..ab1b8652e 100644 --- a/src/flags/bolt.hpp +++ b/src/flags/bolt.hpp @@ -26,4 +26,4 @@ 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); +// DECLARE_string(bolt_server_name_for_init); Moved to run_time_configurable diff --git a/src/flags/general.cpp b/src/flags/general.cpp index 01c7a4f12..eb8fae589 100644 --- a/src/flags/general.cpp +++ b/src/flags/general.cpp @@ -145,11 +145,6 @@ 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, -1, - "Maximum allowed query execution time. Queries exceeding this " - "limit will be aborted. Value of 0 means no limit."); - // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) DEFINE_uint64(replication_replica_check_frequency_sec, 1, "The time duration between two replica checks/pings. If < 1, replicas will NOT be checked at all. NOTE: " diff --git a/src/flags/general.hpp b/src/flags/general.hpp index 6963f980a..f9b8f3517 100644 --- a/src/flags/general.hpp +++ b/src/flags/general.hpp @@ -100,7 +100,7 @@ DECLARE_string(pulsar_service_url); // Query flags. // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -DECLARE_double(query_execution_timeout_sec); +// DECLARE_double(query_execution_timeout_sec); Moved to run_time_configurable // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) DECLARE_string(query_modules_directory); // NOLINTNEXTLINE (cppcoreguidelines-avoid-non-const-global-variables) diff --git a/src/flags/log_level.cpp b/src/flags/log_level.cpp index 1a4998b13..eb7b56620 100644 --- a/src/flags/log_level.cpp +++ b/src/flags/log_level.cpp @@ -26,7 +26,6 @@ using namespace std::string_view_literals; // Logging flags // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -DEFINE_HIDDEN_bool(also_log_to_stderr, false, "Log messages go to stderr in addition to logfiles"); DEFINE_string(log_file, "", "Path to where the log should be stored."); inline constexpr std::array log_level_mappings{ @@ -34,11 +33,8 @@ inline constexpr std::array log_level_mappings{ std::pair{"INFO"sv, spdlog::level::info}, std::pair{"WARNING"sv, spdlog::level::warn}, std::pair{"ERROR"sv, spdlog::level::err}, std::pair{"CRITICAL"sv, spdlog::level::critical}}; -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(), - { return memgraph::flags::ValidLogLevel(value); }); +const std::string memgraph::flags::log_level_help_string = fmt::format( + "Minimum log level. Allowed values: {}", memgraph::utils::GetAllowedEnumValuesString(log_level_mappings)); bool memgraph::flags::ValidLogLevel(std::string_view value) { if (const auto result = memgraph::utils::IsValidEnumValueString(value, log_level_mappings); result.HasError()) { @@ -65,7 +61,9 @@ std::optional memgraph::flags::LogLevelToEnum(std::st } spdlog::level::level_enum ParseLogLevel() { - const auto log_level = memgraph::flags::LogLevelToEnum(FLAGS_log_level); + std::string ll; + gflags::GetCommandLineOption("log_level", &ll); + const auto log_level = memgraph::flags::LogLevelToEnum(ll); MG_ASSERT(log_level, "Invalid log level"); return *log_level; } @@ -77,10 +75,6 @@ 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() { @@ -117,6 +111,14 @@ void memgraph::flags::AddLoggerSink(spdlog::sink_ptr new_sink) { // 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); + auto stderr = default_logger->sinks().front(); + stderr->set_level(log_level); +} + +void memgraph::flags::UpdateStderr(spdlog::level::level_enum log_level) { + auto default_logger = spdlog::default_logger(); + auto stderr = default_logger->sinks().front(); + if (stderr->level() != spdlog::level::off) { + stderr->set_level(log_level); + } } diff --git a/src/flags/log_level.hpp b/src/flags/log_level.hpp index 1f1824c86..9d70efe5a 100644 --- a/src/flags/log_level.hpp +++ b/src/flags/log_level.hpp @@ -14,16 +14,19 @@ #include #include "gflags/gflags.h" -DECLARE_string(log_level); -DECLARE_bool(also_log_to_stderr); +// DECLARE_string(log_level); Moved to run_time_configurable +// DECLARE_bool(also_log_to_stderr); Moved to run_time_configurable namespace memgraph::flags { +extern const std::string log_level_help_string; + 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); +void UpdateStderr(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 index 93cf6f3ce..a861769d8 100644 --- a/src/flags/run_time_configurable.cpp +++ b/src/flags/run_time_configurable.cpp @@ -10,102 +10,176 @@ // licenses/APL.txt. #include "flags/run_time_configurable.hpp" + #include +#include + +#include "gflags/gflags.h" + #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/flag_validation.hpp" #include "utils/settings.hpp" #include "utils/string.hpp" +/* + * Setup GFlags + */ + +// Bolt server flags. +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +DEFINE_string(bolt_server_name_for_init, "Neo4j/v5.11.0 compatible graph database server - Memgraph", + "Server name which the database should send to the client in the " + "Bolt INIT message."); + +// Logging flags +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +DEFINE_HIDDEN_bool(also_log_to_stderr, false, "Log messages go to stderr in addition to logfiles"); +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables, misc-unused-parameters) +DEFINE_VALIDATED_string(log_level, "WARNING", memgraph::flags::log_level_help_string.c_str(), + { return memgraph::flags::ValidLogLevel(value); }); + +// Query flags +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +DEFINE_double(query_execution_timeout_sec, 600, + "Maximum allowed query execution time. Queries exceeding this " + "limit will be aborted. Value of 0 means no limit."); + namespace { // Bolt server name constexpr auto kServerNameSettingKey = "server.name"; -constexpr auto kDefaultServerName = "Neo4j/v5.11.0 compatible graph database server - Memgraph"; +constexpr auto kServerNameGFlagsKey = "bolt_server_name_for_init"; // Query timeout constexpr auto kQueryTxSettingKey = "query.timeout"; -constexpr auto kDefaultQueryTx = "600"; // seconds +constexpr auto kQueryTxGFlagsKey = "query_execution_timeout_sec"; // Log level // No default value because it is not persistent constexpr auto kLogLevelSettingKey = "log.level"; +constexpr auto kLogLevelGFlagsKey = "log_level"; // Log to stderr // No default value because it is not persistent constexpr auto kLogToStderrSettingKey = "log.to_stderr"; +constexpr auto kLogToStderrGFlagsKey = "also_log_to_stderr"; + +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +std::atomic execution_timeout_sec_; // Local cache-like thing + +auto ToLLEnum(std::string_view val) { + const auto ll_enum = memgraph::flags::LogLevelToEnum(val); + if (!ll_enum) { + throw memgraph::utils::BasicException("Unsupported log level {}", val); + } + return *ll_enum; +} + +bool ValidBoolStr(std::string_view in) { + const auto lc = memgraph::utils::ToLowerCase(in); + return lc == "false" || lc == "true"; +} + +auto GenHandler(std::string flag, std::string key) { + return [key = std::move(key), flag = std::move(flag)]() -> std::string { + const auto &val = memgraph::utils::global_settings.GetValue(key); + MG_ASSERT(val, "Failed to read value at '{}' from settings.", key); + gflags::SetCommandLineOption(flag.c_str(), val->c_str()); + return *val; + }; +} + } // 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); - } + constexpr bool kRestore = true; //!< run-time flag is persistent between Memgraph restarts - // 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)); - } + /** + * @brief Helper function that registers a run-time flag + * + * @param flag - GFlag name + * @param key - Settings key used to store the flag + * @param restore - true if the flag is persistent between restarts + * @param post_update - user defined callback executed post flag update + * @param validator - user defined value correctness checker + */ + auto register_flag = [&]( + const std::string &flag, const std::string &key, bool restore, + std::function post_update = [](auto) {}, + std::function validator = [](std::string_view) { return true; }) { + // Get flag info + gflags::CommandLineFlagInfo info; + gflags::GetCommandLineFlagInfo(flag.c_str(), &info); + // Register setting + auto update = GenHandler(flag, key); + memgraph::utils::global_settings.RegisterSetting( + key, info.default_value, + [update, post_update = std::move(post_update)] { + const auto &val = update(); + post_update(val); + }, + validator); - // 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); + if (restore && info.is_default) { + // No input from the user, restore persistent value from settings + update(); + } else { + // Override with current value - user defined a new value or the run-time flag is not persistent between starts + memgraph::utils::global_settings.SetValue(key, info.current_value); } - 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()); + /* + * Register bolt server name settings + */ + register_flag(kServerNameGFlagsKey, kServerNameSettingKey, kRestore); + + /* + * Register query timeout + */ + register_flag(kQueryTxGFlagsKey, kQueryTxSettingKey, !kRestore, [&](const std::string &val) { + execution_timeout_sec_ = std::stod(val); // Cache for faster reads + }); + + /* + * Register log level + */ + register_flag( + kLogLevelGFlagsKey, kLogLevelSettingKey, !kRestore, + [](const std::string &val) { + const auto ll_enum = ToLLEnum(val); + spdlog::set_level(ll_enum); + UpdateStderr(ll_enum); // Updates level if active + }, + memgraph::flags::ValidLogLevel); + + /* + * Register logging to stderr + */ + register_flag( + kLogToStderrGFlagsKey, kLogToStderrSettingKey, !kRestore, + [](const std::string &val) { + if (val == "true") { + // No need to check if ll_val exists, we got here, so the log_level must exist already + const auto &ll_val = memgraph::utils::global_settings.GetValue(kLogLevelSettingKey); + LogToStderr(ToLLEnum(*ll_val)); } 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); + ValidBoolStr); } + +std::string GetServerName() { + std::string s; + // Thread safe read of gflag + gflags::GetCommandLineOption(kServerNameGFlagsKey, &s); + return s; +} + +double GetExecutionTimeout() { return execution_timeout_sec_; } + } // namespace memgraph::flags::run_time diff --git a/src/flags/run_time_configurable.hpp b/src/flags/run_time_configurable.hpp index 86c28721b..a45258853 100644 --- a/src/flags/run_time_configurable.hpp +++ b/src/flags/run_time_configurable.hpp @@ -15,12 +15,24 @@ 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_; - +/** + * @brief Initialize the run-time flags (must be done before run-time flags are used). + * + */ void Initialize(); +/** + * @brief Get the bolt server name value + * + * @return std::string + */ +std::string GetServerName(); + +/** + * @brief Get the query execution timeout value + * + * @return double + */ +double GetExecutionTimeout(); + } // namespace memgraph::flags::run_time diff --git a/src/glue/CMakeLists.txt b/src/glue/CMakeLists.txt index 83b0168ac..5940e607c 100644 --- a/src/glue/CMakeLists.txt +++ b/src/glue/CMakeLists.txt @@ -7,5 +7,5 @@ target_sources(mg-glue PRIVATE auth.cpp ServerT.cpp MonitoringServerT.cpp run_id.cpp) -target_link_libraries(mg-glue mg-query mg-auth mg-audit) +target_link_libraries(mg-glue mg-query mg-auth mg-audit mg-flags) target_precompile_headers(mg-glue INTERFACE auth_checker.hpp auth_handler.hpp) diff --git a/src/glue/SessionHL.cpp b/src/glue/SessionHL.cpp index c2051dde4..4b996af57 100644 --- a/src/glue/SessionHL.cpp +++ b/src/glue/SessionHL.cpp @@ -122,8 +122,8 @@ std::string SessionHL::GetCurrentDB() const { } std::optional SessionHL::GetServerNameForInit() { - auto locked_name = flags::run_time::bolt_server_name_.Lock(); - return locked_name->empty() ? std::nullopt : std::make_optional(*locked_name); + const auto &name = flags::run_time::GetServerName(); + return name.empty() ? std::nullopt : std::make_optional(name); } bool SessionHL::Authenticate(const std::string &username, const std::string &password) { diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp index 9cb34fbc4..03e4723e9 100644 --- a/src/query/interpreter.cpp +++ b/src/query/interpreter.cpp @@ -1433,7 +1433,7 @@ Interpreter::Interpreter(InterpreterContext *interpreter_context, memgraph::dbms auto DetermineTxTimeout(std::optional tx_timeout_ms, InterpreterConfig const &config) -> TxTimeout { using double_seconds = std::chrono::duration; - auto const global_tx_timeout = double_seconds{flags::run_time::execution_timeout_sec_}; + auto const global_tx_timeout = double_seconds{flags::run_time::GetExecutionTimeout()}; auto const valid_global_tx_timeout = global_tx_timeout > double_seconds{0}; if (tx_timeout_ms) { @@ -3918,7 +3918,7 @@ void RunTriggersAfterCommit(dbms::DatabaseAccess db_acc, InterpreterContext *int auto trigger_context = original_trigger_context; trigger_context.AdaptForAccessor(&db_accessor); try { - trigger.Execute(&db_accessor, &execution_memory, flags::run_time::execution_timeout_sec_, + trigger.Execute(&db_accessor, &execution_memory, flags::run_time::GetExecutionTimeout(), &interpreter_context->is_shutting_down, transaction_status, trigger_context, interpreter_context->auth_checker); } catch (const utils::BasicException &exception) { @@ -4032,9 +4032,9 @@ void Interpreter::Commit() { utils::MonotonicBufferResource execution_memory{kExecutionMemoryBlockSize}; AdvanceCommand(); try { - trigger.Execute(&*current_db_.execution_db_accessor_, &execution_memory, - flags::run_time::execution_timeout_sec_, &interpreter_context_->is_shutting_down, - &transaction_status_, *trigger_context, interpreter_context_->auth_checker); + trigger.Execute(&*current_db_.execution_db_accessor_, &execution_memory, flags::run_time::GetExecutionTimeout(), + &interpreter_context_->is_shutting_down, &transaction_status_, *trigger_context, + interpreter_context_->auth_checker); } catch (const utils::BasicException &e) { throw utils::BasicException( fmt::format("Trigger '{}' caused the transaction to fail.\nException: {}", trigger.Name(), e.what())); diff --git a/tests/e2e/configuration/default_config.py b/tests/e2e/configuration/default_config.py index d9c9bf769..8c056cab3 100644 --- a/tests/e2e/configuration/default_config.py +++ b/tests/e2e/configuration/default_config.py @@ -56,8 +56,8 @@ startup_config_dict = { ), "bolt_port": ("7687", "7687", "Port on which the Bolt server should listen."), "bolt_server_name_for_init": ( - "", - "", + "Neo4j/v5.11.0 compatible graph database server - Memgraph", + "Neo4j/v5.11.0 compatible graph database server - Memgraph", "Server name which the database should send to the client in the Bolt INIT message.", ), "bolt_session_inactivity_timeout": ( @@ -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": ( - "-1", - "-1", + "600", + "600", "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/run_time_settings/CMakeLists.txt b/tests/integration/run_time_settings/CMakeLists.txt index b04229cf4..8fc2fa311 100644 --- a/tests/integration/run_time_settings/CMakeLists.txt +++ b/tests/integration/run_time_settings/CMakeLists.txt @@ -2,6 +2,7 @@ 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) +set(config_checker_target_name ${target_name}__config_checker) add_executable(${tester_target_name} tester.cpp) set_target_properties(${tester_target_name} PROPERTIES OUTPUT_NAME tester) @@ -14,3 +15,7 @@ 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) + +add_executable(${config_checker_target_name} config_checker.cpp) +set_target_properties(${config_checker_target_name} PROPERTIES OUTPUT_NAME config_checker) +target_link_libraries(${config_checker_target_name} mg-communication) diff --git a/tests/integration/run_time_settings/config_checker.cpp b/tests/integration/run_time_settings/config_checker.cpp new file mode 100644 index 000000000..1f914fcad --- /dev/null +++ b/tests/integration/run_time_settings/config_checker.cpp @@ -0,0 +1,70 @@ +// 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(config, "", "Expected config 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 CONFIG", {}); + constexpr int kNameIdx = 0; + constexpr int kValueIdx = 2; + MG_ASSERT(res.fields[kNameIdx] == "name", "Expected \"name\" field in the query result."); + MG_ASSERT(res.fields[kValueIdx] == "current_value", "Expected \"current_value\" field in the query result."); + + for (const auto &record : res.records) { + const auto &settings_name = record[kNameIdx].ValueString(); + if (settings_name == FLAGS_config) { + const auto &settings_value = record[kValueIdx].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_config, FLAGS_value, + settings_value); + } catch (const std::invalid_argument &) { + MG_ASSERT(settings_value == FLAGS_value, "Failed when checking \"{}\"; expected \"{}\", found \"{}\"!", + FLAGS_config, FLAGS_value, settings_value); + } + return 0; + } + } + + LOG_FATAL("No setting named \"{}\" found!", FLAGS_config); +} diff --git a/tests/integration/run_time_settings/runner.py b/tests/integration/run_time_settings/runner.py index 98ee032f2..32e20d018 100755 --- a/tests/integration/run_time_settings/runner.py +++ b/tests/integration/run_time_settings/runner.py @@ -84,6 +84,11 @@ def check_flag(tester_binary: str, flag: str, value: str) -> None: subprocess.run(args).check_returncode() +def check_config(tester_binary: str, flag: str, value: str) -> None: + args = [tester_binary, "--config", flag, "--value", value] + subprocess.run(args).check_returncode() + + def cleanup(memgraph: subprocess): if memgraph.poll() is None: memgraph.terminate() @@ -156,7 +161,22 @@ def run_log_test(tester_binary: str, memgraph_args: List[str], executor_binary: atexit.unregister(cleanup) -def execute_test(memgraph_binary: str, tester_binary: str, flag_tester_binary: str, executor_binary: str) -> None: +def run_check_config(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';"]) + execute_query(executor_binary, ["SET DATABASE SETTING 'log.level' TO 'CRITICAL';"]) + check_config(tester_binary, "bolt_server_name_for_init", "New Name") + check_config(tester_binary, "query_execution_timeout_sec", "123") + check_config(tester_binary, "log_level", "CRITICAL") + cleanup(memgraph) + atexit.unregister(cleanup) + + +def execute_test( + memgraph_binary: str, tester_binary: str, flag_tester_binary: str, executor_binary: str, test_config_binary: str +) -> None: storage_directory = tempfile.TemporaryDirectory() memgraph_args = [memgraph_binary, "--data-directory", storage_directory.name] @@ -181,6 +201,10 @@ def execute_test(memgraph_binary: str, tester_binary: str, flag_tester_binary: s # Check log settings run_log_test(tester_binary, memgraph_args, executor_binary) + print("\033[1;34m~~ check show config ~~\033[0m") + # Check log settings + run_check_config(test_config_binary, memgraph_args, executor_binary) + print("\033[1;36m~~ Finished run-time settings check test ~~\033[0m") @@ -189,14 +213,18 @@ if __name__ == "__main__": 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") + config_checker_binary = os.path.join( + PROJECT_DIR, "build", "tests", "integration", "run_time_settings", "config_checker" + ) 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) + parser.add_argument("--config_checker", default=config_checker_binary) args = parser.parse_args() - execute_test(args.memgraph, args.tester, args.flag_tester, args.executor) + execute_test(args.memgraph, args.tester, args.flag_tester, args.executor, args.config_checker) sys.exit(0) diff --git a/tests/unit/interpreter.cpp b/tests/unit/interpreter.cpp index ff8df198e..c5d616e24 100644 --- a/tests/unit/interpreter.cpp +++ b/tests/unit/interpreter.cpp @@ -60,7 +60,7 @@ class InterpreterTest : public ::testing::Test { const std::string testSuiteCsv = "interpreter_csv"; std::filesystem::path data_directory = std::filesystem::temp_directory_path() / "MG_tests_unit_interpreter"; - InterpreterTest() : interpreter_context({}, kNoHandler) { memgraph::flags::run_time::execution_timeout_sec_ = 600.0; } + InterpreterTest() : interpreter_context({}, kNoHandler) {} memgraph::utils::Gatekeeper db_gk{ [&]() {