Merge pull request #31607 from vitlibar/stop-reloading-all-configs-on-shutdown-early

Stop all periodic reloading of all the configuration files on shutdown earlier

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<std::shared_ptr<UsersConfigAccessStorage>>(storage))
+            users_config_storage->stopPeriodicReloading();
+    }
+}
+
 void AccessControl::addReplicatedStorage(
     const String & storage_name_,
     const String & zookeeper_path_,

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.

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<UUID> UsersConfigAccessStorage::findImpl(AccessEntityType type, const String & name) const
 {
     return memory_storage.find(type, name);

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);

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__);
     }
 }
 

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); }
 

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();