From 751861e9a0b5da86e4a2ba1e25415debaee2bd9f Mon Sep 17 00:00:00 2001 From: Igor Gaponenko Date: Sun, 16 Jan 2022 00:30:23 -0600 Subject: [PATCH 1/2] Uncoupled general config parameters from the persistent backend --- src/replica/ChunksApp.cc | 7 +- src/replica/ConfigApp.cc | 51 +---- src/replica/ConfigApp.h | 9 - src/replica/ConfigAppBase.cc | 27 --- src/replica/ConfigAppBase.h | 3 +- src/replica/ConfigParserMySQL.cc | 39 ---- src/replica/ConfigParserMySQL.h | 24 --- src/replica/ConfigTestApp.cc | 316 +------------------------------ src/replica/ConfigTestApp.h | 1 - src/replica/Configuration.cc | 30 +-- src/replica/Configuration.h | 35 +--- src/replica/IndexApp.cc | 4 +- src/replica/SqlApp.cc | 4 +- 13 files changed, 19 insertions(+), 531 deletions(-) diff --git a/src/replica/ChunksApp.cc b/src/replica/ChunksApp.cc index 33636d0721..66b3b540bc 100644 --- a/src/replica/ChunksApp.cc +++ b/src/replica/ChunksApp.cc @@ -222,11 +222,8 @@ int ChunksApp::runImpl() { // Limit execution timeout for requests if such limit was provided if (_timeoutSec != 0) { - bool const updatePersistentState = false; - serviceProvider()->config()->set( - "controller", "request-timeout-sec", _timeoutSec, updatePersistentState); - serviceProvider()->config()->set( - "xrootd", "request-timeout-sec", _timeoutSec, updatePersistentState); + serviceProvider()->config()->set("controller", "request-timeout-sec", _timeoutSec); + serviceProvider()->config()->set("xrootd", "request-timeout-sec", _timeoutSec); } /////////////////////////////////////////////////////////////////// diff --git a/src/replica/ConfigApp.cc b/src/replica/ConfigApp.cc index 5170b5c03e..bdbdc25c68 100644 --- a/src/replica/ConfigApp.cc +++ b/src/replica/ConfigApp.cc @@ -81,7 +81,6 @@ ConfigApp::ConfigApp(int argc, char* argv[]) "command", {"DUMP", "CONFIG_INIT_FILE", - "UPDATE_GENERAL", "UPDATE_WORKER", "ADD_WORKER", "DELETE_WORKER", "ADD_DATABASE_FAMILY", "DELETE_DATABASE_FAMILY", "ADD_DATABASE", "PUBLISH_DATABASE", "DELETE_DATABASE", @@ -96,13 +95,13 @@ ConfigApp::ConfigApp(int argc, char* argv[]) "Dump existing configuration parameters. The command allows specifying" " an optional scope to limit the output. If the parameter 'scope' is" " omitted then complete configuration will be printed. Allowed scopes:" - " 'GENERAL', 'WORKERS', 'FAMILIES', 'DATABASES'." + " 'WORKERS', 'FAMILIES', 'DATABASES'." ).optional( "scope", "This optional parameter narrows a scope of the operation down to a specific" " context. If no scope is specified then everything will be dumped.", _dumpScope, - vector({"GENERAL", "WORKERS", "FAMILIES", "DATABASES"}) + vector({"WORKERS", "FAMILIES", "DATABASES"}) ); parser().command( @@ -165,29 +164,6 @@ ConfigApp::ConfigApp(int argc, char* argv[]) " This includes: replicas info and transaction contribution records." ); - // Add options for the general parameters named as: - // --.= - // Note that since no database connection is available at this time (that would have - // required knowing a value of the parameter 'configUrl', and no parsing has been made - // yet) then the loop below will set the default value of each option to be the empty - // string. Any changes from that will be detected when processing the input. - auto&& updateGeneralCmd = parser().command( - "UPDATE_GENERAL" - ).description( - "Update the general configuration parameters." - ); - for (auto&& itr: ConfigurationSchema::parameters()) { - string const& category = itr.first; - for (auto&& param: itr.second) { - // The read-only parameters can't be updated programmatically. - if (ConfigurationSchema::readOnly(category, param)) continue; - _generalParams[category][param] = ConfigurationSchema::defaultValueAsString(category, param); - updateGeneralCmd.option( - category + "-" + param, - ConfigurationSchema::description(category, param), - _generalParams[category][param]); - } - } parser().command( "ADD_DATABASE_FAMILY" @@ -335,7 +311,6 @@ int ConfigApp::runSubclassImpl() { if (_command == "DUMP") return _dump(); if (_command == "CONFIG_INIT_FILE") return _configInitFile(); - if (_command == "UPDATE_GENERAL") return _updateGeneral(); if (_command == "UPDATE_WORKER") return _updateWorker(); if (_command == "ADD_WORKER") return _addWorker(); if (_command == "DELETE_WORKER") return _deleteWorker(); @@ -420,7 +395,6 @@ void ConfigApp::_configureWorkerOptions(detail::Command& command) { int ConfigApp::_dump() const { string const indent = " "; cout << "\n" << indent << "CONFIG_URL: " << configUrl() << "\n" << "\n"; - if (_dumpScope.empty() or _dumpScope == "GENERAL") dumpGeneralAsTable(indent); if (_dumpScope.empty() or _dumpScope == "WORKERS") dumpWorkersAsTable(indent); if (_dumpScope.empty() or _dumpScope == "FAMILIES") dumpFamiliesAsTable(indent); if (_dumpScope.empty() or _dumpScope == "DATABASES") dumpDatabasesAsTable(indent); @@ -443,27 +417,6 @@ int ConfigApp::_configInitFile() const { } -int ConfigApp::_updateGeneral() { - try { - // Note that options specified by a user will have non-empty values. - for (auto&& categoryItr: _generalParams) { - string const& category = categoryItr.first; - for (auto&& paramItr: categoryItr.second) { - string const& param = paramItr.first; - string const& value = paramItr.second; - if (!value.empty()) { - config()->setFromString(category, param, value); - } - } - } - } catch (exception const& ex) { - LOGS(_log, LOG_LVL_ERROR, "ConfigApp::" << __func__ << ": " << ex.what()); - throw; - } - return 0; -} - - int ConfigApp::_updateWorker() const { // Configuration changes will be updated in the transient object obtained from // the database and then be saved to the the persistent configuration. diff --git a/src/replica/ConfigApp.h b/src/replica/ConfigApp.h index dc8c0bddb1..ac9f7ea883 100644 --- a/src/replica/ConfigApp.h +++ b/src/replica/ConfigApp.h @@ -92,12 +92,6 @@ class ConfigApp: public ConfigAppBase { */ int _configInitFile() const; - /** - * Update the general configuration parameters - * @return A status code to be returned to the shell. - */ - int _updateGeneral(); - /** * Update parameters of a worker * @return A status code to be returned to the shell. @@ -181,9 +175,6 @@ class ConfigApp: public ConfigAppBase { /// of 1 (or any positive number) means - enable the read-write mode for the worker . int _workerReadOnly = -1; - /// General parameters - std::map> _generalParams; - /// For database families DatabaseFamilyInfo _familyInfo; diff --git a/src/replica/ConfigAppBase.cc b/src/replica/ConfigAppBase.cc index a385414d5a..d83251a71a 100644 --- a/src/replica/ConfigAppBase.cc +++ b/src/replica/ConfigAppBase.cc @@ -75,33 +75,6 @@ int ConfigAppBase::runImpl() { } -void ConfigAppBase::dumpGeneralAsTable(string const& indent) const { - // Extract general attributes and put them into the corresponding - // columns. Translate tables cell values into strings when required. - vector categories; - vector parameters; - vector values; - vector descriptions; - for (auto&& itr: ConfigurationSchema::parameters()) { - string const& category = itr.first; - for (auto&& param: itr.second) { - categories.push_back(category); - parameters.push_back(param); - values.push_back(ConfigurationSchema::securityContext(category, param) ? - "xxxxxx" : _config->getAsString(category, param) - ); - descriptions.push_back(ConfigurationSchema::description(category, param)); - } - } - util::ColumnTablePrinter table("GENERAL PARAMETERS:", indent, verticalSeparator()); - table.addColumn("category", categories, util::ColumnTablePrinter::LEFT); - table.addColumn("param", parameters, util::ColumnTablePrinter::LEFT); - table.addColumn("value", values); - table.addColumn("description", descriptions, util::ColumnTablePrinter::LEFT); - table.print(cout, false, false); -} - - void ConfigAppBase::dumpWorkersAsTable(string const& indent, string const& caption) const { // Extract attributes of each worker and put them into the corresponding diff --git a/src/replica/ConfigAppBase.h b/src/replica/ConfigAppBase.h index 7e1183a62b..af43851a35 100644 --- a/src/replica/ConfigAppBase.h +++ b/src/replica/ConfigAppBase.h @@ -68,9 +68,8 @@ class ConfigAppBase: public Application { * Dump the Configuration into the standard output stream * @return A status code to be returned to the shell. */ - void dumpGeneralAsTable(std::string const& indent) const; void dumpWorkersAsTable(std::string const& indent, std::string const& caption="WORKERS:") const; - void dumpFamiliesAsTable(std::string const& indent, std::string const& caption="DATABASES FAMILIES") const; + void dumpFamiliesAsTable(std::string const& indent, std::string const& caption="DATABASE FAMILIES:") const; void dumpDatabasesAsTable(std::string const& indent, std::string const& caption="DATABASES:") const; private: diff --git a/src/replica/ConfigParserMySQL.cc b/src/replica/ConfigParserMySQL.cc index 4e86bbdd48..501ae55ed1 100644 --- a/src/replica/ConfigParserMySQL.cc +++ b/src/replica/ConfigParserMySQL.cc @@ -56,11 +56,6 @@ void ConfigParserMySQL::parse() { _parseVersion(); - // Read and update the transient state the general parameters and defaults - // shared by all components of the Replication system. The table also provides - // default values for some critical parameters of the worker-side services. - _parseGeneral(); - // Parse groupped parameters _parseWorkers(); _parseDatabaseFamilies(); @@ -94,40 +89,6 @@ void ConfigParserMySQL::_parseVersion() { } -void ConfigParserMySQL::_parseGeneral() { - _conn->execute("SELECT * FROM " + _conn->sqlId("config")); - while (_conn->next(_row)) { - string category; - _row.get("category", category); - - string param; - _row.get("param", param); - - json obj; - try { - obj = _data.at(category).at(param); - } catch (exception const& ex) { - throw invalid_argument( - _context + " no transient schema match for the parameter, category: '" - + category + "' param: '" + param + "'."); - } - if (obj.is_string()) { - _storeGeneralParameter(category, param); - } else if (obj.is_number_unsigned()) { - _storeGeneralParameter(category, param); - } else if (obj.is_number_integer()) { - _storeGeneralParameter(category, param); - } else if (obj.is_number_float()) { - _storeGeneralParameter(category, param); - } else { - throw invalid_argument( - _context + " unsupported transient schema type for the parameter, category: '" - + category + "' param: '" + param + "'."); - } - } -} - - void ConfigParserMySQL::_parseWorkers() { json& defaults = _data.at("worker-defaults"); _conn->execute("SELECT * FROM " + _conn->sqlId("config_worker")); diff --git a/src/replica/ConfigParserMySQL.h b/src/replica/ConfigParserMySQL.h index 733d9e0498..92d55fd7af 100644 --- a/src/replica/ConfigParserMySQL.h +++ b/src/replica/ConfigParserMySQL.h @@ -33,7 +33,6 @@ #include "replica/ConfigDatabase.h" #include "replica/ConfigDatabaseFamily.h" #include "replica/ConfigWorker.h" -#include "replica/ConfigurationSchema.h" #include "replica/DatabaseMySQL.h" // This header declarations @@ -86,9 +85,6 @@ class ConfigParserMySQL { */ void _parseVersion(); - /// Parse general (category/parameter) parameters. - void _parseGeneral(); - /** * Parse a collection of workers. * @@ -105,26 +101,6 @@ class ConfigParserMySQL { /// Parse a collection of the databases. void _parseDatabases(); - /** - * Extract a value of the general parameter into the requested type, sanitize the value - * if needed, and store it in the transent state. - * @throws std::runtime_error If a required field has NULL. - * @throws std::invalid_argument If the parameter's value didn't pass the validation. - */ - template - void _storeGeneralParameter(std::string const& category, std::string const& param) { - T value; - if (!_row.get("value", value)) { - throw std::runtime_error( - _context + " NULL is not allowed, category:'" + category - + "' param: '" + param + "'."); - } - // Sanitize the input to ensure it matches schema requirements before - // pushing the value into the configuration. - ConfigurationSchema::validate(category, param, value); - _data[category][param] = value; - } - template T _parseParam(std::string const& name) { T value; diff --git a/src/replica/ConfigTestApp.cc b/src/replica/ConfigTestApp.cc index 75488ccf70..7202fff926 100644 --- a/src/replica/ConfigTestApp.cc +++ b/src/replica/ConfigTestApp.cc @@ -53,124 +53,13 @@ string const description = " ATTENTION: Plan carefully when using this flag to avoid destroying any" " valuable data. Avoid running this command in the production environment."; -/** - * Register an option with a parser (which could also represent a command). - * @param parser The handler responsible for processing options - * @param param Parameter handler.` - */ -template -void addCommandOption(PARSER& parser, T& param) { - parser.option(param.key, param.description(), param.value); -} - // The strings for operaton completion reporting. string const PASSED_STR = "[PASSED]"; string const FAILED_STR = "[FAILED]"; string const OK_STR = "OK"; string const VALUE_MISMATCH_STR = "VALUE MISMATCH"; -string const TYPE_MISMATCH_STR = "TYPE MISMATCH"; -string const MISSING_STR = "MISSING"; -string const NOT_TESTED_STR = "NOT TESTED"; - -/** - * The class TestGeneral facilitates testing and reporting values of - * the general defaults. - */ -class TestGeneral { -public: - TestGeneral() = delete; - TestGeneral(TestGeneral const&) = delete; - TestGeneral& operator=(TestGeneral const&) = delete; - - TestGeneral(Configuration::Ptr const& config, - string const& caption, - string const& indent, - bool verticalSeparator) - : _config(config), - _caption(caption), - _indent(indent), - _verticalSeparator(verticalSeparator) { - } - - template - void verify(string const& category, string const& parameter, T const& expectedValue) { - _testedParameters[category].insert(parameter); - _category.push_back(category); - _parameter.push_back(parameter); - try { - T const actualValue = _config->get(category, parameter); - bool const equal = actualValue == expectedValue; - _result.push_back(equal ? OK_STR : VALUE_MISMATCH_STR); - _actual.push_back(detail::TypeConversionTrait::to_string(actualValue)); - if (!equal) ++_failed; - } catch(invalid_argument const& ex) { - _result.push_back(MISSING_STR); - _actual.push_back(""); - ++_failed; - } catch(ConfigTypeMismatch const& ex) { - _result.push_back(TYPE_MISMATCH_STR); - _actual.push_back(""); - ++_failed; - } catch(exception const& ex) { - _result.push_back(ex.what()); - _actual.push_back(""); - ++_failed; - } - _expected.push_back(detail::TypeConversionTrait::to_string(expectedValue)); - } - - /// @return 'true' if the test was successful. - bool reportResults() { - // Locate and report parameters that as not been tested. - map> const knownParameters = _config->parameters(); - if (knownParameters != _testedParameters) { - for (auto&& categoryItr: knownParameters) { - string const& category = categoryItr.first; - for (string const& parameter: categoryItr.second) { - if ((_testedParameters.count(category) == 0) || - (_testedParameters.at(category).count(parameter) == 0)) { - _result.push_back(NOT_TESTED_STR); - _category.push_back(category); - _parameter.push_back(parameter); - _actual.push_back(""); - _expected.push_back(""); - ++_failed; - } - } - } - } - string const caption = (_failed == 0 ? PASSED_STR : FAILED_STR) + " " + _caption; - util::ColumnTablePrinter table(caption, _indent, _verticalSeparator); - table.addColumn("result", _result); - table.addColumn("category", _category, util::ColumnTablePrinter::LEFT); - table.addColumn("parameter", _parameter, util::ColumnTablePrinter::LEFT); - table.addColumn("actual", _actual); - table.addColumn("expected", _expected); - table.print(cout, false, false); - return _failed == 0; - } - -private: - // Input parameters - Configuration::Ptr const _config; - string const _caption; - string const _indent; - bool const _verticalSeparator; - // Parameters that have been tested so far - map> _testedParameters; - - // The number of failed tests. - int _failed = 0; - - // Values accumulated along table columns. - vector _result; - vector _category; - vector _parameter; - vector _actual; - vector _expected; -}; /** * The class ComparatorBase represents the base class for specific comparators @@ -381,9 +270,9 @@ ConfigTestApp::ConfigTestApp(int argc, char* argv[]) parser().optional( "scope", "This optional parameter narrows a scope of the operation down to a specific" - " context. Allowed values: ALL, GENERAL, WORKERS, DATABASES_AND_FAMILIES, TABLES.", + " context. Allowed values: ALL, WORKERS, DATABASES_AND_FAMILIES, TABLES.", _testScope, - vector({"ALL", "GENERAL", "WORKERS", "DATABASES_AND_FAMILIES", "TABLES"}) + vector({"ALL", "WORKERS", "DATABASES_AND_FAMILIES", "TABLES"}) ); } @@ -391,12 +280,9 @@ ConfigTestApp::ConfigTestApp(int argc, char* argv[]) int ConfigTestApp::runSubclassImpl() { int result = 0; if (_testScope == "ALL") { - result += _testGeneral() ? 0 : 1; result += _testWorkers() ? 0 : 1; result += _testDatabasesAndFamilies() ? 0 : 1; result += _testTables() ? 0 : 1; - } else if (_testScope == "GENERAL") { - result += _testGeneral() ? 0 : 1; } else if (_testScope == "WORKERS") { result += _testWorkers() ? 0 : 1; } else if (_testScope == "DATABASES_AND_FAMILIES") { @@ -408,204 +294,6 @@ int ConfigTestApp::runSubclassImpl() { } -bool ConfigTestApp::_testGeneral() { - - string const indent = ""; - bool success = true; - - // Testing reading the default values using the generic API. results will be reported - // as a table onto the standard output. Note that the last argument in each - // call represents an expected value of the parameter's value. - { - TestGeneral test(config(), "READING DEAFULT STATE OF THE GENERAL PARAMETERS:", indent, verticalSeparator()); - test.verify( "common", "request-buf-size-bytes", 131072); - test.verify( "common", "request-retry-interval-sec", 1); - test.verify( "controller", "num-threads", 2); - test.verify( "controller", "http-server-threads", 2); - test.verify( "controller", "http-server-port", 25081); - test.verify( "controller", "http-max-listen-conn", - boost::asio::socket_base::max_listen_connections); - test.verify( "controller", "request-timeout-sec", 600); - test.verify( "controller", "job-timeout-sec", 600); - test.verify( "controller", "job-heartbeat-sec", 0); - test.verify( "controller", "empty-chunks-dir", "/qserv/data/qserv"); - test.verify( "controller", "worker-evict-priority-level", PRIORITY_VERY_HIGH); - test.verify( "controller", "health-monitor-priority-level", PRIORITY_VERY_HIGH); - test.verify( "controller", "ingest-priority-level", PRIORITY_HIGH); - test.verify( "controller", "catalog-management-priority-level", PRIORITY_LOW); - - test.verify( "database", "services-pool-size", 2); - test.verify( "database", "host", "localhost"); - test.verify( "database", "port", 23306); - test.verify( "database", "user", "root"); - test.verify( "database", "password", "CHANGEME"); - test.verify( "database", "name", "qservReplica"); - test.verify( "database", "qserv-master-user", "qsmaster"); - test.verify( "database", "qserv-master-services-pool-size", 2); - test.verify( "database", "qserv-master-tmp-dir", "/qserv/data/ingest"); - test.verify( "xrootd", "auto-notify", 1); - test.verify( "xrootd", "request-timeout-sec", 180); - test.verify( "xrootd", "host", "localhost"); - test.verify( "xrootd", "port", 1094); - test.verify( "xrootd", "allow-reconnect", 1); - test.verify( "xrootd", "reconnect-timeout", 3600); - test.verify( "worker", "technology", "FS"); - test.verify( "worker", "num-svc-processing-threads", 2); - test.verify( "worker", "num-fs-processing-threads", 2); - test.verify( "worker", "fs-buf-size-bytes", 4194304); - test.verify( "worker", "num-loader-processing-threads", 2); - test.verify( "worker", "num-exporter-processing-threads", 2); - test.verify( "worker", "num-http-loader-processing-threads", 2); - test.verify( "worker", "num-async-loader-processing-threads", 2); - test.verify( "worker", "async-loader-auto-resume", 1); - test.verify( "worker", "async-loader-cleanup-on-resume", 1); - test.verify( "worker", "http-max-listen-conn", - boost::asio::socket_base::max_listen_connections); - test.verify( "worker-defaults", "svc_port", 25000); - test.verify( "worker-defaults", "fs_port", 25001); - test.verify( "worker-defaults", "data_dir", "/qserv/data/mysql"); - test.verify( "worker-defaults", "loader_port", 25002); - test.verify( "worker-defaults", "loader_tmp_dir", "/qserv/data/ingest"); - test.verify( "worker-defaults", "exporter_port", 25003); - test.verify( "worker-defaults", "exporter_tmp_dir", "/qserv/data/export"); - test.verify( "worker-defaults", "http_loader_port", 25004); - test.verify( "worker-defaults", "http_loader_tmp_dir", "/qserv/data/ingest"); - success = success && test.reportResults(); - cout << "\n"; - } - - // Test updating the defaults and reading them back after reopening the database - // to ensure the values are read from the database rather than from the transinet state. - // - // Note that database parameters that are computed from the configUrl can't be updated - // in this way: - // "database.host" - // "database.port" - // "database.user" - // "database.password" - // "database.name" - // These parameters be be skipped in the update sequence. - { - config()->set( "common", "request-buf-size-bytes", 131072 + 1); - config()->set( "common", "request-retry-interval-sec", 1 + 1); - config()->set( "controller", "num-threads", 2 + 1); - config()->set( "controller", "http-server-threads", 2 + 1); - config()->set( "controller", "http-server-port", 25081 + 1); - config()->set( "controller", "http-max-listen-conn", - boost::asio::socket_base::max_listen_connections * 2); - config()->set( "controller", "request-timeout-sec", 600 + 1); - config()->set( "controller", "job-timeout-sec", 600 + 1); - config()->set( "controller", "job-heartbeat-sec", 0 + 1); - config()->set( "controller", "empty-chunks-dir", "/qserv/data/qserv-1"); - config()->set( "controller", "worker-evict-priority-level", PRIORITY_VERY_HIGH + 1); - config()->set( "controller", "health-monitor-priority-level", PRIORITY_VERY_HIGH + 1); - config()->set( "controller", "ingest-priority-level", PRIORITY_HIGH + 1); - config()->set( "controller", "catalog-management-priority-level", PRIORITY_LOW + 1); - - config()->set( "database", "services-pool-size", 2 + 1); - // These 5 parameters are deduced from 'configUrl'. Changing them here won't make - // a sense since they're not stored within MySQL. - if (false) { - config()->set("database", "host", "localhost"); - config()->set( "database", "port", 23306); - config()->set("database", "user", "root"); - config()->set("database", "password", "CHANGEME"); - config()->set("database", "name", "qservReplica"); - } - config()->set( "database", "qserv-master-user", "qsmaster-1"); - config()->set( "database", "qserv-master-services-pool-size", 2 + 1); - config()->set( "database", "qserv-master-tmp-dir", "/qserv/data/ingest-1"); - config()->set( "xrootd", "auto-notify", 0); - config()->set( "xrootd", "request-timeout-sec", 180 + 1); - config()->set( "xrootd", "host", "localhost-1"); - config()->set( "xrootd", "port", 1094 + 1); - config()->set( "xrootd", "allow-reconnect", 0); - config()->set( "xrootd", "reconnect-timeout", 120); - config()->set( "worker", "technology", "POSIX"); - config()->set( "worker", "num-svc-processing-threads", 2 + 1); - config()->set( "worker", "num-fs-processing-threads", 2 + 1); - config()->set( "worker", "fs-buf-size-bytes", 4194304 + 1); - config()->set( "worker", "num-loader-processing-threads", 2 + 1); - config()->set( "worker", "num-exporter-processing-threads", 2 + 1); - config()->set( "worker", "num-http-loader-processing-threads", 2 + 1); - config()->set( "worker", "num-async-loader-processing-threads", 2 + 1); - config()->set( "worker", "async-loader-auto-resume", 0); - config()->set( "worker", "async-loader-cleanup-on-resume", 0); - config()->set( "worker", "http-max-listen-conn", - boost::asio::socket_base::max_listen_connections * 4); - config()->set( "worker-defaults", "svc_port", 25000 + 1); - config()->set( "worker-defaults", "fs_port", 25001 + 1); - config()->set( "worker-defaults", "data_dir", "/qserv/data/mysql-1"); - config()->set( "worker-defaults", "loader_port", 25002 + 1); - config()->set( "worker-defaults", "loader_tmp_dir", "/qserv/data/ingest-1"); - config()->set( "worker-defaults", "exporter_port", 25003 + 1); - config()->set( "worker-defaults", "exporter_tmp_dir", "/qserv/data/export-1"); - config()->set( "worker-defaults", "http_loader_port", 25004 + 1); - config()->set( "worker-defaults", "http_loader_tmp_dir", "/qserv/data/ingest-1"); - - config()->reload(); - TestGeneral test(config(), "READING UPDATED GENERAL PARAMETERS:", indent, verticalSeparator()); - test.verify( "common", "request-buf-size-bytes", 131072 + 1); - test.verify( "common", "request-retry-interval-sec", 1 + 1); - test.verify( "controller", "num-threads", 2 + 1); - test.verify( "controller", "http-server-threads", 2 + 1); - test.verify( "controller", "http-server-port", 25081 + 1); - test.verify( "controller", "http-max-listen-conn", - boost::asio::socket_base::max_listen_connections * 2); - test.verify( "controller", "request-timeout-sec", 600 + 1); - test.verify( "controller", "job-timeout-sec", 600 + 1); - test.verify( "controller", "job-heartbeat-sec", 0 + 1); - test.verify( "controller", "empty-chunks-dir", "/qserv/data/qserv-1"); - test.verify( "controller", "worker-evict-priority-level", PRIORITY_VERY_HIGH + 1); - test.verify( "controller", "health-monitor-priority-level", PRIORITY_VERY_HIGH + 1); - test.verify( "controller", "ingest-priority-level", PRIORITY_HIGH + 1); - test.verify( "controller", "catalog-management-priority-level", PRIORITY_LOW + 1); - test.verify( "database", "services-pool-size", 2 + 1); - - // These 5 parameters deduced from 'configUrl' shouldn't be changed. - test.verify( "database", "host", "localhost"); - test.verify( "database", "port", 23306); - test.verify( "database", "user", "root"); - test.verify( "database", "password", "CHANGEME"); - test.verify( "database", "name", "qservReplica"); - - test.verify( "database", "qserv-master-user", "qsmaster-1"); - test.verify( "database", "qserv-master-services-pool-size", 2 + 1); - test.verify( "database", "qserv-master-tmp-dir", "/qserv/data/ingest-1"); - test.verify( "xrootd", "auto-notify", 0); - test.verify( "xrootd", "request-timeout-sec", 180 + 1); - test.verify( "xrootd", "host", "localhost-1"); - test.verify( "xrootd", "port", 1094 + 1); - test.verify( "xrootd", "allow-reconnect", 0); - test.verify( "xrootd", "reconnect-timeout", 120); - test.verify( "worker", "technology", "POSIX"); - test.verify( "worker", "num-svc-processing-threads", 2 + 1); - test.verify( "worker", "num-fs-processing-threads", 2 + 1); - test.verify( "worker", "fs-buf-size-bytes", 4194304 + 1); - test.verify( "worker", "num-loader-processing-threads", 2 + 1); - test.verify( "worker", "num-exporter-processing-threads", 2 + 1); - test.verify( "worker", "num-http-loader-processing-threads", 2 + 1); - test.verify( "worker", "num-async-loader-processing-threads", 2 + 1); - test.verify( "worker", "async-loader-auto-resume", 0); - test.verify( "worker", "async-loader-cleanup-on-resume", 0); - test.verify( "worker", "http-max-listen-conn", - boost::asio::socket_base::max_listen_connections * 4); - test.verify( "worker-defaults", "svc_port", 25000 + 1); - test.verify( "worker-defaults", "fs_port", 25001 + 1); - test.verify( "worker-defaults", "data_dir", "/qserv/data/mysql-1"); - test.verify( "worker-defaults", "loader_port", 25002 + 1); - test.verify( "worker-defaults", "loader_tmp_dir", "/qserv/data/ingest-1"); - test.verify( "worker-defaults", "exporter_port", 25003 + 1); - test.verify( "worker-defaults", "exporter_tmp_dir", "/qserv/data/export-1"); - test.verify( "worker-defaults", "http_loader_port", 25004 + 1); - test.verify( "worker-defaults", "http_loader_tmp_dir", "/qserv/data/ingest-1"); - success = success && test.reportResults(); - cout << "\n"; - } - return success; -} - - bool ConfigTestApp::_testWorkers() { // IMPORTANT: This test will reload configuration from the database after diff --git a/src/replica/ConfigTestApp.h b/src/replica/ConfigTestApp.h index 01c7627df0..f93e34c0e0 100644 --- a/src/replica/ConfigTestApp.h +++ b/src/replica/ConfigTestApp.h @@ -66,7 +66,6 @@ class ConfigTestApp: public ConfigAppBase { * The complete integration test for the Configuration service. */ int _test(); - bool _testGeneral(); bool _testWorkers(); bool _testDatabasesAndFamilies(); bool _testTables(); diff --git a/src/replica/Configuration.cc b/src/replica/Configuration.cc index 3674e6b40f..35ddccf23e 100644 --- a/src/replica/Configuration.cc +++ b/src/replica/Configuration.cc @@ -29,7 +29,7 @@ // Qserv headers #include "replica/ConfigParserJSON.h" #include "replica/ConfigParserMySQL.h" -#include "replica/DatabaseMySQL.h" +#include "replica/DatabaseMySQLExceptions.h" #include "util/Timer.h" // LSST headers @@ -281,20 +281,20 @@ string Configuration::getAsString(string const& category, string const& param) c void Configuration::setFromString(string const& category, string const& param, - string const& val, bool updatePersistentState) { + string const& val) { json obj; { util::Lock const lock(_mtx, _context(__func__)); obj = _get(lock, category, param); } if (obj.is_string()) { - Configuration::set(category, param, val, updatePersistentState); + Configuration::set(category, param, val); } else if (obj.is_number_unsigned()) { - Configuration::set(category, param, stoull(val), updatePersistentState); + Configuration::set(category, param, stoull(val)); } else if (obj.is_number_integer()) { - Configuration::set(category, param, stoll(val), updatePersistentState); + Configuration::set(category, param, stoll(val)); } else if (obj.is_number_float()) { - Configuration::set(category, param, stod(val), updatePersistentState); + Configuration::set(category, param, stod(val)); } else { throw invalid_argument( _context(__func__) + " unsupported data type of category: '" + category + "' param: '" + param @@ -782,24 +782,6 @@ json& Configuration::_get( } -void Configuration::_set( - string const& category, string const& param, string const& value) const -{ - _connectionPtr->executeInsertOrUpdate( - [&](decltype(_connectionPtr) conn) { - conn->executeInsertQuery("config", category, param, value); - }, - [&](decltype(_connectionPtr) conn) { - conn->executeSimpleUpdateQuery( - "config", - conn->sqlEqual("category", category) + " AND " + conn->sqlEqual("param", param), - make_pair("value", value) - ); - } - ); -} - - WorkerInfo Configuration::_updateWorker(util::Lock const& lock, WorkerInfo const& inputInfo) { // Make sure all required fields are set in the input worker descriptor. diff --git a/src/replica/Configuration.h b/src/replica/Configuration.h index 56cac433ee..4da5d2dae9 100644 --- a/src/replica/Configuration.h +++ b/src/replica/Configuration.h @@ -49,7 +49,7 @@ #include "replica/ConfigWorker.h" #include "replica/ConfigurationExceptions.h" #include "replica/ConfigurationSchema.h" -#include "replica/DatabaseMySQLExceptions.h" +#include "replica/DatabaseMySQL.h" #include "replica/DatabaseMySQLTypes.h" #include "util/Mutex.h" @@ -61,7 +61,6 @@ namespace replica { namespace database { namespace mysql { class Connection; - class ConnectionParams; }}}}} // Forward declarations // This header declarations @@ -92,10 +91,6 @@ struct TypeConversionTrait { /** * Class Configuration is the main API class that provide configuration services * for the components of the Replication system. - * @note The optional flag 'updatePersistentState' found in signatures of several - * "setter" methods will result in in propagating requested changes to the persistent - * store as well. The default value is set to 'true' since this would be a desired - * effect of the operations. * @note Exceptions mentioned in the documentation of the class's methods may not be * complete. Additional exceptions may be thrown depending on a presence of a persistent * backend for the configuration (such MySQL), Those which are mentioned explicitly are @@ -369,7 +364,7 @@ class Configuration { * @throws std::invalid_argument If there was a problem during setting a value of the parameter. */ template - void set(std::string const& category, std::string const& param, T const& val, bool updatePersistentState=true) { + void set(std::string const& category, std::string const& param, T const& val) { std::string const context_ = _context(__func__) + " category='" + category + "' param='" + param + "' "; util::Lock const lock(_mtx, context_); // Some parameters can't be updated using this interface. @@ -378,18 +373,10 @@ class Configuration { } // Validate the value in case if the schema enforces restrictions. ConfigurationSchema::validate(category, param, val); - // Update the persistent (if allowed) and then the transient states. + // Update transient states. try { nlohmann::json& obj = _get(lock, category, param); - if (updatePersistentState && (_connectionPtr != nullptr)) { - // Convert the value into a string. This is possible because values of - // the general parameters are stored as strings in the corresponding MySQL table. - _set(category, param, detail::TypeConversionTrait::to_string(val)); - } obj = val; - } catch (database::mysql::Error const& ex) { - throw std::invalid_argument( - context_ + " failed to update the persistent state of the parameter, ex: " + std::string(ex.what())); } catch (std::exception const& ex) { throw std::invalid_argument( context_ + " failed to set a new value of the parameter, ex: " + std::string(ex.what()) + "'."); @@ -402,7 +389,7 @@ class Configuration { * @see Configuration::set() */ void setFromString(std::string const& category, std::string const& param, - std::string const& val, bool updatePersistentState=true); + std::string const& val); /** * Return the names of known workers as per the selection criteria. @@ -740,20 +727,6 @@ class Configuration { std::string const& category, std::string const& param); - /** - * Update the general parameter parameter in the MySQL-based persistent store. - * @note This method won't check for correctness of the parameter values. It's expected - * it has been done by a caller. - * @param category The name of the parameter's category. - * @param param The name of the parameter within its category. - * @param value A value to be updated that has been serialized into a string to break the type barrier. - * @throws database::mysql::Error (or any derivatives of that exception)in case of any - * problems during the update. - */ - void _set(std::string const& category, - std::string const& param, - std::string const& value) const; - /** * Validate the input and add or update worker entry. * @param lock The lock on '_mtx' to be acquired prior to calling the method. diff --git a/src/replica/IndexApp.cc b/src/replica/IndexApp.cc index a0393ea0f4..69a1e10d2a 100644 --- a/src/replica/IndexApp.cc +++ b/src/replica/IndexApp.cc @@ -151,9 +151,7 @@ int IndexApp::runImpl() { // Limit execution timeout for requests if such limit was provided if (_timeoutSec != 0) { - bool const updatePersistentState = false; - serviceProvider()->config()->set( - "controller", "request-timeout-sec", _timeoutSec, updatePersistentState); + serviceProvider()->config()->set("controller", "request-timeout-sec", _timeoutSec); } string const noParentJobId; diff --git a/src/replica/SqlApp.cc b/src/replica/SqlApp.cc index 2714b91312..30b4505e5b 100644 --- a/src/replica/SqlApp.cc +++ b/src/replica/SqlApp.cc @@ -414,9 +414,7 @@ int SqlApp::runImpl() { // Limit request execution time if such limit was provided if (_timeoutSec != 0) { - bool const updatePersistentState = false; - serviceProvider()->config()->set( - "controller", "request-timeout-sec", _timeoutSec, updatePersistentState); + serviceProvider()->config()->set("controller", "request-timeout-sec", _timeoutSec); } auto const controller = Controller::create(serviceProvider()); From 494d46b7b16b9c0234cc3fd6ed7350c07959be1c Mon Sep 17 00:00:00 2001 From: Igor Gaponenko Date: Sun, 16 Jan 2022 00:37:44 -0600 Subject: [PATCH 2/2] Schema migration due to elimination of the config table --- src/replica/ConfigParserMySQL.cc | 2 +- src/replica/schema/migrate-6-to-7.sql | 4 ++++ ...migrate-None-to-6.sql => migrate-None-to-7.sql} | 14 +------------- 3 files changed, 6 insertions(+), 14 deletions(-) create mode 100644 src/replica/schema/migrate-6-to-7.sql rename src/replica/schema/{migrate-None-to-6.sql => migrate-None-to-7.sql} (97%) diff --git a/src/replica/ConfigParserMySQL.cc b/src/replica/ConfigParserMySQL.cc index 501ae55ed1..8cf75083c5 100644 --- a/src/replica/ConfigParserMySQL.cc +++ b/src/replica/ConfigParserMySQL.cc @@ -36,7 +36,7 @@ namespace lsst { namespace qserv { namespace replica { -int const ConfigParserMySQL::expectedSchemaVersion = 6; +int const ConfigParserMySQL::expectedSchemaVersion = 7; ConfigParserMySQL::ConfigParserMySQL(database::mysql::Connection::Ptr const& conn, diff --git a/src/replica/schema/migrate-6-to-7.sql b/src/replica/schema/migrate-6-to-7.sql new file mode 100644 index 0000000000..1701bb855e --- /dev/null +++ b/src/replica/schema/migrate-6-to-7.sql @@ -0,0 +1,4 @@ +-- This table used to store the general configuration parameters of the system. +-- It's no longer used in the current version of the code. + +DROP TABLE IF EXISTS `config`; diff --git a/src/replica/schema/migrate-None-to-6.sql b/src/replica/schema/migrate-None-to-7.sql similarity index 97% rename from src/replica/schema/migrate-None-to-6.sql rename to src/replica/schema/migrate-None-to-7.sql index acbbcd8cb9..cb603e7365 100644 --- a/src/replica/schema/migrate-None-to-6.sql +++ b/src/replica/schema/migrate-None-to-7.sql @@ -3,18 +3,6 @@ -- be to use the schema migration tool 'smig'. The 'smig' support has been added -- to this package as well. See the version specific files for further details. -CREATE TABLE IF NOT EXISTS `config` ( - `category` VARCHAR(255) NOT NULL , - `param` VARCHAR(255) NOT NULL , - `value` VARCHAR(255) NOT NULL , - PRIMARY KEY (`category`,`param`) -) -ENGINE = InnoDB -COMMENT = 'The common parameters and defaults shared by all components - of the replication system. It also provides default values - for some critical parameters of the worker-side services'; - - CREATE TABLE IF NOT EXISTS `config_worker` ( `name` VARCHAR(255) NOT NULL , -- the name of the worker `is_enabled` BOOLEAN NOT NULL , -- is enabled for replication @@ -478,4 +466,4 @@ ENGINE = InnoDB COMMENT = 'Metadata about database as a whole, key-value pairs' ; -- Add record for schema version, migration script expects this record to exist -INSERT INTO `QMetadata` (`metakey`, `value`) VALUES ('version', '6'); +INSERT INTO `QMetadata` (`metakey`, `value`) VALUES ('version', '7');