Merge pull request #3947 from yandex/fix-config-reloader

Retry loading ZK substitutions after ZK errors
ClickHouse · Dec 27, 2018 · 3455da0 · 3455da0
2 parents cbc9e03 + da00c6e
commit 3455da0
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 1 deletion.
diff --git a/dbms/src/Common/Config/ConfigProcessor.cpp b/dbms/src/Common/Config/ConfigProcessor.cpp
@@ -447,6 +447,11 @@ XMLDocumentPtr ConfigProcessor::processConfig(
             merge(config, with);
             contributing_files.push_back(merge_file);
         }
+        catch (Exception & e)
+        {
+            e.addMessage("while merging config '" + path + "' with '" + merge_file + "'");
+            throw;
+        }
         catch (Poco::Exception & e)
         {
             throw Poco::Exception("Failed to merge config with '" + merge_file + "': " + e.displayText());
@@ -479,6 +484,11 @@ XMLDocumentPtr ConfigProcessor::processConfig(
 
         doIncludesRecursive(config, include_from, getRootNode(config.get()), zk_node_cache, zk_changed_event, contributing_zk_paths);
     }
+    catch (Exception & e)
+    {
+        e.addMessage("while preprocessing config '" + path + "'");
+        throw;
+    }
     catch (Poco::Exception & e)
     {
         throw Poco::Exception("Failed to preprocess config '" + path + "': " + e.displayText(), e);

diff --git a/dbms/src/Common/Config/ConfigReloader.cpp b/dbms/src/Common/Config/ConfigReloader.cpp
@@ -81,7 +81,7 @@ void ConfigReloader::reloadIfNewer(bool force, bool throw_on_error, bool fallbac
     std::lock_guard<std::mutex> lock(reload_mutex);
 
     FilesChangesTracker new_files = getNewFileList();
-    if (force || new_files.isDifferOrNewerThan(files))
+    if (force || need_reload_from_zk || new_files.isDifferOrNewerThan(files))
     {
         ConfigProcessor config_processor(path);
         ConfigProcessor::LoadedConfig loaded_config;
@@ -94,6 +94,17 @@ void ConfigReloader::reloadIfNewer(bool force, bool throw_on_error, bool fallbac
                 loaded_config = config_processor.loadConfigWithZooKeeperIncludes(
                     zk_node_cache, zk_changed_event, fallback_to_preprocessed);
         }
+        catch (const Coordination::Exception & e)
+        {
+            if (Coordination::isHardwareError(e.code))
+                need_reload_from_zk = true;
+
+            if (throw_on_error)
+                throw;
+
+            tryLogCurrentException(log, "ZooKeeper error when loading config from `" + path + "'");
+            return;
+        }
         catch (...)
         {
             if (throw_on_error)
@@ -110,7 +121,10 @@ void ConfigReloader::reloadIfNewer(bool force, bool throw_on_error, bool fallbac
          *  When file has been written (and contain valid data), we don't load new data since modification time remains the same.
          */
         if (!loaded_config.loaded_from_preprocessed)
+        {
             files = std::move(new_files);
+            need_reload_from_zk = false;
+        }
 
         try
         {

diff --git a/dbms/src/Common/Config/ConfigReloader.h b/dbms/src/Common/Config/ConfigReloader.h
@@ -75,6 +75,7 @@ class ConfigReloader
     std::string preprocessed_dir;
     FilesChangesTracker files;
     zkutil::ZooKeeperNodeCache zk_node_cache;
+    bool need_reload_from_zk = false;
     zkutil::EventPtr zk_changed_event = std::make_shared<Poco::Event>();
 
     Updater updater;