CLICKHOUSE-3553 Rebuild loggers on every config change (#1973)

* Macos test fix

* Fix freebsd build

* CLICKHOUSE-3553 Rebuild loggers on every config change

* rename clickhouse_common_configprocessor -> clickhouse_common_config; move Server/ConfigReloader.cpp -> Common/Config/

* CLICKHOUSE-3553 Set new level to all loggers on reload

* Dont use hash

* Update BaseDaemon.h

dbms/CMakeLists.txt

@@ -87,7 +87,7 @@ if (ARCH_FREEBSD)
 endif ()
 
 add_subdirectory(src/Common/ZooKeeper)
-add_subdirectory(src/Common/ConfigProcessor)
+add_subdirectory(src/Common/Config)
 
 if (MAKE_STATIC_LIBRARIES)
     add_library(dbms ${dbms_headers} ${dbms_sources})
@@ -141,7 +141,7 @@ target_link_libraries (clickhouse_common_io
 
 target_link_libraries (dbms
     clickhouse_parsers
-    clickhouse_common_configprocessor
+    clickhouse_common_config
     clickhouse_common_io
     ${MYSQLXX_LIBRARY}
     ${FARMHASH_LIBRARIES}

dbms/src/Common/Config/CMakeLists.txt

+
+include(${ClickHouse_SOURCE_DIR}/cmake/dbms_glob_sources.cmake)
+
+add_headers_and_sources(clickhouse_common_config .)
+
+add_library(clickhouse_common_config ${SPLIT_SHARED} ${clickhouse_common_config_headers} ${clickhouse_common_config_sources})
+
+target_link_libraries (clickhouse_common_config clickhouse_common_zookeeper string_utils)
+target_include_directories (clickhouse_common_config PRIVATE ${DBMS_INCLUDE_DIR})

dbms/src/Server/ConfigReloader.cpp → dbms/src/Common/Config/ConfigReloader.cpp

@@ -2,12 +2,9 @@
 
 #include <Poco/Util/Application.h>
 #include <Poco/File.h>
-
 #include <common/logger_useful.h>
-
-#include <Interpreters/Context.h>
 #include <Common/setThreadName.h>
-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include "ConfigProcessor.h"
 
 
 namespace DB

dbms/src/Server/ConfigReloader.h → dbms/src/Common/Config/ConfigReloader.h

 #pragma once
 
-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include "ConfigProcessor.h"
 #include <Common/ZooKeeper/Common.h>
 #include <Common/ZooKeeper/ZooKeeperNodeCache.h>
-
 #include <time.h>
 #include <string>
 #include <thread>

dbms/src/Common/ConfigProcessor/CMakeLists.txt

-
-include(${ClickHouse_SOURCE_DIR}/cmake/dbms_glob_sources.cmake)
-
-add_headers_and_sources(clickhouse_common_configprocessor .)
-
-add_library(clickhouse_common_configprocessor ${SPLIT_SHARED} ${clickhouse_common_configprocessor_headers} ${clickhouse_common_configprocessor_sources})
-
-target_link_libraries (clickhouse_common_configprocessor clickhouse_common_zookeeper string_utils)
-target_include_directories (clickhouse_common_configprocessor PRIVATE ${DBMS_INCLUDE_DIR})

dbms/src/Common/ZooKeeper/tests/CMakeLists.txt

@@ -14,4 +14,4 @@ add_executable(zkutil_zookeeper_holder zkutil_zookeeper_holder.cpp)
 target_link_libraries(zkutil_zookeeper_holder clickhouse_common_zookeeper)
 
 add_executable (zk_many_watches_reconnect zk_many_watches_reconnect.cpp)
-target_link_libraries (zk_many_watches_reconnect clickhouse_common_zookeeper clickhouse_common_configprocessor)
+target_link_libraries (zk_many_watches_reconnect clickhouse_common_zookeeper clickhouse_common_config)

dbms/src/Common/ZooKeeper/tests/zk_many_watches_reconnect.cpp

-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include <Common/Config/ConfigProcessor.h>
 #include <Common/ZooKeeper/ZooKeeper.h>
 #include <Poco/Event.h>
 #include <iostream>

dbms/src/Interpreters/Context.cpp

@@ -45,7 +45,7 @@
 #include <Parsers/ParserCreateQuery.h>
 #include <Parsers/parseQuery.h>
 
-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include <Common/Config/ConfigProcessor.h>
 #include <Common/ZooKeeper/ZooKeeper.h>
 #include <common/logger_useful.h>
 

dbms/src/Interpreters/ExternalLoaderConfigRepository.cpp

 #include <Interpreters/ExternalLoaderConfigRepository.h>
 
 #include <Common/StringUtils/StringUtils.h>
-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include <Common/Config/ConfigProcessor.h>
 #include <Common/getMultipleKeysFromConfig.h>
 
 #include <Poco/Glob.h>

dbms/src/Interpreters/tests/users.cpp

-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include <Common/Config/ConfigProcessor.h>
 #include <Interpreters/SecurityManager.h>
 
 #include <boost/filesystem.hpp>

dbms/src/Server/CMakeLists.txt

@@ -15,7 +15,6 @@ option (ENABLE_CLICKHOUSE_COPIER "Enable copier" ${ENABLE_CLICKHOUSE_COPIER})
 configure_file (config_tools.h.in ${CMAKE_CURRENT_BINARY_DIR}/config_tools.h)
 
 add_library (clickhouse-server-lib
-    ConfigReloader.cpp
     HTTPHandler.cpp
     InterserverIOHTTPHandler.cpp
     MetricsTransmitter.cpp
@@ -35,7 +34,7 @@ add_library (clickhouse-local-lib LocalServer.cpp)
 target_link_libraries (clickhouse-local-lib clickhouse-server-lib clickhouse_functions clickhouse_aggregate_functions clickhouse_table_functions)
 
 add_library (clickhouse-extract-from-config-lib ${SPLIT_SHARED} ExtractFromConfig.cpp)
-target_link_libraries (clickhouse-extract-from-config-lib clickhouse_common_configprocessor clickhouse_common_io ${Boost_PROGRAM_OPTIONS_LIBRARY})
+target_link_libraries (clickhouse-extract-from-config-lib clickhouse_common_config clickhouse_common_io ${Boost_PROGRAM_OPTIONS_LIBRARY})
 
 add_library (clickhouse-client-lib Client.cpp)
 target_link_libraries (clickhouse-client-lib clickhouse_functions clickhouse_aggregate_functions clickhouse_table_functions ${LINE_EDITING_LIBS} ${Boost_PROGRAM_OPTIONS_LIBRARY})

dbms/src/Server/ExtractFromConfig.cpp

@@ -9,7 +9,7 @@
 #include <Poco/Util/XMLConfiguration.h>
 
 #include <Common/ZooKeeper/ZooKeeperNodeCache.h>
-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include <Common/Config/ConfigProcessor.h>
 #include <Common/Exception.h>
 
 

dbms/src/Server/LocalServer.cpp

@@ -14,7 +14,7 @@
 #include <Interpreters/loadMetadata.h>
 #include <Common/Exception.h>
 #include <Common/Macros.h>
-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include <Common/Config/ConfigProcessor.h>
 #include <Common/escapeForFileName.h>
 #include <IO/ReadBufferFromString.h>
 #include <IO/WriteBufferFromString.h>

dbms/src/Server/Server.cpp

@@ -30,7 +30,7 @@
 #include <Functions/registerFunctions.h>
 #include <TableFunctions/registerTableFunctions.h>
 #include <Storages/registerStorages.h>
-#include "ConfigReloader.h"
+#include <Common/Config/ConfigReloader.h>
 #include "HTTPHandlerFactory.h"
 #include "MetricsTransmitter.h"
 #include "StatusFile.h"
@@ -228,7 +228,11 @@ int Server::main(const std::vector<std::string> & /*args*/)
     auto main_config_reloader = std::make_unique<ConfigReloader>(config_path,
         include_from_path,
         std::move(main_config_zk_node_cache),
-        [&](ConfigurationPtr config) { global_context->setClustersConfig(config); },
+        [&](ConfigurationPtr config)
+        {
+            buildLoggers(*config);
+            global_context->setClustersConfig(config);
+        },
         /* already_loaded = */ true);
 
     /// Initialize users config reloader.

dbms/src/Server/config.xml

 <?xml version="1.0"?>
 <yandex>
     <logger>
+        <!-- Possible levels: https://github.com/pocoproject/poco/blob/develop/Foundation/include/Poco/Logger.h#L105 -->
         <level>trace</level>
         <log>/var/log/clickhouse-server/clickhouse-server.log</log>
         <errorlog>/var/log/clickhouse-server/clickhouse-server.err.log</errorlog>

libs/libdaemon/CMakeLists.txt

@@ -17,4 +17,4 @@ endif ()
 target_include_directories (daemon PUBLIC include)
 target_include_directories (daemon PRIVATE ${ClickHouse_SOURCE_DIR}/libs/libpocoext/include)
 
-target_link_libraries (daemon clickhouse_common_io clickhouse_common_configprocessor ${EXECINFO_LIBRARY})
+target_link_libraries (daemon clickhouse_common_io clickhouse_common_config ${EXECINFO_LIBRARY})

libs/libdaemon/include/daemon/BaseDaemon.h

@@ -23,7 +23,7 @@
 #include <common/Types.h>
 #include <common/logger_useful.h>
 #include <daemon/GraphiteWriter.h>
-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include <Common/Config/ConfigProcessor.h>
 
 namespace Poco { class TaskManager; }
 
@@ -67,7 +67,7 @@ public:
     void reloadConfiguration();
 
     /// Строит необходимые логгеры
-    void buildLoggers();
+    void buildLoggers(Poco::Util::AbstractConfiguration & config);
 
     /// Определяет параметр командной строки
     void defineOptions(Poco::Util::OptionSet & _options) override;
@@ -228,6 +228,11 @@ protected:
     std::string config_path;
     ConfigProcessor::LoadedConfig loaded_config;
     Poco::Util::AbstractConfiguration * last_configuration = nullptr;
+
+private:
+
+    /// Previous value of logger element in config. It is used to reinitialize loggers whenever the value changed.
+    std::string config_logger;
 };
 
 

libs/libdaemon/src/BaseDaemon.cpp

 #include <daemon/BaseDaemon.h>
 
-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include <Common/Config/ConfigProcessor.h>
 
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -565,67 +565,58 @@ void BaseDaemon::wakeup()
 }
 
 
-void BaseDaemon::buildLoggers()
+void BaseDaemon::buildLoggers(Poco::Util::AbstractConfiguration & config)
 {
-    bool is_daemon = config().getBool("application.runAsDaemon", false);
+    auto current_logger = config.getString("logger");
+    if (config_logger == current_logger)
+        return;
+    config_logger = current_logger;
 
-    /// Change path for logging.
-    if (config().hasProperty("logger.log"))
-    {
-        std::string path = createDirectory(config().getString("logger.log"));
-        if (is_daemon
-            && chdir(path.c_str()) != 0)
-            throw Poco::Exception("Cannot change directory to " + path);
-    }
-    else
-    {
-        if (is_daemon
-            && chdir("/tmp") != 0)
-            throw Poco::Exception("Cannot change directory to /tmp");
-    }
+    bool is_daemon = config.getBool("application.runAsDaemon", false);
 
     // Split log and error log.
     Poco::AutoPtr<SplitterChannel> split = new SplitterChannel;
 
-    if (config().hasProperty("logger.log"))
+    auto log_level = config.getString("logger.level", "trace");
+    if (config.hasProperty("logger.log"))
     {
-        createDirectory(config().getString("logger.log"));
-        std::cerr << "Logging to " << config().getString("logger.log") << std::endl;
+        createDirectory(config.getString("logger.log"));
+        std::cerr << "Logging " << log_level << " to " << config.getString("logger.log") << std::endl;
 
         // Set up two channel chains.
         Poco::AutoPtr<OwnPatternFormatter> pf = new OwnPatternFormatter(this);
         pf->setProperty("times", "local");
         Poco::AutoPtr<FormattingChannel> log = new FormattingChannel(pf);
         log_file = new FileChannel;
-        log_file->setProperty(Poco::FileChannel::PROP_PATH, Poco::Path(config().getString("logger.log")).absolute().toString());
-        log_file->setProperty(Poco::FileChannel::PROP_ROTATION, config().getRawString("logger.size", "100M"));
+        log_file->setProperty(Poco::FileChannel::PROP_PATH, Poco::Path(config.getString("logger.log")).absolute().toString());
+        log_file->setProperty(Poco::FileChannel::PROP_ROTATION, config.getRawString("logger.size", "100M"));
         log_file->setProperty(Poco::FileChannel::PROP_ARCHIVE, "number");
-        log_file->setProperty(Poco::FileChannel::PROP_COMPRESS, config().getRawString("logger.compress", "true"));
-        log_file->setProperty(Poco::FileChannel::PROP_PURGECOUNT, config().getRawString("logger.count", "1"));
-        log_file->setProperty(Poco::FileChannel::PROP_FLUSH, config().getRawString("logger.flush", "true"));
-        log_file->setProperty(Poco::FileChannel::PROP_ROTATEONOPEN, config().getRawString("logger.rotateOnOpen", "false"));
+        log_file->setProperty(Poco::FileChannel::PROP_COMPRESS, config.getRawString("logger.compress", "true"));
+        log_file->setProperty(Poco::FileChannel::PROP_PURGECOUNT, config.getRawString("logger.count", "1"));
+        log_file->setProperty(Poco::FileChannel::PROP_FLUSH, config.getRawString("logger.flush", "true"));
+        log_file->setProperty(Poco::FileChannel::PROP_ROTATEONOPEN, config.getRawString("logger.rotateOnOpen", "false"));
         log->setChannel(log_file);
         split->addChannel(log);
         log_file->open();
     }
 
-    if (config().hasProperty("logger.errorlog"))
+    if (config.hasProperty("logger.errorlog"))
     {
-        createDirectory(config().getString("logger.errorlog"));
-        std::cerr << "Logging errors to " << config().getString("logger.errorlog") << std::endl;
+        createDirectory(config.getString("logger.errorlog"));
+        std::cerr << "Logging errors to " << config.getString("logger.errorlog") << std::endl;
         Poco::AutoPtr<Poco::LevelFilterChannel> level = new Poco::LevelFilterChannel;
         level->setLevel(Message::PRIO_NOTICE);
         Poco::AutoPtr<OwnPatternFormatter> pf = new OwnPatternFormatter(this);
         pf->setProperty("times", "local");
         Poco::AutoPtr<FormattingChannel> errorlog = new FormattingChannel(pf);
         error_log_file = new FileChannel;
-        error_log_file->setProperty(Poco::FileChannel::PROP_PATH, Poco::Path(config().getString("logger.errorlog")).absolute().toString());
-        error_log_file->setProperty(Poco::FileChannel::PROP_ROTATION, config().getRawString("logger.size", "100M"));
+        error_log_file->setProperty(Poco::FileChannel::PROP_PATH, Poco::Path(config.getString("logger.errorlog")).absolute().toString());
+        error_log_file->setProperty(Poco::FileChannel::PROP_ROTATION, config.getRawString("logger.size", "100M"));
         error_log_file->setProperty(Poco::FileChannel::PROP_ARCHIVE, "number");
-        error_log_file->setProperty(Poco::FileChannel::PROP_COMPRESS, config().getRawString("logger.compress", "true"));
-        error_log_file->setProperty(Poco::FileChannel::PROP_PURGECOUNT, config().getRawString("logger.count", "1"));
-        error_log_file->setProperty(Poco::FileChannel::PROP_FLUSH, config().getRawString("logger.flush", "true"));
-        error_log_file->setProperty(Poco::FileChannel::PROP_ROTATEONOPEN, config().getRawString("logger.rotateOnOpen", "false"));
+        error_log_file->setProperty(Poco::FileChannel::PROP_COMPRESS, config.getRawString("logger.compress", "true"));
+        error_log_file->setProperty(Poco::FileChannel::PROP_PURGECOUNT, config.getRawString("logger.count", "1"));
+        error_log_file->setProperty(Poco::FileChannel::PROP_FLUSH, config.getRawString("logger.flush", "true"));
+        error_log_file->setProperty(Poco::FileChannel::PROP_ROTATEONOPEN, config.getRawString("logger.rotateOnOpen", "false"));
         errorlog->setChannel(error_log_file);
         level->setChannel(errorlog);
         split->addChannel(level);
@@ -635,7 +626,7 @@ void BaseDaemon::buildLoggers()
     /// "dynamic_layer_selection" is needed only for Yandex.Metrika, that share part of ClickHouse code.
     /// We don't need this configuration parameter.
 
-    if (config().getBool("logger.use_syslog", false) || config().getBool("dynamic_layer_selection", false))
+    if (config.getBool("logger.use_syslog", false) || config.getBool("dynamic_layer_selection", false))
     {
         Poco::AutoPtr<OwnPatternFormatter> pf = new OwnPatternFormatter(this, OwnPatternFormatter::ADD_LAYER_TAG);
         pf->setProperty("times", "local");
@@ -646,14 +637,14 @@ void BaseDaemon::buildLoggers()
         syslog_channel->open();
     }
 
-    if (config().getBool("logger.console", false) || (!config().hasProperty("logger.console") && !is_daemon && (isatty(STDIN_FILENO) || isatty(STDERR_FILENO))))
+    if (config.getBool("logger.console", false) || (!config.hasProperty("logger.console") && !is_daemon && (isatty(STDIN_FILENO) || isatty(STDERR_FILENO))))
     {
         Poco::AutoPtr<ConsoleChannel> file = new ConsoleChannel;
         Poco::AutoPtr<OwnPatternFormatter> pf = new OwnPatternFormatter(this);
         pf->setProperty("times", "local");
         Poco::AutoPtr<FormattingChannel> log = new FormattingChannel(pf);
         log->setChannel(file);
-        logger().warning("Logging to console");
+        logger().warning("Logging " + log_level + " to console");
         split->addChannel(log);
     }
 
@@ -662,19 +653,25 @@ void BaseDaemon::buildLoggers()
     logger().setChannel(split);
 
     // Global logging level (it can be overridden for specific loggers).
-    logger().setLevel(config().getString("logger.level", "trace"));
+    logger().setLevel(log_level);
+
+    // Set level to all already created loggers
+    std::vector <std::string> names;
+    Logger::root().names(names);
+    for (const auto & name : names)
+        Logger::root().get(name).setLevel(log_level);
 
     // Attach to the root logger.
-    Logger::root().setLevel(logger().getLevel());
+    Logger::root().setLevel(log_level);
     Logger::root().setChannel(logger().getChannel());
 
     // Explicitly specified log levels for specific loggers.
     AbstractConfiguration::Keys levels;
-    config().keys("logger.levels", levels);
+    config.keys("logger.levels", levels);
 
     if(!levels.empty())
         for(AbstractConfiguration::Keys::iterator it = levels.begin(); it != levels.end(); ++it)
-            Logger::get(*it).setLevel(config().getString("logger.levels." + *it, "trace"));
+            Logger::get(*it).setLevel(config.getString("logger.levels." + *it, "trace"));
 }
 
 
@@ -839,7 +836,22 @@ void BaseDaemon::initialize(Application & self)
             pid.seed(config().getString("pid"));
     }
 
-    buildLoggers();
+    /// Change path for logging.
+    if (config().hasProperty("logger.log"))
+    {
+        std::string path = createDirectory(config().getString("logger.log"));
+        if (is_daemon
+            && chdir(path.c_str()) != 0)
+            throw Poco::Exception("Cannot change directory to " + path);
+    }
+    else
+    {
+        if (is_daemon
+            && chdir("/tmp") != 0)
+            throw Poco::Exception("Cannot change directory to /tmp");
+    }
+
+    buildLoggers(config());
 
     if (is_daemon)
     {

utils/config-processor/CMakeLists.txt

 add_executable (config-processor config-processor.cpp)
-target_link_libraries (config-processor clickhouse_common_configprocessor)
+target_link_libraries (config-processor clickhouse_common_config)
 
 INSTALL(TARGETS config-processor RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT config-processor)

utils/config-processor/config-processor.cpp

-#include <Common/ConfigProcessor/ConfigProcessor.h>
+#include <Common/Config/ConfigProcessor.h>
 #include <iostream>
 
 int main(int argc, char ** argv)