From 40c69662238b3d3b5e3f7734113e639e31704737 Mon Sep 17 00:00:00 2001
From: alesapin <alesapin@gmail.com>
Date: Wed, 4 Dec 2019 18:23:05 +0300
Subject: [PATCH] Don't check dictionary modification if it's already have an
 exception.

---
 dbms/src/Interpreters/ExternalLoader.cpp | 31 ++++++++++++------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/dbms/src/Interpreters/ExternalLoader.cpp b/dbms/src/Interpreters/ExternalLoader.cpp
index acf7632737c..2e7785700e2 100644
--- a/dbms/src/Interpreters/ExternalLoader.cpp
+++ b/dbms/src/Interpreters/ExternalLoader.cpp
@@ -560,8 +560,8 @@ public:
     /// The function doesn't touch the objects which were never tried to load.
     void reloadOutdated()
     {
-        /// Iterate through all the objects and find loaded ones which should be checked if they were modified.
-        std::unordered_map<LoadablePtr, bool> is_modified_map;
+        /// Iterate through all the objects and find loaded ones which should be checked if they need update.
+        std::unordered_map<LoadablePtr, bool> should_update_map;
         {
             std::lock_guard lock{mutex};
             TimePoint now = std::chrono::system_clock::now();
@@ -569,22 +569,26 @@ public:
             {
                 const auto & info = name_and_info.second;
                 if ((now >= info.next_update_time) && !info.loading() && info.loaded())
-                    is_modified_map.emplace(info.object, true);
+                    should_update_map.emplace(info.object, info.hasException());
             }
         }
 
         /// Find out which of the loaded objects were modified.
-        /// We couldn't perform these checks while we were building `is_modified_map` because
+        /// We couldn't perform these checks while we were building `should_update_map` because
         /// the `mutex` should be unlocked while we're calling the function object->isModified()
-        for (auto & [object, is_modified_flag] : is_modified_map)
+        for (auto & [object, should_update_flag] : should_update_map)
         {
             try
             {
-                is_modified_flag = object->isModified();
+                /// Maybe alredy true, if we have an exception
+                if (!should_update_flag)
+                    should_update_flag = object->isModified();
             }
             catch (...)
             {
                 tryLogCurrentException(log, "Could not check if " + type_name + " '" + object->getName() + "' was modified");
+                /// Cannot check isModified, so update
+                should_update_flag = true;
             }
         }
 
@@ -598,16 +602,13 @@ public:
                 {
                     if (info.loaded())
                     {
-                        auto it = is_modified_map.find(info.object);
-                        if (it == is_modified_map.end())
-                            continue; /// Object has been just loaded (it wasn't loaded while we were building the map `is_modified_map`), so we don't have to reload it right now.
+                        auto it = should_update_map.find(info.object);
+                        if (it == should_update_map.end())
+                            continue; /// Object has been just loaded (it wasn't loaded while we were building the map `should_update_map`), so we don't have to reload it right now.
 
-                        bool is_modified_flag = it->second;
-                        /// Object maybe successfully loaded in some old state, but have an exception from new loads.
-                        /// so even if it's not modified better to reload it.
-                        if (!is_modified_flag && !info.hasException())
+                        bool should_update_flag = it->second;
+                        if (!should_update_flag)
                         {
-                            /// Object wasn't modified so we only have to set `next_update_time`.
                             info.next_update_time = calculateNextUpdateTime(info.object, info.error_count);
                             continue;
                         }
@@ -635,7 +636,7 @@ private:
         bool loading() const { return loading_id != 0; }
         bool wasLoading() const { return loaded() || failed() || loading(); }
         bool ready() const { return (loaded() || failed()) && !forced_to_reload; }
-        bool hasException() { return exception != nullptr; }
+        bool hasException() const { return exception != nullptr; }
 
         Status status() const
         {