Skip to content

Commit

Permalink
Remove unnecessary reloading while renaming a dictionary in Atomic da…
Browse files Browse the repository at this point in the history
…tabase.
  • Loading branch information
vitlibar committed Mar 15, 2024
1 parent 902367a commit 347ea28
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 35 deletions.
90 changes: 56 additions & 34 deletions src/Common/Config/AbstractConfigurationComparison.cpp
Expand Up @@ -19,6 +19,55 @@ namespace
result += subkey;
return result;
}

bool isSameConfigurationImpl(const Poco::Util::AbstractConfiguration & left, const String & left_key,
const Poco::Util::AbstractConfiguration & right, const String & right_key,
const std::unordered_set<String> * ignore_keys)
{
if (&left == &right && left_key == right_key)
return true;

bool has_property = left.hasProperty(left_key);
if (has_property != right.hasProperty(right_key))
return false;
if (has_property)
{
/// The left and right configurations contains values so we can compare them.
if (left.getRawString(left_key) == right.getRawString(right_key))
return true;
}

/// Get the subkeys of the left and right configurations.
Poco::Util::AbstractConfiguration::Keys subkeys;
left.keys(left_key, subkeys);

if (ignore_keys)
std::erase_if(subkeys, [&](const String & key) { return ignore_keys->contains(key); });

{
/// Check that the right configuration has the same set of subkeys as the left configuration.
Poco::Util::AbstractConfiguration::Keys right_subkeys;
right.keys(right_key, right_subkeys);

if (ignore_keys)
std::erase_if(right_subkeys, [&](const String & key) { return ignore_keys->contains(key); });

std::unordered_set<std::string_view> left_subkeys{subkeys.begin(), subkeys.end()};

if ((left_subkeys.size() != right_subkeys.size()) || (left_subkeys.size() != subkeys.size()))
return false;
for (const auto & right_subkey : right_subkeys)
if (!left_subkeys.contains(right_subkey))
return false;
}

/// Go through all the subkeys and compare corresponding parts of the configurations.
for (const auto & subkey : subkeys)
if (!isSameConfiguration(left, concatKeyAndSubKey(left_key, subkey), right, concatKeyAndSubKey(right_key, subkey)))
return false;

return true;
}
}


Expand Down Expand Up @@ -52,41 +101,14 @@ bool isSameConfigurationWithMultipleKeys(const Poco::Util::AbstractConfiguration
bool isSameConfiguration(const Poco::Util::AbstractConfiguration & left, const String & left_key,
const Poco::Util::AbstractConfiguration & right, const String & right_key)
{
if (&left == &right && left_key == right_key)
return true;

bool has_property = left.hasProperty(left_key);
if (has_property != right.hasProperty(right_key))
return false;
if (has_property)
{
/// The left and right configurations contains values so we can compare them.
if (left.getRawString(left_key) != right.getRawString(right_key))
return false;
}

/// Get the subkeys of the left and right configurations.
Poco::Util::AbstractConfiguration::Keys subkeys;
left.keys(left_key, subkeys);

{
/// Check that the right configuration has the same set of subkeys as the left configuration.
Poco::Util::AbstractConfiguration::Keys right_subkeys;
right.keys(right_key, right_subkeys);
std::unordered_set<std::string_view> left_subkeys{subkeys.begin(), subkeys.end()};
if ((left_subkeys.size() != right_subkeys.size()) || (left_subkeys.size() != subkeys.size()))
return false;
for (const auto & right_subkey : right_subkeys)
if (!left_subkeys.contains(right_subkey))
return false;
}

/// Go through all the subkeys and compare corresponding parts of the configurations.
for (const auto & subkey : subkeys)
if (!isSameConfiguration(left, concatKeyAndSubKey(left_key, subkey), right, concatKeyAndSubKey(right_key, subkey)))
return false;
return isSameConfigurationImpl(left, left_key, right, right_key, /* ignore_keys= */ nullptr);
}

return true;
bool isSameConfigurationIgnoringKeys(const Poco::Util::AbstractConfiguration & left, const String & left_key,
const Poco::Util::AbstractConfiguration & right, const String & right_key,
const std::unordered_set<String> & ignore_keys)
{
return isSameConfigurationImpl(left, left_key, right, right_key, !ignore_keys.empty() ? &ignore_keys : nullptr);
}

}
6 changes: 6 additions & 0 deletions src/Common/Config/AbstractConfigurationComparison.h
@@ -1,6 +1,7 @@
#pragma once

#include <base/types.h>
#include <unordered_set>

namespace Poco::Util
{
Expand Down Expand Up @@ -33,6 +34,11 @@ namespace DB
bool isSameConfiguration(const Poco::Util::AbstractConfiguration & left, const String & left_key,
const Poco::Util::AbstractConfiguration & right, const String & right_key);

/// Returns true if specified subviews of the two configurations contains the same keys and values, but without checking specified keys.
bool isSameConfigurationIgnoringKeys(const Poco::Util::AbstractConfiguration & left, const String & left_key,
const Poco::Util::AbstractConfiguration & right, const String & right_key,
const std::unordered_set<String> & ignore_keys);

inline bool operator==(const Poco::Util::AbstractConfiguration & left, const Poco::Util::AbstractConfiguration & right)
{
return isSameConfiguration(left, right);
Expand Down
16 changes: 16 additions & 0 deletions src/Interpreters/ExternalDictionariesLoader.cpp
Expand Up @@ -5,6 +5,7 @@
#include <Dictionaries/DictionaryStructure.h>
#include <Databases/IDatabase.h>
#include <Storages/IStorage.h>
#include <Common/Config/AbstractConfigurationComparison.h>

#include "config.h"

Expand Down Expand Up @@ -46,6 +47,21 @@ StorageID ExternalDictionariesLoader::getStorageIDFromObjectConfig(const String
return StorageID::fromDictionaryConfig(config, key_in_config);
}

bool ExternalDictionariesLoader::isObjectConfigSame(const String & name,
const Poco::Util::AbstractConfiguration & old_config, const String & old_key_in_config,
const Poco::Util::AbstractConfiguration & new_config, const String & new_key_in_config) const
{
/// If the database is atomic then a dictionary can be renamed without reloading.
if (!old_config.getString(old_key_in_config + ".uuid", "").empty() && !new_config.getString(new_key_in_config + ".uuid", "").empty())
{
static const std::unordered_set<String> ignore_keys{"name", "database"};
return isSameConfigurationIgnoringKeys(old_config, old_key_in_config, new_config, new_key_in_config, ignore_keys);
}

return ExternalLoader::isObjectConfigSame(name, old_config, old_key_in_config, new_config, new_key_in_config);
}


ExternalDictionariesLoader::DictPtr ExternalDictionariesLoader::getDictionary(const std::string & dictionary_name, ContextPtr local_context) const
{
std::string resolved_dictionary_name = resolveDictionaryName(dictionary_name, local_context->getCurrentDatabase());
Expand Down
4 changes: 4 additions & 0 deletions src/Interpreters/ExternalDictionariesLoader.h
Expand Up @@ -45,6 +45,10 @@ class ExternalDictionariesLoader : public ExternalLoader, WithContext

StorageID getStorageIDFromObjectConfig(const String & name, const Poco::Util::AbstractConfiguration & config, const String & key_in_config) const override;

bool isObjectConfigSame(const String & name,
const Poco::Util::AbstractConfiguration & old_config, const String & old_key_in_config,
const Poco::Util::AbstractConfiguration & new_config, const String & new_key_in_config) const override;

std::string resolveDictionaryName(const std::string & dictionary_name, const std::string & current_database_name) const;

/// Try convert qualified dictionary name to persistent UUID
Expand Down
16 changes: 15 additions & 1 deletion src/Interpreters/ExternalLoader.cpp
Expand Up @@ -89,6 +89,7 @@ namespace
};

using GetStorageIDFromObjectConfigFunction = std::function<StorageID(const String & object_name, const Poco::Util::AbstractConfiguration & config, const String & key_in_config)>;
using IsObjectConfigSameFunction = std::function<bool(const String & object_name, const Poco::Util::AbstractConfiguration & config_1, const String & key_in_config_1, const Poco::Util::AbstractConfiguration & config_2, const String & key_in_config_2)>;
}


Expand Down Expand Up @@ -407,9 +408,11 @@ class ExternalLoader::LoadingDispatcher : private boost::noncopyable

LoadingDispatcher(
const CreateObjectFunction & create_object_function_,
const IsObjectConfigSameFunction & is_object_config_same_,
const String & type_name_,
LoggerPtr log_)
: create_object(create_object_function_)
, is_object_config_same(is_object_config_same_)
, type_name(type_name_)
, log(log_)
{
Expand Down Expand Up @@ -464,7 +467,7 @@ class ExternalLoader::LoadingDispatcher : private boost::noncopyable
else
{
const auto & new_config = new_config_it->second;
bool is_same_config = isSameConfiguration(*info.config->config, info.config->key_in_config, *new_config->config, new_config->key_in_config);
bool is_same_config = is_object_config_same(name, *info.config->config, info.config->key_in_config, *new_config->config, new_config->key_in_config);
bool is_same_storage_id = !new_config->storage_id.empty() && new_config->storage_id != info.config->storage_id;

info.config = new_config;
Expand Down Expand Up @@ -1209,6 +1212,7 @@ class ExternalLoader::LoadingDispatcher : private boost::noncopyable
}

const CreateObjectFunction create_object;
const IsObjectConfigSameFunction is_object_config_same;
const String type_name;
LoggerPtr log;

Expand Down Expand Up @@ -1298,6 +1302,8 @@ ExternalLoader::ExternalLoader(const String & type_name_, LoggerPtr log_)
log_))
, loading_dispatcher(std::make_unique<LoadingDispatcher>(
[this](auto && a, auto && b, auto && c) { return createObject(a, b, c); },
[this](const String & name, const Poco::Util::AbstractConfiguration & config_1, const String & key_in_config_1, const Poco::Util::AbstractConfiguration & config_2, const String & key_in_config_2)
{ return isObjectConfigSame(name, config_1, key_in_config_1, config_2, key_in_config_2); },
type_name_,
log_))
, periodic_updater(std::make_unique<PeriodicUpdater>(*config_files_reader, *loading_dispatcher))
Expand Down Expand Up @@ -1328,6 +1334,14 @@ void ExternalLoader::setConfigSettings(const ExternalLoaderConfigSettings & sett
config_files_reader->setConfigSettings(settings);
}

bool ExternalLoader::isObjectConfigSame(const String & /* name */,
const Poco::Util::AbstractConfiguration & old_config, const String & old_key_in_config,
const Poco::Util::AbstractConfiguration & new_config, const String & new_key_in_config) const
{
return isSameConfiguration(old_config, old_key_in_config, new_config, new_key_in_config);
}


void ExternalLoader::enableAlwaysLoadEverything(bool enable)
{
loading_dispatcher->enableAlwaysLoadEverything(enable);
Expand Down
6 changes: 6 additions & 0 deletions src/Interpreters/ExternalLoader.h
Expand Up @@ -218,6 +218,12 @@ class ExternalLoader
/// Extracts a StorageID associated with the object's configuration.
virtual StorageID getStorageIDFromObjectConfig(const String & /* name */, const Poco::Util::AbstractConfiguration & /* config */, const String & /* key_in_config */) const { return StorageID::createEmpty(); }

/// Checks if the configuration is the same after reloading config.
/// (If the configuration of an object is the same after reloading of the configuration then the object itself won't be reloaded.)
virtual bool isObjectConfigSame(const String & name,
const Poco::Util::AbstractConfiguration & old_config, const String & old_key_in_config,
const Poco::Util::AbstractConfiguration & new_config, const String & new_key_in_config) const;

private:
void checkLoaded(const LoadResult & result, bool check_no_errors) const;
void checkLoaded(const LoadResults & results, bool check_no_errors) const;
Expand Down

0 comments on commit 347ea28

Please sign in to comment.