style + flaky test fix

src/Dictionaries/CacheDictionary.cpp

@@ -10,6 +10,7 @@
 #include <Common/randomSeed.h>
 #include <Common/typeid_cast.h>
 #include <Core/Defines.h>
+#include <IO/WriteBufferFromOStream.h>
 #include <ext/range.h>
 #include <ext/size.h>
 #include <Common/setThreadName.h>
@@ -284,7 +285,7 @@ Overloaded(Ts...) -> Overloaded<Ts...>;
 
 std::string CacheDictionary::AttributeValuesForKey::dump()
 {
-    std::ostringstream os;
+    WriteBufferFromOwnString os;
     for (auto & attr : values)
         std::visit(Overloaded {
             [&os](UInt8 arg)   { os << "type: UInt8, value: "   <<  std::to_string(arg) << "\n"; },
@@ -309,7 +310,7 @@ std::string CacheDictionary::AttributeValuesForKey::dump()
 
 std::string CacheDictionary::UpdateUnit::dumpFoundIds()
 {
-    std::ostringstream os;
+    WriteBufferFromOwnString os;
     for (auto it : found_ids)
     {
         os << "Key: " << std::to_string(it.first) << "\n";
@@ -344,7 +345,7 @@ CacheDictionary::FindResult CacheDictionary::findCellIdxForGet(const Key & id, c
     return {pos & size_overlap_mask, ResultState::NotFound};
 }
 
-/// Returns cell_idx such that cells[cell_idx].id = id or the oldest cell in bounds of max_coolision_length. 
+/// Returns cell_idx such that cells[cell_idx].id = id or the oldest cell in bounds of max_coolision_length.
 size_t CacheDictionary::findCellIdxForSet(const Key & id) const
 {
     auto pos = getCellIdx(id);
@@ -514,15 +515,16 @@ CacheDictionary::Attribute CacheDictionary::createAttributeWithTypeAndName(const
 
     switch (type)
     {
-
         /* Macro argument should be enclosed in parentheses, but if do so we cannot initialize \
          * NearestFieldType which takes TYPE as a template parameter. */
 #define DISPATCH(TYPE)\
-    case AttributeUnderlyingType::ut##TYPE: {\
+        case AttributeUnderlyingType::ut##TYPE:\
-        attr.null_value = TYPE(null_value.get<NearestFieldType<TYPE>>()); /* NOLINT(bugprone-macro-parentheses) */ \
+        {\
-        attr.arrays = std::make_unique<ContainerType<TYPE>>(size); /* NOLINT(bugprone-macro-parentheses) */ \
+            attr.null_value = TYPE(null_value.get<NearestFieldType<TYPE>>()); /* NOLINT(bugprone-macro-parentheses) */ \
-        bytes_allocated += size * sizeof(TYPE);\
+            attr.arrays = std::make_unique<ContainerType<TYPE>>(size); /* NOLINT(bugprone-macro-parentheses) */ \
-        break;} 
+            bytes_allocated += size * sizeof(TYPE);\
+            break;\
+        }
         DISPATCH(UInt8)
         DISPATCH(UInt16)
         DISPATCH(UInt32)

src/Dictionaries/CacheDictionary.h

@@ -257,7 +257,8 @@ private:
 
     void createAttributes();
 
-    Attribute createAttributeWithTypeAndName(const AttributeUnderlyingType type, const String & name, const Field & null_value); /* NOLINT(readability-convert-member-functions-to-static) */
+    /* NOLINTNEXTLINE(readability-convert-member-functions-to-static) */
+    Attribute createAttributeWithTypeAndName(const AttributeUnderlyingType type, const String & name, const Field & null_value);
 
     template <typename AttributeType, typename OutputType, typename DefaultGetter>
     void getItemsNumberImpl(
@@ -310,11 +311,10 @@ private:
         }
         else
         {
-            /// This maybe not obvious, but when we define is this cell is expired or expired permanently, we add extra_lifetime_seconds 
+            /// This maybe not obvious, but when we define is this cell is expired or expired permanently, we add extra_lifetime_seconds
-            /// to the expiration time. And it overflows pretty well. 
+            /// to the expiration time. And it overflows pretty well.
             cell.setExpiresAt(std::chrono::time_point<std::chrono::system_clock>::max() - 2 * std::chrono::seconds(extra_lifetime_seconds));
         }
-            
     }
 
     inline bool isExpired(time_point_t now, time_point_t deadline) const
@@ -332,12 +332,12 @@ private:
         NotFound,
         FoundAndValid,
         FoundButExpired,
-        /// Here is a gap between there two states in which a key could be read 
+        /// Here is a gap between there two states in which a key could be read
         /// with an enabled setting in config enable_read_expired_keys.
         FoundButExpiredPermanently
     };
 
-    using FindResult = std::pair<size_t, ResultState>; 
+    using FindResult = std::pair<size_t, ResultState>;
 
     FindResult findCellIdxForGet(const Key & id, const time_point_t now) const;
 
@@ -400,7 +400,7 @@ private:
     /*
      * How the update goes: we basically have a method like get(keys)->values. Values are cached, so sometimes we
      * can return them from the cache. For values not in cache, we query them from the source, and add to the
-     * cache. The cache is lossy, so we can't expect it to store all the keys, and we store them separately. 
+     * cache. The cache is lossy, so we can't expect it to store all the keys, and we store them separately.
      * So, there is a map of found keys to all its attributes.
      */
     struct UpdateUnit

src/Dictionaries/CacheDictionary.inc.h

@@ -80,7 +80,7 @@ void CacheDictionary::getItemsNumberImpl(
 
             const auto [cell_idx, state] = findCellIdxForGet(id, now);
 
-            if (state == ResultState::FoundAndValid) 
+            if (state == ResultState::FoundAndValid)
             {
                 ++cache_hit;
                 insert_to_answer_routine(row, cell_idx);
@@ -299,7 +299,7 @@ void CacheDictionary::getItemsString(
                 const auto it = local_cache.find(id);
                 if (it != local_cache.end())
                     value = StringRef(it->second);
-                else 
+                else
                     value = get_default(row);
 
                 out->insertData(value.data, value.size);
@@ -354,7 +354,7 @@ void CacheDictionary::getItemsString(
         {
             const auto found_it = update_unit_ptr->found_ids.find(id);
 
-            /// Previously we didn't store defaults in local cache. 
+            /// Previously we didn't store defaults in local cache.
             if (found_it != update_unit_ptr->found_ids.end() && found_it->second.found)
                 value = std::get<String>(found_it->second.values[attribute_index]);
             else

tests/integration/helpers/cluster.py

@@ -1028,6 +1028,10 @@ class ClickHouseInstance:
             ["bash", "-c", 'grep "{}" /var/log/clickhouse-server/clickhouse-server.log || true'.format(substring)])
         return len(result) > 0
 
+    def file_exists(self, path):
+        return self.exec_in_container(
+            ["bash", "-c", "echo $(if [ -e '{}' ]; then echo 'yes'; else echo 'no'; fi)".format(path)]) == 'yes\n'
+
     def copy_file_to_container(self, local_path, dest_path):
         container_id = self.get_docker_handle().id
         return self.cluster.copy_file_to_container(container_id, local_path, dest_path)

tests/integration/test_dictionaries_update_and_reload/test.py

@@ -204,7 +204,10 @@ def test_reload_after_fail_by_timer(started_cluster):
     # Creating the file source makes the dictionary able to load.
     instance.copy_file_to_container(os.path.join(SCRIPT_DIR, "configs/dictionaries/file.txt"),
                                     "/etc/clickhouse-server/config.d/no_file_2.txt")
-    time.sleep(6);
+    # Check that file appears in container and wait if needed.
+    while not instance.file_exists("/etc/clickhouse-server/config.d/no_file_2.txt"):
+        time.sleep(1)
+    assert("9\t10\n" == instance.exec_in_container("cat /etc/clickhouse-server/config.d/no_file_2.txt"))
     query("SELECT dictGetInt32('no_file_2', 'a', toUInt64(9))") == "10\n"
     assert get_status("no_file_2") == "LOADED"