From 2f8c8293955b32c1ac7895021f55acf20f346693 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Sun, 21 Nov 2021 22:39:36 +0300 Subject: [PATCH] Stop all periodic reloading of all the configuration files on shutdown earlier. --- src/Access/AccessControl.cpp | 10 +++++++++ src/Access/AccessControl.h | 1 + src/Access/UsersConfigAccessStorage.cpp | 7 +++++++ src/Access/UsersConfigAccessStorage.h | 1 + src/Common/Config/ConfigReloader.cpp | 28 ++++++++++++++++++------- src/Common/Config/ConfigReloader.h | 6 +++++- src/Interpreters/Context.cpp | 13 +++++++++++- 7 files changed, 57 insertions(+), 9 deletions(-) diff --git a/src/Access/AccessControl.cpp b/src/Access/AccessControl.cpp index 54ba8853b17..da1fd94239a 100644 --- a/src/Access/AccessControl.cpp +++ b/src/Access/AccessControl.cpp @@ -229,6 +229,16 @@ void AccessControl::startPeriodicReloadingUsersConfigs() } } +void AccessControl::stopPeriodicReloadingUsersConfigs() +{ + auto storages = getStoragesPtr(); + for (const auto & storage : *storages) + { + if (auto users_config_storage = typeid_cast>(storage)) + users_config_storage->stopPeriodicReloading(); + } +} + void AccessControl::addReplicatedStorage( const String & storage_name_, const String & zookeeper_path_, diff --git a/src/Access/AccessControl.h b/src/Access/AccessControl.h index 12e9986a13a..d891432266e 100644 --- a/src/Access/AccessControl.h +++ b/src/Access/AccessControl.h @@ -71,6 +71,7 @@ public: void reloadUsersConfigs(); void startPeriodicReloadingUsersConfigs(); + void stopPeriodicReloadingUsersConfigs(); /// Loads access entities from the directory on the local disk. /// Use that directory to keep created users/roles/etc. diff --git a/src/Access/UsersConfigAccessStorage.cpp b/src/Access/UsersConfigAccessStorage.cpp index dbaf4e002b1..7b4ff2d3296 100644 --- a/src/Access/UsersConfigAccessStorage.cpp +++ b/src/Access/UsersConfigAccessStorage.cpp @@ -591,6 +591,13 @@ void UsersConfigAccessStorage::startPeriodicReloading() config_reloader->start(); } +void UsersConfigAccessStorage::stopPeriodicReloading() +{ + std::lock_guard lock{load_mutex}; + if (config_reloader) + config_reloader->stop(); +} + std::optional UsersConfigAccessStorage::findImpl(AccessEntityType type, const String & name) const { return memory_storage.find(type, name); diff --git a/src/Access/UsersConfigAccessStorage.h b/src/Access/UsersConfigAccessStorage.h index 7fb08790f77..8f87e5ad928 100644 --- a/src/Access/UsersConfigAccessStorage.h +++ b/src/Access/UsersConfigAccessStorage.h @@ -39,6 +39,7 @@ public: const zkutil::GetZooKeeper & get_zookeeper_function = {}); void reload(); void startPeriodicReloading(); + void stopPeriodicReloading(); private: void parseFromConfig(const Poco::Util::AbstractConfiguration & config); diff --git a/src/Common/Config/ConfigReloader.cpp b/src/Common/Config/ConfigReloader.cpp index 25e0ff7221c..0ec67a7976b 100644 --- a/src/Common/Config/ConfigReloader.cpp +++ b/src/Common/Config/ConfigReloader.cpp @@ -36,7 +36,25 @@ ConfigReloader::ConfigReloader( void ConfigReloader::start() { - thread = ThreadFromGlobalPool(&ConfigReloader::run, this); + std::lock_guard lock(reload_mutex); + if (!thread.joinable()) + { + quit = false; + thread = ThreadFromGlobalPool(&ConfigReloader::run, this); + } +} + + +void ConfigReloader::stop() +{ + std::unique_lock lock(reload_mutex); + if (!thread.joinable()) + return; + quit = true; + zk_changed_event->set(); + auto temp_thread = std::move(thread); + lock.unlock(); + temp_thread.join(); } @@ -44,15 +62,11 @@ ConfigReloader::~ConfigReloader() { try { - quit = true; - zk_changed_event->set(); - - if (thread.joinable()) - thread.join(); + stop(); } catch (...) { - DB::tryLogCurrentException(__PRETTY_FUNCTION__); + tryLogCurrentException(log, __PRETTY_FUNCTION__); } } diff --git a/src/Common/Config/ConfigReloader.h b/src/Common/Config/ConfigReloader.h index 2e4399d3c4e..841c0fe57e1 100644 --- a/src/Common/Config/ConfigReloader.h +++ b/src/Common/Config/ConfigReloader.h @@ -42,9 +42,13 @@ public: ~ConfigReloader(); - /// Call this method to run the background thread. + /// Starts periodic reloading in the background thread. void start(); + /// Stops periodic reloading reloading in the background thread. + /// This function is automatically called by the destructor. + void stop(); + /// Reload immediately. For SYSTEM RELOAD CONFIG query. void reload() { reloadIfNewer(/* force */ true, /* throw_on_error */ true, /* fallback_to_preprocessed */ false, /* initial_loading = */ false); } diff --git a/src/Interpreters/Context.cpp b/src/Interpreters/Context.cpp index 9bf66a6ac9c..9c003efdf71 100644 --- a/src/Interpreters/Context.cpp +++ b/src/Interpreters/Context.cpp @@ -307,12 +307,23 @@ struct ContextSharedPart return; shutdown_called = true; + /// Stop periodic reloading of the configuration files. + /// This must be done first because otherwise the reloading may pass a changed config + /// to some destroyed parts of ContextSharedPart. + if (access_control) + access_control->stopPeriodicReloadingUsersConfigs(); + if (external_dictionaries_loader) + external_dictionaries_loader->enablePeriodicUpdates(false); + if (external_user_defined_executable_functions_loader) + external_user_defined_executable_functions_loader->enablePeriodicUpdates(false); + if (external_models_loader) + external_models_loader->enablePeriodicUpdates(false); + Session::shutdownNamedSessions(); /** After system_logs have been shut down it is guaranteed that no system table gets created or written to. * Note that part changes at shutdown won't be logged to part log. */ - if (system_logs) system_logs->shutdown();