From bc6caba4db9de6cf723a7707a4e486718bff40df Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 30 Sep 2019 14:18:01 +0300 Subject: [PATCH 1/5] Remove redundant argument --- dbms/src/Interpreters/ExternalLoader.cpp | 25 ++++++++++++------------ 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/dbms/src/Interpreters/ExternalLoader.cpp b/dbms/src/Interpreters/ExternalLoader.cpp index 0709abc337e..659f3471247 100644 --- a/dbms/src/Interpreters/ExternalLoader.cpp +++ b/dbms/src/Interpreters/ExternalLoader.cpp @@ -48,15 +48,15 @@ public: repositories.emplace_back(std::move(repository), std::move(settings)); } - using ObjectConfigs = std::shared_ptr>; + using ObjectConfigsPtr = std::shared_ptr>; /// Reads configuration files. - ObjectConfigs read(bool ignore_last_modification_time = false) + ObjectConfigsPtr read() { std::lock_guard lock{mutex}; // Check last modification times of files and read those files which are new or changed. - if (!readFileInfos(ignore_last_modification_time)) + if (!readFileInfos()) return configs; // Nothing changed, so we can return the previous result. // Generate new result. @@ -86,13 +86,13 @@ public: private: struct FileInfo { - Poco::Timestamp last_modification_time; + Poco::Timestamp last_modification_time = 0; std::vector> configs; // Parsed file's contents. bool in_use = true; // Whether the `FileInfo` should be destroyed because the correspondent file is deleted. }; /// Read files and store them to the map `file_infos`. - bool readFileInfos(bool ignore_last_modification_time) + bool readFileInfos() { bool changed = false; @@ -111,13 +111,13 @@ private: if (it != file_infos.end()) { FileInfo & file_info = it->second; - if (readFileInfo(*repository, path, settings, ignore_last_modification_time, file_info)) + if (readFileInfo(*repository, path, settings, file_info)) changed = true; } else { FileInfo file_info; - if (readFileInfo(*repository, path, settings, true, file_info)) + if (readFileInfo(*repository, path, settings, file_info)) { file_infos.emplace(path, std::move(file_info)); changed = true; @@ -143,7 +143,6 @@ private: ExternalLoaderConfigRepository & repository, const String & path, const ExternalLoaderConfigSettings & settings, - bool ignore_last_modification_time, FileInfo & file_info) const { try @@ -155,7 +154,7 @@ private: } Poco::Timestamp last_modification_time = repository.getLastModificationTime(path); - if (!ignore_last_modification_time && (last_modification_time <= file_info.last_modification_time)) + if (last_modification_time <= file_info.last_modification_time) { file_info.in_use = true; return false; @@ -206,7 +205,7 @@ private: std::mutex mutex; std::vector, ExternalLoaderConfigSettings>> repositories; - ObjectConfigs configs; + ObjectConfigsPtr configs; std::unordered_map file_infos; }; @@ -249,10 +248,10 @@ public: } } - using ObjectConfigs = ConfigFilesReader::ObjectConfigs; + using ObjectConfigsPtr = ConfigFilesReader::ObjectConfigsPtr; /// Sets new configurations for all the objects. - void setConfiguration(const ObjectConfigs & new_configs) + void setConfiguration(const ObjectConfigsPtr & new_configs) { std::lock_guard lock{mutex}; if (configs == new_configs) @@ -869,7 +868,7 @@ private: mutable std::mutex mutex; std::condition_variable event; - ObjectConfigs configs; + ObjectConfigsPtr configs; std::unordered_map infos; bool always_load_everything = false; bool enable_async_loading = false; From 08681ac210dfdd7703d7e9629b830ba99692e8a5 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 30 Sep 2019 19:12:08 +0300 Subject: [PATCH 2/5] Preparation for AST configurations for external dictionaries --- dbms/src/Dictionaries/HashedDictionary.cpp | 4 +- dbms/src/Interpreters/Context.cpp | 7 +- .../ExternalDictionariesLoader.cpp | 12 +-- .../Interpreters/ExternalDictionariesLoader.h | 4 +- dbms/src/Interpreters/ExternalLoader.cpp | 96 +++++++++---------- dbms/src/Interpreters/ExternalLoader.h | 12 +-- .../ExternalLoaderConfigRepository.cpp | 72 -------------- .../ExternalLoaderConfigRepository.h | 31 ------ .../ExternalLoaderXMLConfigRepository.cpp | 78 +++++++++++++++ .../ExternalLoaderXMLConfigRepository.h | 45 +++++++++ .../src/Interpreters/ExternalModelsLoader.cpp | 11 +-- dbms/src/Interpreters/ExternalModelsLoader.h | 2 +- .../IExternalLoaderConfigRepository.h | 42 ++++++++ 13 files changed, 232 insertions(+), 184 deletions(-) delete mode 100644 dbms/src/Interpreters/ExternalLoaderConfigRepository.cpp delete mode 100644 dbms/src/Interpreters/ExternalLoaderConfigRepository.h create mode 100644 dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.cpp create mode 100644 dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h create mode 100644 dbms/src/Interpreters/IExternalLoaderConfigRepository.h diff --git a/dbms/src/Dictionaries/HashedDictionary.cpp b/dbms/src/Dictionaries/HashedDictionary.cpp index 7fe5dd7abf2..d81b259b184 100644 --- a/dbms/src/Dictionaries/HashedDictionary.cpp +++ b/dbms/src/Dictionaries/HashedDictionary.cpp @@ -715,8 +715,8 @@ template PaddedPODArray HashedDictionary::getIds(const Attribute & attribute) const { if (!sparse) - return getIdsAttrImpl(*std::get>(attribute.maps)); - return getIdsAttrImpl(*std::get>(attribute.sparse_maps)); + return getIdsAttrImpl(*std::get>(attribute.maps)); + return getIdsAttrImpl(*std::get>(attribute.sparse_maps)); } PaddedPODArray HashedDictionary::getIds() const diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index 6e0fe3ba3ec..fef48898828 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -1320,8 +1321,8 @@ const ExternalDictionariesLoader & Context::getExternalDictionariesLoader() cons if (!this->global_context) throw Exception("Logical error: there is no global context", ErrorCodes::LOGICAL_ERROR); - auto config_repository = std::make_unique(); - shared->external_dictionaries_loader.emplace(std::move(config_repository), config, *this->global_context); + auto config_repository = std::make_unique(config, "dictionaries_config"); + shared->external_dictionaries_loader.emplace(std::move(config_repository), *this->global_context); } return *shared->external_dictionaries_loader; } @@ -1340,7 +1341,7 @@ const ExternalModelsLoader & Context::getExternalModelsLoader() const if (!this->global_context) throw Exception("Logical error: there is no global context", ErrorCodes::LOGICAL_ERROR); - auto config_repository = std::make_unique(); + auto config_repository = std::make_unique(getConfigRef(), "models_config"); shared->external_models_loader.emplace(std::move(config_repository), *this->global_context); } return *shared->external_models_loader; diff --git a/dbms/src/Interpreters/ExternalDictionariesLoader.cpp b/dbms/src/Interpreters/ExternalDictionariesLoader.cpp index a9f4cd3bd81..5e84da0a0cd 100644 --- a/dbms/src/Interpreters/ExternalDictionariesLoader.cpp +++ b/dbms/src/Interpreters/ExternalDictionariesLoader.cpp @@ -7,15 +7,11 @@ namespace DB /// Must not acquire Context lock in constructor to avoid possibility of deadlocks. ExternalDictionariesLoader::ExternalDictionariesLoader( - std::unique_ptr config_repository, - const Poco::Util::AbstractConfiguration & config, - Context & context_) - : ExternalLoader(config, - "external dictionary", - &Logger::get("ExternalDictionariesLoader")), - context(context_) + ExternalLoaderConfigRepositoryPtr config_repository, Context & context_) + : ExternalLoader("external dictionary", &Logger::get("ExternalDictionariesLoader")) + , context(context_) { - addConfigRepository(std::move(config_repository), {"dictionary", "name", "dictionaries_config"}); + addConfigRepository(std::move(config_repository), {"dictionary", "name"}); enableAsyncLoading(true); enablePeriodicUpdates(true); } diff --git a/dbms/src/Interpreters/ExternalDictionariesLoader.h b/dbms/src/Interpreters/ExternalDictionariesLoader.h index e2f53d1cb53..04916f90d9d 100644 --- a/dbms/src/Interpreters/ExternalDictionariesLoader.h +++ b/dbms/src/Interpreters/ExternalDictionariesLoader.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -19,8 +20,7 @@ public: /// Dictionaries will be loaded immediately and then will be updated in separate thread, each 'reload_period' seconds. ExternalDictionariesLoader( - std::unique_ptr config_repository, - const Poco::Util::AbstractConfiguration & config, + ExternalLoaderConfigRepositoryPtr config_repository, Context & context_); DictPtr getDictionary(const std::string & name) const diff --git a/dbms/src/Interpreters/ExternalLoader.cpp b/dbms/src/Interpreters/ExternalLoader.cpp index 659f3471247..1b0af14a086 100644 --- a/dbms/src/Interpreters/ExternalLoader.cpp +++ b/dbms/src/Interpreters/ExternalLoader.cpp @@ -29,20 +29,18 @@ struct ExternalLoader::ObjectConfig }; -/** Reads configuration files and parses them as XML. - * Stores parsed contents of the files along with their last modification time to - * avoid unnecessary parsing on repetetive reading. +/** Reads configurations from configuration repository and parses it. */ -class ExternalLoader::ConfigFilesReader : private boost::noncopyable +class ExternalLoader::LoadablesConfigReader : private boost::noncopyable { public: - ConfigFilesReader(const Poco::Util::AbstractConfiguration & main_config_, const String & type_name_, Logger * log_) - : main_config(main_config_), type_name(type_name_), log(log_) + LoadablesConfigReader(const String & type_name_, Logger * log_) + : type_name(type_name_), log(log_) { } - ~ConfigFilesReader() = default; + ~LoadablesConfigReader() = default; - void addConfigRepository(std::unique_ptr repository, const ExternalLoaderConfigSettings & settings) + void addConfigRepository(std::unique_ptr repository, const ExternalLoaderConfigSettings & settings) { std::lock_guard lock{mutex}; repositories.emplace_back(std::move(repository), std::move(settings)); @@ -56,14 +54,14 @@ public: std::lock_guard lock{mutex}; // Check last modification times of files and read those files which are new or changed. - if (!readFileInfos()) + if (!readLoadablesInfos()) return configs; // Nothing changed, so we can return the previous result. // Generate new result. auto new_configs = std::make_shared>(); - for (const auto & [path, file_info] : file_infos) + for (const auto & [path, loadable_info] : loadables_infos) { - for (const auto & [name, config] : file_info.configs) + for (const auto & [name, config] : loadable_info.configs) { auto already_added_it = new_configs->find(name); if (already_added_it != new_configs->end()) @@ -84,42 +82,41 @@ public: } private: - struct FileInfo + struct LoadablesInfos { - Poco::Timestamp last_modification_time = 0; std::vector> configs; // Parsed file's contents. - bool in_use = true; // Whether the `FileInfo` should be destroyed because the correspondent file is deleted. + bool in_use = true; // Whether the ` LoadablesInfos` should be destroyed because the correspondent file is deleted. }; - /// Read files and store them to the map `file_infos`. - bool readFileInfos() + /// Read files and store them to the map ` loadables_infos`. + bool readLoadablesInfos() { bool changed = false; - for (auto & path_and_file_info : file_infos) + for (auto & name_and_loadable_info : loadables_infos) { - FileInfo & file_info = path_and_file_info.second; - file_info.in_use = false; + LoadablesInfos & loadable_info = name_and_loadable_info.second; + loadable_info.in_use = false; } for (const auto & [repository, settings] : repositories) { - const auto paths = repository->list(main_config, settings.path_setting_name); - for (const auto & path : paths) + const auto names = repository->getAllLoadablesDefinitionNames(); + for (const auto & name : names) { - auto it = file_infos.find(path); - if (it != file_infos.end()) + auto it = loadables_infos.find(name); + if (it != loadables_infos.end()) { - FileInfo & file_info = it->second; - if (readFileInfo(*repository, path, settings, file_info)) + LoadablesInfos & loadable_info = it->second; + if (readLoadablesInfo(*repository, name, settings, loadable_info)) changed = true; } else { - FileInfo file_info; - if (readFileInfo(*repository, path, settings, file_info)) + LoadablesInfos loadable_info; + if (readLoadablesInfo(*repository, name, settings, loadable_info)) { - file_infos.emplace(path, std::move(file_info)); + loadables_infos.emplace(name, std::move(loadable_info)); changed = true; } } @@ -127,23 +124,23 @@ private: } std::vector deleted_files; - for (auto & [path, file_info] : file_infos) - if (!file_info.in_use) + for (auto & [path, loadable_info] : loadables_infos) + if (!loadable_info.in_use) deleted_files.emplace_back(path); if (!deleted_files.empty()) { for (const String & deleted_file : deleted_files) - file_infos.erase(deleted_file); + loadables_infos.erase(deleted_file); changed = true; } return changed; } - bool readFileInfo( - ExternalLoaderConfigRepository & repository, + bool readLoadablesInfo( + IExternalLoaderConfigRepository & repository, const String & path, const ExternalLoaderConfigSettings & settings, - FileInfo & file_info) const + LoadablesInfos & loadable_info) const { try { @@ -153,14 +150,13 @@ private: return false; } - Poco::Timestamp last_modification_time = repository.getLastModificationTime(path); - if (last_modification_time <= file_info.last_modification_time) + if (!repository.isUpdated(path)) { - file_info.in_use = true; + loadable_info.in_use = true; return false; } - auto file_contents = repository.load(path, main_config.getString("path", DBMS_DEFAULT_PATH)); + auto file_contents = repository.load(path); /// get all objects' definitions Poco::Util::AbstractConfiguration::Keys keys; @@ -187,9 +183,8 @@ private: configs_from_file.emplace_back(name, ObjectConfig{path, file_contents, key}); } - file_info.configs = std::move(configs_from_file); - file_info.last_modification_time = last_modification_time; - file_info.in_use = true; + loadable_info.configs = std::move(configs_from_file); + loadable_info.in_use = true; return true; } catch (...) @@ -199,18 +194,17 @@ private: } } - const Poco::Util::AbstractConfiguration & main_config; const String type_name; Logger * log; std::mutex mutex; - std::vector, ExternalLoaderConfigSettings>> repositories; + std::vector, ExternalLoaderConfigSettings>> repositories; ObjectConfigsPtr configs; - std::unordered_map file_infos; + std::unordered_map loadables_infos; }; -/** Manages loading and reloading objects. Uses configurations from the class ConfigFilesReader. +/** Manages loading and reloading objects. Uses configurations from the class LoadablesConfigReader. * Supports parallel loading. */ class ExternalLoader::LoadingDispatcher : private boost::noncopyable @@ -248,7 +242,7 @@ public: } } - using ObjectConfigsPtr = ConfigFilesReader::ObjectConfigsPtr; + using ObjectConfigsPtr = LoadablesConfigReader::ObjectConfigsPtr; /// Sets new configurations for all the objects. void setConfiguration(const ObjectConfigsPtr & new_configs) @@ -883,7 +877,7 @@ class ExternalLoader::PeriodicUpdater : private boost::noncopyable public: static constexpr UInt64 check_period_sec = 5; - PeriodicUpdater(ConfigFilesReader & config_files_reader_, LoadingDispatcher & loading_dispatcher_) + PeriodicUpdater(LoadablesConfigReader & config_files_reader_, LoadingDispatcher & loading_dispatcher_) : config_files_reader(config_files_reader_), loading_dispatcher(loading_dispatcher_) { } @@ -933,7 +927,7 @@ private: } } - ConfigFilesReader & config_files_reader; + LoadablesConfigReader & config_files_reader; LoadingDispatcher & loading_dispatcher; mutable std::mutex mutex; @@ -943,8 +937,8 @@ private: }; -ExternalLoader::ExternalLoader(const Poco::Util::AbstractConfiguration & main_config, const String & type_name_, Logger * log) - : config_files_reader(std::make_unique(main_config, type_name_, log)) +ExternalLoader::ExternalLoader(const String & type_name_, Logger * log) + : config_files_reader(std::make_unique(type_name_, log)) , loading_dispatcher(std::make_unique( std::bind(&ExternalLoader::createObject, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4), type_name_, @@ -957,7 +951,7 @@ ExternalLoader::ExternalLoader(const Poco::Util::AbstractConfiguration & main_co ExternalLoader::~ExternalLoader() = default; void ExternalLoader::addConfigRepository( - std::unique_ptr config_repository, const ExternalLoaderConfigSettings & config_settings) + std::unique_ptr config_repository, const ExternalLoaderConfigSettings & config_settings) { config_files_reader->addConfigRepository(std::move(config_repository), config_settings); loading_dispatcher->setConfiguration(config_files_reader->read()); diff --git a/dbms/src/Interpreters/ExternalLoader.h b/dbms/src/Interpreters/ExternalLoader.h index 30b54d4490c..5c1fd1c0416 100644 --- a/dbms/src/Interpreters/ExternalLoader.h +++ b/dbms/src/Interpreters/ExternalLoader.h @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include @@ -24,8 +24,6 @@ struct ExternalLoaderConfigSettings { std::string external_config; std::string external_name; - - std::string path_setting_name; }; @@ -78,12 +76,12 @@ public: using LoadResults = std::vector>; - ExternalLoader(const Poco::Util::AbstractConfiguration & main_config, const String & type_name_, Logger * log); + ExternalLoader(const String & type_name_, Logger * log); virtual ~ExternalLoader(); /// Adds a repository which will be used to read configurations from. void addConfigRepository( - std::unique_ptr config_repository, const ExternalLoaderConfigSettings & config_settings); + std::unique_ptr config_repository, const ExternalLoaderConfigSettings & config_settings); /// Sets whether all the objects from the configuration should be always loaded (even those which are never used). void enableAlwaysLoadEverything(bool enable); @@ -157,8 +155,8 @@ private: LoadablePtr createObject(const String & name, const ObjectConfig & config, bool config_changed, const LoadablePtr & previous_version) const; - class ConfigFilesReader; - std::unique_ptr config_files_reader; + class LoadablesConfigReader; + std::unique_ptr config_files_reader; class LoadingDispatcher; std::unique_ptr loading_dispatcher; diff --git a/dbms/src/Interpreters/ExternalLoaderConfigRepository.cpp b/dbms/src/Interpreters/ExternalLoaderConfigRepository.cpp deleted file mode 100644 index bb8dd61ee2d..00000000000 --- a/dbms/src/Interpreters/ExternalLoaderConfigRepository.cpp +++ /dev/null @@ -1,72 +0,0 @@ -#include - -#include -#include -#include - -#include -#include -#include - - -namespace DB -{ - -ExternalLoaderConfigRepository::Files ExternalLoaderConfigRepository::list( - const Poco::Util::AbstractConfiguration & config, - const std::string & path_key) const -{ - Files files; - - auto patterns = getMultipleValuesFromConfig(config, "", path_key); - - for (auto & pattern : patterns) - { - if (pattern.empty()) - continue; - - if (pattern[0] != '/') - { - const auto app_config_path = config.getString("config-file", "config.xml"); - const auto config_dir = Poco::Path{app_config_path}.parent().toString(); - const auto absolute_path = config_dir + pattern; - Poco::Glob::glob(absolute_path, files, 0); - if (!files.empty()) - continue; - } - - Poco::Glob::glob(pattern, files, 0); - } - - for (Files::iterator it = files.begin(); it != files.end();) - { - if (ConfigProcessor::isPreprocessedFile(*it)) - files.erase(it++); - else - ++it; - } - - return files; -} - -bool ExternalLoaderConfigRepository::exists(const std::string & config_file) const -{ - return Poco::File(config_file).exists(); -} - -Poco::Timestamp ExternalLoaderConfigRepository::getLastModificationTime( - const std::string & config_file) const -{ - return Poco::File(config_file).getLastModified(); -} - -Poco::AutoPtr ExternalLoaderConfigRepository::load( - const std::string & config_file, const std::string & preprocessed_dir) const -{ - ConfigProcessor config_processor{config_file}; - ConfigProcessor::LoadedConfig preprocessed = config_processor.loadConfig(); - config_processor.savePreprocessedConfig(preprocessed, preprocessed_dir); - return preprocessed.configuration; -} - -} diff --git a/dbms/src/Interpreters/ExternalLoaderConfigRepository.h b/dbms/src/Interpreters/ExternalLoaderConfigRepository.h deleted file mode 100644 index 06280803a43..00000000000 --- a/dbms/src/Interpreters/ExternalLoaderConfigRepository.h +++ /dev/null @@ -1,31 +0,0 @@ -#pragma once - -#include -#include -#include - -#include -#include - -namespace DB -{ - -/** Config repository used by native server application. - * Represents files in local filesystem. - */ -class ExternalLoaderConfigRepository -{ -public: - using Files = std::set; - Files list( - const Poco::Util::AbstractConfiguration & config, - const std::string & path_key) const; - - bool exists(const std::string & config_file) const; - - Poco::Timestamp getLastModificationTime(const std::string & config_file) const; - - Poco::AutoPtr load(const std::string & config_file, const std::string & preprocessed_dir = "") const; -}; - -} diff --git a/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.cpp b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.cpp new file mode 100644 index 00000000000..9592360a92f --- /dev/null +++ b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.cpp @@ -0,0 +1,78 @@ +#include + +#include +#include +#include + +#include +#include +#include + + +namespace DB +{ + +bool ExternalLoaderXMLConfigRepository::isUpdated(const std::string & definition_entity_name) +{ + Poco::Timestamp last_modified = Poco::File(definition_entity_name).getLastModified(); + + auto itr = update_time_mapping.find(definition_entity_name); + if (itr == update_time_mapping.end() || last_modified > itr->second) + { + update_time_mapping[definition_entity_name] = last_modified; + return true; + } + + return false; +} + +std::set ExternalLoaderXMLConfigRepository::getAllLoadablesDefinitionNames() const +{ + std::set files; + + auto patterns = getMultipleValuesFromConfig(main_config, "", config_key); + + for (auto & pattern : patterns) + { + if (pattern.empty()) + continue; + + if (pattern[0] != '/') + { + const auto app_config_path = main_config.getString("config-file", "config.xml"); + const auto config_dir = Poco::Path{app_config_path}.parent().toString(); + const auto absolute_path = config_dir + pattern; + Poco::Glob::glob(absolute_path, files, 0); + if (!files.empty()) + continue; + } + + Poco::Glob::glob(pattern, files, 0); + } + + for (std::set::iterator it = files.begin(); it != files.end();) + { + if (ConfigProcessor::isPreprocessedFile(*it)) + files.erase(it++); + else + ++it; + } + + return files; +} + +bool ExternalLoaderXMLConfigRepository::exists(const std::string & definition_entity_name) const +{ + return Poco::File(definition_entity_name).exists(); +} + +Poco::AutoPtr ExternalLoaderXMLConfigRepository::load( + const std::string & config_file) const +{ + ConfigProcessor config_processor{config_file}; + ConfigProcessor::LoadedConfig preprocessed = config_processor.loadConfig(); + config_processor.savePreprocessedConfig(preprocessed, main_config.getString("path", DBMS_DEFAULT_PATH)); + return preprocessed.configuration; +} + +} diff --git a/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h new file mode 100644 index 00000000000..94dfeb13fba --- /dev/null +++ b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h @@ -0,0 +1,45 @@ +#pragma once + +#include +#include +#include +#include + +namespace DB +{ + +/// XML config repository used by ExternalLoader. +/// Represents xml-files in local filesystem. +class ExternalLoaderXMLConfigRepository : public IExternalLoaderConfigRepository +{ +public: + + ExternalLoaderXMLConfigRepository(const Poco::Util::AbstractConfiguration & main_config_, const std::string & config_key_) + : main_config(main_config_) + , config_key(config_key_) + { + } + + std::set getAllLoadablesDefinitionNames() const override; + + bool exists(const std::string & definition_entity_name) const override; + + bool isUpdated(const std::string & definition_entity_name) override; + + /// May contain definition about several entities (several dictionaries in one .xml file) + LoadablesConfigurationPtr load(const std::string & definition_entity_name) const override; + +private: + + /// Simple map with last modification time with path -> timestamp, + /// modification time received by stat. + std::unordered_map update_time_mapping; + + /// Main server config (config.xml). + const Poco::Util::AbstractConfiguration & main_config; + + /// Key which contains path to dicrectory with .xml configs for entries + std::string config_key; +}; + +} diff --git a/dbms/src/Interpreters/ExternalModelsLoader.cpp b/dbms/src/Interpreters/ExternalModelsLoader.cpp index 77ee6147524..462e8110249 100644 --- a/dbms/src/Interpreters/ExternalModelsLoader.cpp +++ b/dbms/src/Interpreters/ExternalModelsLoader.cpp @@ -11,14 +11,11 @@ namespace ErrorCodes ExternalModelsLoader::ExternalModelsLoader( - std::unique_ptr config_repository, - Context & context_) - : ExternalLoader(context_.getConfigRef(), - "external model", - &Logger::get("ExternalModelsLoader")), - context(context_) + ExternalLoaderConfigRepositoryPtr config_repository, Context & context_) + : ExternalLoader("external model", &Logger::get("ExternalModelsLoader")) + , context(context_) { - addConfigRepository(std::move(config_repository), {"model", "name", "models_config"}); + addConfigRepository(std::move(config_repository), {"model", "name"}); enablePeriodicUpdates(true); } diff --git a/dbms/src/Interpreters/ExternalModelsLoader.h b/dbms/src/Interpreters/ExternalModelsLoader.h index d1523fabf29..fa860d08b2b 100644 --- a/dbms/src/Interpreters/ExternalModelsLoader.h +++ b/dbms/src/Interpreters/ExternalModelsLoader.h @@ -19,7 +19,7 @@ public: /// Models will be loaded immediately and then will be updated in separate thread, each 'reload_period' seconds. ExternalModelsLoader( - std::unique_ptr config_repository, + ExternalLoaderConfigRepositoryPtr config_repository, Context & context_); ModelPtr getModel(const std::string & name) const diff --git a/dbms/src/Interpreters/IExternalLoaderConfigRepository.h b/dbms/src/Interpreters/IExternalLoaderConfigRepository.h new file mode 100644 index 00000000000..cc404112021 --- /dev/null +++ b/dbms/src/Interpreters/IExternalLoaderConfigRepository.h @@ -0,0 +1,42 @@ +#pragma once + +#include +#include + +#include +#include + +namespace DB +{ + +using LoadablesConfigurationPtr = Poco::AutoPtr; + +/// Base interface for configurations source for Loadble objects, which can be +/// loaded with ExternalLoader. Configurations may came from filesystem (XML-files), +/// server memory (from database), etc. It's important that main result of this class +/// (LoadablesConfigurationPtr) may contain more than one loadable config, +/// each one with own key, which can be obtained with keys method, +/// for example multiple dictionaries can be defined in single .xml file. +class IExternalLoaderConfigRepository +{ +public: + /// Return all available sources of loadables structure + /// (all .xml files from fs, all entities from database, etc) + virtual std::set getAllLoadablesDefinitionNames() const = 0; + + /// Checks that source of loadables configuration exist. + virtual bool exists(const std::string & loadable_definition_name) const = 0; + + /// Checks that entity was updated since last call of this method. + /// Assumes usage of some state and probably some mutex. + virtual bool isUpdated(const std::string & loadable_definition_name) = 0; + + /// Load configuration from some concrete source to AbstractConfiguration + virtual LoadablesConfigurationPtr load(const std::string & loadable_definition_name) const = 0; + + virtual ~IExternalLoaderConfigRepository() = default; +}; + +using ExternalLoaderConfigRepositoryPtr = std::unique_ptr; + +} From 0e6331e8923f36fbec0c8a8efaf97d6a481fd946 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 30 Sep 2019 19:54:50 +0300 Subject: [PATCH 3/5] Add comments --- dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h index 94dfeb13fba..e043a97d737 100644 --- a/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h +++ b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h @@ -20,10 +20,13 @@ public: { } + /// Return set of .xml files from path in main_config (config_key) std::set getAllLoadablesDefinitionNames() const override; + /// Checks that file with name exists on filesystem bool exists(const std::string & definition_entity_name) const override; + /// Checks that file was updated since last check bool isUpdated(const std::string & definition_entity_name) override; /// May contain definition about several entities (several dictionaries in one .xml file) From 709783a1bc28dbe4c4b7f4be01e14d4359fadea9 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 30 Sep 2019 22:42:31 +0300 Subject: [PATCH 4/5] Add missed header --- dbms/src/Interpreters/IExternalLoaderConfigRepository.h | 1 + 1 file changed, 1 insertion(+) diff --git a/dbms/src/Interpreters/IExternalLoaderConfigRepository.h b/dbms/src/Interpreters/IExternalLoaderConfigRepository.h index cc404112021..9855c074bd8 100644 --- a/dbms/src/Interpreters/IExternalLoaderConfigRepository.h +++ b/dbms/src/Interpreters/IExternalLoaderConfigRepository.h @@ -3,6 +3,7 @@ #include #include +#include #include #include From 8429f46f3c9eae3b96973eccda7fedbc0faf9aa5 Mon Sep 17 00:00:00 2001 From: alesapin Date: Tue, 1 Oct 2019 11:58:47 +0300 Subject: [PATCH 5/5] Return getUpdateTime method to interface --- dbms/src/Interpreters/ExternalLoader.cpp | 7 ++++++- .../ExternalLoaderXMLConfigRepository.cpp | 13 ++----------- .../ExternalLoaderXMLConfigRepository.h | 9 ++------- .../Interpreters/IExternalLoaderConfigRepository.h | 6 +++--- 4 files changed, 13 insertions(+), 22 deletions(-) diff --git a/dbms/src/Interpreters/ExternalLoader.cpp b/dbms/src/Interpreters/ExternalLoader.cpp index 1b0af14a086..b4c61f5a5da 100644 --- a/dbms/src/Interpreters/ExternalLoader.cpp +++ b/dbms/src/Interpreters/ExternalLoader.cpp @@ -84,6 +84,7 @@ public: private: struct LoadablesInfos { + Poco::Timestamp last_update_time = 0; std::vector> configs; // Parsed file's contents. bool in_use = true; // Whether the ` LoadablesInfos` should be destroyed because the correspondent file is deleted. }; @@ -150,7 +151,10 @@ private: return false; } - if (!repository.isUpdated(path)) + auto update_time_from_repository = repository.getUpdateTime(path); + + /// Actually it can't be less, but for sure we check less or equal + if (update_time_from_repository <= loadable_info.last_update_time) { loadable_info.in_use = true; return false; @@ -184,6 +188,7 @@ private: } loadable_info.configs = std::move(configs_from_file); + loadable_info.last_update_time = update_time_from_repository; loadable_info.in_use = true; return true; } diff --git a/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.cpp b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.cpp index 9592360a92f..9a5e32697df 100644 --- a/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.cpp +++ b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.cpp @@ -12,18 +12,9 @@ namespace DB { -bool ExternalLoaderXMLConfigRepository::isUpdated(const std::string & definition_entity_name) +Poco::Timestamp ExternalLoaderXMLConfigRepository::getUpdateTime(const std::string & definition_entity_name) { - Poco::Timestamp last_modified = Poco::File(definition_entity_name).getLastModified(); - - auto itr = update_time_mapping.find(definition_entity_name); - if (itr == update_time_mapping.end() || last_modified > itr->second) - { - update_time_mapping[definition_entity_name] = last_modified; - return true; - } - - return false; + return Poco::File(definition_entity_name).getLastModified(); } std::set ExternalLoaderXMLConfigRepository::getAllLoadablesDefinitionNames() const diff --git a/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h index e043a97d737..b8676209c14 100644 --- a/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h +++ b/dbms/src/Interpreters/ExternalLoaderXMLConfigRepository.h @@ -26,18 +26,13 @@ public: /// Checks that file with name exists on filesystem bool exists(const std::string & definition_entity_name) const override; - /// Checks that file was updated since last check - bool isUpdated(const std::string & definition_entity_name) override; + /// Return xml-file modification time via stat call + Poco::Timestamp getUpdateTime(const std::string & definition_entity_name) override; /// May contain definition about several entities (several dictionaries in one .xml file) LoadablesConfigurationPtr load(const std::string & definition_entity_name) const override; private: - - /// Simple map with last modification time with path -> timestamp, - /// modification time received by stat. - std::unordered_map update_time_mapping; - /// Main server config (config.xml). const Poco::Util::AbstractConfiguration & main_config; diff --git a/dbms/src/Interpreters/IExternalLoaderConfigRepository.h b/dbms/src/Interpreters/IExternalLoaderConfigRepository.h index 9855c074bd8..93cefe0a0d4 100644 --- a/dbms/src/Interpreters/IExternalLoaderConfigRepository.h +++ b/dbms/src/Interpreters/IExternalLoaderConfigRepository.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -28,9 +29,8 @@ public: /// Checks that source of loadables configuration exist. virtual bool exists(const std::string & loadable_definition_name) const = 0; - /// Checks that entity was updated since last call of this method. - /// Assumes usage of some state and probably some mutex. - virtual bool isUpdated(const std::string & loadable_definition_name) = 0; + /// Returns entity last update time + virtual Poco::Timestamp getUpdateTime(const std::string & loadable_definition_name) = 0; /// Load configuration from some concrete source to AbstractConfiguration virtual LoadablesConfigurationPtr load(const std::string & loadable_definition_name) const = 0;