From e780c1292d837b03365f4e210d3a270e66257c4f Mon Sep 17 00:00:00 2001 From: DF5HSE Date: Tue, 18 Jan 2022 01:57:05 +0300 Subject: [PATCH] Add tests, remove saving host in config --- programs/client/Client.cpp | 64 +++++++++---------- src/Client/ClientBase.h | 2 +- src/Client/ConnectionParameters.cpp | 27 +++++--- src/Client/ConnectionParameters.h | 1 + ..._multiple_hosts_command_line_set.reference | 8 +++ .../02100_multiple_hosts_command_line_set.sh | 35 ++++++++++ 6 files changed, 92 insertions(+), 45 deletions(-) create mode 100644 tests/queries/0_stateless/02100_multiple_hosts_command_line_set.reference create mode 100755 tests/queries/0_stateless/02100_multiple_hosts_command_line_set.sh diff --git a/programs/client/Client.cpp b/programs/client/Client.cpp index 191cb5bd23c..1ab47472b22 100644 --- a/programs/client/Client.cpp +++ b/programs/client/Client.cpp @@ -1,17 +1,12 @@ #include #include -#include #include #include -#include #include -#include -#include #include #include #include #include -#include #include #include #include "Client.h" @@ -24,6 +19,7 @@ #include #include #include +#include #include #include @@ -33,13 +29,13 @@ #include #include #include -#include #include #include #include #include -#include +#include +#include #include #include @@ -77,12 +73,6 @@ void Client::processError(const String & query) const fmt::print(stderr, "Received exception from server (version {}):\n{}\n", server_version, getExceptionMessage(*server_exception, print_stack_trace, true)); - bool print_stack_trace = config().getBool("stacktrace", false); - fmt::print( - stderr, - "Received exception from server (version {}):\n{}\n", - server_version, - getExceptionMessage(*server_exception, print_stack_trace, true)); if (is_interactive) { fmt::print(stderr, "\n"); @@ -492,17 +482,23 @@ catch (...) void Client::connect() { + for (size_t attempted_address_index = 0; attempted_address_index < hosts_ports.size(); ++attempted_address_index) + { + DB::DNSResolver::instance().resolveHost(hosts_ports[attempted_address_index].host); + } bool is_secure = config().getBool("secure", false); - connection_parameters = ConnectionParameters(config()); + String tcp_port_config_key = is_secure ? "tcp_port_secure" : "tcp_port"; + UInt16 default_port = config().getInt("port", + config().getInt(tcp_port_config_key, + is_secure ? DBMS_DEFAULT_SECURE_PORT : DBMS_DEFAULT_PORT)); + connection_parameters = ConnectionParameters(config(), hosts_ports[0].host, + hosts_ports[0].port.value_or(default_port)); String server_name; UInt64 server_version_major = 0; UInt64 server_version_minor = 0; UInt64 server_version_patch = 0; - String tcp_port_config_key = is_secure ? "tcp_port_secure" : "tcp_port"; - UInt16 default_port = config().getInt(tcp_port_config_key, is_secure ? DBMS_DEFAULT_SECURE_PORT : DBMS_DEFAULT_PORT); - for (size_t attempted_address_index = 0; attempted_address_index < hosts_ports.size(); ++attempted_address_index) { connection_parameters.host = hosts_ports[attempted_address_index].host; @@ -527,6 +523,8 @@ void Client::connect() connection->getServerVersion( connection_parameters.timeouts, server_name, server_version_major, server_version_minor, server_version_patch, server_revision); + config().setString("host", connection_parameters.host); + config().setInt("port", connection_parameters.port); break; } catch (const Exception & e) @@ -549,14 +547,16 @@ void Client::connect() if (attempted_address_index == hosts_ports.size() - 1) throw; - std::cerr << "Connection attempt to database at " - << connection_parameters.host << ":" << connection_parameters.port - << " resulted in failure" - << std::endl - << getExceptionMessage(e, false) - << std::endl - << "Attempting connection to the next provided address" - << std::endl; + if (is_interactive) { + std::cerr << "Connection attempt to database at " + << connection_parameters.host << ":" << connection_parameters.port + << " resulted in failure" + << std::endl + << getExceptionMessage(e, false) + << std::endl + << "Attempting connection to the next provided address" + << std::endl; + } } } } @@ -1003,9 +1003,10 @@ void Client::addOptions(OptionsDescription & options_description) options_description.main_description->add_options() ("config,c", po::value(), "config-file path (another shorthand)") ("host,h", po::value>()->multitoken()->default_value({{"localhost"}}, "localhost"), - "list of server hosts with optionally assigned port to connect. Every argument looks like '[:] for example" - "'localhost:port'. If port isn't assigned, connection is made by port from '--port' param") - ("port", po::value()->default_value(9000), "server port") + "list of server hosts with optionally assigned port to connect. List elements are separated by a space." + "Every list element looks like '[:]'. If port isn't assigned, connection is made by port from '--port' param" + "Example of usage: '-h host1:1 host2, host3:3'") + ("port", po::value()->default_value(9000), "server port, which is default port for every host from '--host' param") ("secure,s", "Use TLS connection") ("user,u", po::value()->default_value("default"), "user") /** If "--password [value]" is used but the value is omitted, the bad argument exception will be thrown. @@ -1112,13 +1113,8 @@ void Client::processOptions(const OptionsDescription & options_description, if (options.count("config")) config().setString("config-file", options["config"].as()); - if (options.count("host") && !options["host"].defaulted()) - { + if (options.count("host")) hosts_ports = options["host"].as>(); - config().setString("host", hosts_ports[0].host); - if (hosts_ports[0].port.has_value()) - config().setInt("port", hosts_ports[0].port.value()); - } if (options.count("interleave-queries-file")) interleave_queries_files = options["interleave-queries-file"].as>(); if (options.count("port") && !options["port"].defaulted()) diff --git a/src/Client/ClientBase.h b/src/Client/ClientBase.h index 7345dd47ea3..b160417340f 100644 --- a/src/Client/ClientBase.h +++ b/src/Client/ClientBase.h @@ -249,7 +249,7 @@ protected: if (delimiter_pos != String::npos) { hostPort.host = host_with_port.substr(0, delimiter_pos); - hostPort.port = std::stoi(host_with_port.substr(delimiter_pos + 1, host_with_port.length())); + hostPort.port = boost::lexical_cast(host_with_port.substr(delimiter_pos + 1, host_with_port.length())); } else hostPort.host = host_with_port; diff --git a/src/Client/ConnectionParameters.cpp b/src/Client/ConnectionParameters.cpp index dbd463583f5..18da169fae8 100644 --- a/src/Client/ConnectionParameters.cpp +++ b/src/Client/ConnectionParameters.cpp @@ -23,15 +23,13 @@ namespace ErrorCodes extern const int BAD_ARGUMENTS; } -ConnectionParameters::ConnectionParameters(const Poco::Util::AbstractConfiguration & config) +ConnectionParameters::ConnectionParameters(const Poco::Util::AbstractConfiguration & config, + std::string connection_host, + int connection_port) : host(connection_host), port(connection_port) { bool is_secure = config.getBool("secure", false); security = is_secure ? Protocol::Secure::Enable : Protocol::Secure::Disable; - host = config.getString("host", "localhost"); - port = config.getInt( - "port", config.getInt(is_secure ? "tcp_port_secure" : "tcp_port", is_secure ? DBMS_DEFAULT_SECURE_PORT : DBMS_DEFAULT_PORT)); - default_database = config.getString("database", ""); /// changed the default value to "default" to fix the issue when the user in the prompt is blank @@ -61,12 +59,21 @@ ConnectionParameters::ConnectionParameters(const Poco::Util::AbstractConfigurati /// By default compression is disabled if address looks like localhost. compression = config.getBool("compression", !isLocalAddress(DNSResolver::instance().resolveHost(host))) - ? Protocol::Compression::Enable : Protocol::Compression::Disable; + ? Protocol::Compression::Enable : Protocol::Compression::Disable; timeouts = ConnectionTimeouts( - Poco::Timespan(config.getInt("connect_timeout", DBMS_DEFAULT_CONNECT_TIMEOUT_SEC), 0), - Poco::Timespan(config.getInt("send_timeout", DBMS_DEFAULT_SEND_TIMEOUT_SEC), 0), - Poco::Timespan(config.getInt("receive_timeout", DBMS_DEFAULT_RECEIVE_TIMEOUT_SEC), 0), - Poco::Timespan(config.getInt("tcp_keep_alive_timeout", 0), 0)); + Poco::Timespan(config.getInt("connect_timeout", DBMS_DEFAULT_CONNECT_TIMEOUT_SEC), 0), + Poco::Timespan(config.getInt("send_timeout", DBMS_DEFAULT_SEND_TIMEOUT_SEC), 0), + Poco::Timespan(config.getInt("receive_timeout", DBMS_DEFAULT_RECEIVE_TIMEOUT_SEC), 0), + Poco::Timespan(config.getInt("tcp_keep_alive_timeout", 0), 0)); +} + +ConnectionParameters::ConnectionParameters(const Poco::Util::AbstractConfiguration & config) +{ + bool is_secure = config.getBool("secure", false); + std::string connection_host = config.getString("host", "localhost"); + int connection_port = config.getInt("port", + config.getInt(is_secure ? "tcp_port_secure" : "tcp_port", is_secure ? DBMS_DEFAULT_SECURE_PORT : DBMS_DEFAULT_PORT)); + ConnectionParameters(config, connection_host, connection_port); } } diff --git a/src/Client/ConnectionParameters.h b/src/Client/ConnectionParameters.h index a169df8390a..28758ad36d8 100644 --- a/src/Client/ConnectionParameters.h +++ b/src/Client/ConnectionParameters.h @@ -24,6 +24,7 @@ struct ConnectionParameters ConnectionParameters() {} ConnectionParameters(const Poco::Util::AbstractConfiguration & config); + ConnectionParameters(const Poco::Util::AbstractConfiguration & config, std::string host, int port); }; } diff --git a/tests/queries/0_stateless/02100_multiple_hosts_command_line_set.reference b/tests/queries/0_stateless/02100_multiple_hosts_command_line_set.reference new file mode 100644 index 00000000000..c18b4e9b082 --- /dev/null +++ b/tests/queries/0_stateless/02100_multiple_hosts_command_line_set.reference @@ -0,0 +1,8 @@ +1 +1 +1 +1 +1 +1 +1 +1 diff --git a/tests/queries/0_stateless/02100_multiple_hosts_command_line_set.sh b/tests/queries/0_stateless/02100_multiple_hosts_command_line_set.sh new file mode 100755 index 00000000000..00ebbd78e3b --- /dev/null +++ b/tests/queries/0_stateless/02100_multiple_hosts_command_line_set.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + +# default values test +${CLICKHOUSE_CLIENT} --query "SELECT 1" + +# backward compatibility test +${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}" --port "${CLICKHOUSE_PORT_TCP}" --query "SELECT 1"; + +not_resolvable_host="notlocalhost" +exception_msg="Cannot resolve host (${not_resolvable_host}), error 0: ${not_resolvable_host}. +Code: 198. DB::Exception: Not found address of host: ${not_resolvable_host}. (DNS_ERROR) +" +error="$(${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}" "${not_resolvable_host}" --query "SELECT 1" 2>&1 > /dev/null)"; +[ "${error}" == "${exception_msg}" ]; echo "$?" + +not_number_port="abc" +exception_msg="Bad arguments: the argument ('${CLICKHOUSE_HOST}:${not_number_port}') for option '--host' is invalid." +error="$(${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}:${not_number_port}" --query "SELECT 1" 2>&1 > /dev/null)"; +[ "${error}" == "${exception_msg}" ]; echo "$?" + +not_alive_host="10.100.0.0" +${CLICKHOUSE_CLIENT} --host "${not_alive_host}" "${CLICKHOUSE_HOST}" --query "SELECT 1"; + +not_alive_port="1" +exception_msg="Code: 210. DB::NetException: Connection refused (${CLICKHOUSE_HOST}:${not_alive_port}). (NETWORK_ERROR) +" +error="$(${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}" --port "${not_alive_port}" --query "SELECT 1" 2>&1 > /dev/null)" +[ "${error}" == "${exception_msg}" ]; echo "$?" +${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}:${not_alive_port}" "${CLICKHOUSE_HOST}" --query "SELECT 1"; +${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}:${CLICKHOUSE_PORT_TCP}" --port "${not_alive_port}" --query "SELECT 1"; +