diff --git a/contrib/poco b/contrib/poco index d478f62bd93..d805cf5ca4c 160000 --- a/contrib/poco +++ b/contrib/poco @@ -1 +1 @@ -Subproject commit d478f62bd93c9cd14eb343756ef73a4ae622ddf5 +Subproject commit d805cf5ca4cf8bdc642261cfcbe7a0a241cb7298 diff --git a/dbms/programs/server/HTTPHandler.cpp b/dbms/programs/server/HTTPHandler.cpp index d28b4aee104..3708b306b00 100644 --- a/dbms/programs/server/HTTPHandler.cpp +++ b/dbms/programs/server/HTTPHandler.cpp @@ -575,6 +575,7 @@ void HTTPHandler::processQuery( try { char b; + //FIXME looks like MSG_DONTWAIT is useless because of POCO_BROKEN_TIMEOUTS int status = socket.receiveBytes(&b, 1, MSG_DONTWAIT | MSG_PEEK); if (status == 0) context.killCurrentQuery(); diff --git a/dbms/src/Client/Connection.cpp b/dbms/src/Client/Connection.cpp index b3f1e341f80..065a02f3ebe 100644 --- a/dbms/src/Client/Connection.cpp +++ b/dbms/src/Client/Connection.cpp @@ -74,7 +74,8 @@ void Connection::connect(const ConnectionTimeouts & timeouts) current_resolved_address = DNSResolver::instance().resolveAddress(host, port); - socket->connect(*current_resolved_address, timeouts.connection_timeout); + const auto & connection_timeout = static_cast(secure) ? timeouts.secure_connection_timeout : timeouts.connection_timeout; + socket->connect(*current_resolved_address, connection_timeout); socket->setReceiveTimeout(timeouts.receive_timeout); socket->setSendTimeout(timeouts.send_timeout); socket->setNoDelay(true); diff --git a/dbms/src/Core/Defines.h b/dbms/src/Core/Defines.h index 0596a99445a..ce3a8122ead 100644 --- a/dbms/src/Core/Defines.h +++ b/dbms/src/Core/Defines.h @@ -6,6 +6,7 @@ #define DBMS_DEFAULT_HTTP_PORT 8123 #define DBMS_DEFAULT_CONNECT_TIMEOUT_SEC 10 #define DBMS_DEFAULT_CONNECT_TIMEOUT_WITH_FAILOVER_MS 50 +#define DBMS_DEFAULT_CONNECT_TIMEOUT_WITH_FAILOVER_SECURE_MS 100 #define DBMS_DEFAULT_SEND_TIMEOUT_SEC 300 #define DBMS_DEFAULT_RECEIVE_TIMEOUT_SEC 300 /// Timeout for synchronous request-result protocol call (like Ping or TablesStatus). diff --git a/dbms/src/Core/Settings.h b/dbms/src/Core/Settings.h index b330723e14f..524edf8e635 100644 --- a/dbms/src/Core/Settings.h +++ b/dbms/src/Core/Settings.h @@ -62,6 +62,7 @@ struct Settings : public SettingsCollection M(SettingUInt64, interactive_delay, 100000, "The interval in microseconds to check if the request is cancelled, and to send progress info.", 0) \ M(SettingSeconds, connect_timeout, DBMS_DEFAULT_CONNECT_TIMEOUT_SEC, "Connection timeout if there are no replicas.", 0) \ M(SettingMilliseconds, connect_timeout_with_failover_ms, DBMS_DEFAULT_CONNECT_TIMEOUT_WITH_FAILOVER_MS, "Connection timeout for selecting first healthy replica.", 0) \ + M(SettingMilliseconds, connect_timeout_with_failover_secure_ms, DBMS_DEFAULT_CONNECT_TIMEOUT_WITH_FAILOVER_SECURE_MS, "Connection timeout for selecting first healthy replica (for secure connections).", 0) \ M(SettingSeconds, receive_timeout, DBMS_DEFAULT_RECEIVE_TIMEOUT_SEC, "", 0) \ M(SettingSeconds, send_timeout, DBMS_DEFAULT_SEND_TIMEOUT_SEC, "", 0) \ M(SettingSeconds, tcp_keep_alive_timeout, 0, "The time in seconds the connection needs to remain idle before TCP starts sending keepalive probes", 0) \ diff --git a/dbms/src/IO/ConnectionTimeouts.h b/dbms/src/IO/ConnectionTimeouts.h index e9f9643943a..9e87dee4fc3 100644 --- a/dbms/src/IO/ConnectionTimeouts.h +++ b/dbms/src/IO/ConnectionTimeouts.h @@ -16,6 +16,7 @@ struct ConnectionTimeouts Poco::Timespan receive_timeout; Poco::Timespan tcp_keep_alive_timeout; Poco::Timespan http_keep_alive_timeout; + Poco::Timespan secure_connection_timeout; ConnectionTimeouts() = default; @@ -26,7 +27,8 @@ struct ConnectionTimeouts send_timeout(send_timeout_), receive_timeout(receive_timeout_), tcp_keep_alive_timeout(0), - http_keep_alive_timeout(0) + http_keep_alive_timeout(0), + secure_connection_timeout(connection_timeout) { } @@ -38,7 +40,8 @@ struct ConnectionTimeouts send_timeout(send_timeout_), receive_timeout(receive_timeout_), tcp_keep_alive_timeout(tcp_keep_alive_timeout_), - http_keep_alive_timeout(0) + http_keep_alive_timeout(0), + secure_connection_timeout(connection_timeout) { } ConnectionTimeouts(const Poco::Timespan & connection_timeout_, @@ -50,10 +53,25 @@ struct ConnectionTimeouts send_timeout(send_timeout_), receive_timeout(receive_timeout_), tcp_keep_alive_timeout(tcp_keep_alive_timeout_), - http_keep_alive_timeout(http_keep_alive_timeout_) + http_keep_alive_timeout(http_keep_alive_timeout_), + secure_connection_timeout(connection_timeout) { } + ConnectionTimeouts(const Poco::Timespan & connection_timeout_, + const Poco::Timespan & send_timeout_, + const Poco::Timespan & receive_timeout_, + const Poco::Timespan & tcp_keep_alive_timeout_, + const Poco::Timespan & http_keep_alive_timeout_, + const Poco::Timespan & secure_connection_timeout_) + : connection_timeout(connection_timeout_), + send_timeout(send_timeout_), + receive_timeout(receive_timeout_), + tcp_keep_alive_timeout(tcp_keep_alive_timeout_), + http_keep_alive_timeout(http_keep_alive_timeout_), + secure_connection_timeout(secure_connection_timeout_) + { + } static Poco::Timespan saturate(const Poco::Timespan & timespan, const Poco::Timespan & limit) { @@ -69,7 +87,8 @@ struct ConnectionTimeouts saturate(send_timeout, limit), saturate(receive_timeout, limit), saturate(tcp_keep_alive_timeout, limit), - saturate(http_keep_alive_timeout, limit)); + saturate(http_keep_alive_timeout, limit), + saturate(secure_connection_timeout, limit)); } /// Timeouts for the case when we have just single attempt to connect. @@ -81,7 +100,7 @@ struct ConnectionTimeouts /// Timeouts for the case when we will try many addresses in a loop. static ConnectionTimeouts getTCPTimeoutsWithFailover(const Settings & settings) { - return ConnectionTimeouts(settings.connect_timeout_with_failover_ms, settings.send_timeout, settings.receive_timeout, settings.tcp_keep_alive_timeout); + return ConnectionTimeouts(settings.connect_timeout_with_failover_ms, settings.send_timeout, settings.receive_timeout, settings.tcp_keep_alive_timeout, 0, settings.connect_timeout_with_failover_secure_ms); } static ConnectionTimeouts getHTTPTimeouts(const Context & context) diff --git a/dbms/tests/integration/test_distributed_ddl/test.py b/dbms/tests/integration/test_distributed_ddl/test.py index e30880e6ea4..f8d07826990 100755 --- a/dbms/tests/integration/test_distributed_ddl/test.py +++ b/dbms/tests/integration/test_distributed_ddl/test.py @@ -289,6 +289,11 @@ def test_rename(test_cluster): assert instance.query("select count(id), sum(id) from rename where sid like 'new%'").rstrip() == "10\t245" test_cluster.pm_random_drops.push_rules(rules) +def test_socket_timeout(test_cluster): + instance = test_cluster.instances['ch1'] + # queries should not fail with "Timeout exceeded while reading from socket" in case of EINTR caused by query profiler + for i in range(0, 100): + instance.query("select hostName() as host, count() from cluster('cluster', 'system', 'settings') group by host") if __name__ == '__main__': with contextmanager(test_cluster)() as ctx_cluster: diff --git a/dbms/tests/integration/test_distributed_respect_user_timeouts/configs/remote_servers.xml b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs/config.d/remote_servers.xml similarity index 100% rename from dbms/tests/integration/test_distributed_respect_user_timeouts/configs/remote_servers.xml rename to dbms/tests/integration/test_distributed_respect_user_timeouts/configs/config.d/remote_servers.xml diff --git a/dbms/tests/integration/test_distributed_respect_user_timeouts/configs/set_distributed_defaults.xml b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs/users.d/set_distributed_defaults.xml similarity index 100% rename from dbms/tests/integration/test_distributed_respect_user_timeouts/configs/set_distributed_defaults.xml rename to dbms/tests/integration/test_distributed_respect_user_timeouts/configs/users.d/set_distributed_defaults.xml diff --git a/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/config.d/remote_servers.xml b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/config.d/remote_servers.xml new file mode 100644 index 00000000000..caafbded647 --- /dev/null +++ b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/config.d/remote_servers.xml @@ -0,0 +1,21 @@ + + 9440 + + + + + node1 + 9440 + 1 + + + + + node2 + 9440 + 1 + + + + + diff --git a/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/config.d/ssl_conf.xml b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/config.d/ssl_conf.xml new file mode 100644 index 00000000000..696695ddc69 --- /dev/null +++ b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/config.d/ssl_conf.xml @@ -0,0 +1,17 @@ + + + + /etc/clickhouse-server/server.crt + /etc/clickhouse-server/server.key + none + true + + + true + none + + AcceptCertificateHandler + + + + diff --git a/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/dhparam.pem b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/dhparam.pem new file mode 100644 index 00000000000..2e6cee0798d --- /dev/null +++ b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/dhparam.pem @@ -0,0 +1,8 @@ +-----BEGIN DH PARAMETERS----- +MIIBCAKCAQEAua92DDli13gJ+//ZXyGaggjIuidqB0crXfhUlsrBk9BV1hH3i7fR +XGP9rUdk2ubnB3k2ejBStL5oBrkHm9SzUFSQHqfDjLZjKoUpOEmuDc4cHvX1XTR5 +Pr1vf5cd0yEncJWG5W4zyUB8k++SUdL2qaeslSs+f491HBLDYn/h8zCgRbBvxhxb +9qeho1xcbnWeqkN6Kc9bgGozA16P9NLuuLttNnOblkH+lMBf42BSne/TWt3AlGZf +slKmmZcySUhF8aKfJnLKbkBCFqOtFRh8zBA9a7g+BT/lSANATCDPaAk1YVih2EKb +dpc3briTDbRsiqg2JKMI7+VdULY9bh3EawIBAg== +-----END DH PARAMETERS----- diff --git a/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/server.crt b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/server.crt new file mode 100644 index 00000000000..7ade2d96273 --- /dev/null +++ b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/server.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIC/TCCAeWgAwIBAgIJANjx1QSR77HBMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNV +BAMMCWxvY2FsaG9zdDAgFw0xODA3MzAxODE2MDhaGA8yMjkyMDUxNDE4MTYwOFow +FDESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB +CgKCAQEAs9uSo6lJG8o8pw0fbVGVu0tPOljSWcVSXH9uiJBwlZLQnhN4SFSFohfI +4K8U1tBDTnxPLUo/V1K9yzoLiRDGMkwVj6+4+hE2udS2ePTQv5oaMeJ9wrs+5c9T +4pOtlq3pLAdm04ZMB1nbrEysceVudHRkQbGHzHp6VG29Fw7Ga6YpqyHQihRmEkTU +7UCYNA+Vk7aDPdMS/khweyTpXYZimaK9f0ECU3/VOeG3fH6Sp2X6FN4tUj/aFXEj +sRmU5G2TlYiSIUMF2JPdhSihfk1hJVALrHPTU38SOL+GyyBRWdNcrIwVwbpvsvPg +pryMSNxnpr0AK0dFhjwnupIv5hJIOQIDAQABo1AwTjAdBgNVHQ4EFgQUjPLb3uYC +kcamyZHK4/EV8jAP0wQwHwYDVR0jBBgwFoAUjPLb3uYCkcamyZHK4/EV8jAP0wQw +DAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAM/ocuDvfPus/KpMVD51j +4IdlU8R0vmnYLQ+ygzOAo7+hUWP5j0yvq4ILWNmQX6HNvUggCgFv9bjwDFhb/5Vr +85ieWfTd9+LTjrOzTw4avdGwpX9G+6jJJSSq15tw5ElOIFb/qNA9O4dBiu8vn03C +L/zRSXrARhSqTW5w/tZkUcSTT+M5h28+Lgn9ysx4Ff5vi44LJ1NnrbJbEAIYsAAD ++UA+4MBFKx1r6hHINULev8+lCfkpwIaeS8RL+op4fr6kQPxnULw8wT8gkuc8I4+L +P9gg/xDHB44T3ADGZ5Ib6O0DJaNiToO6rnoaaxs0KkotbvDWvRoxEytSbXKoYjYp +0g== +-----END CERTIFICATE----- diff --git a/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/server.key b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/server.key new file mode 100644 index 00000000000..f0fb61ac443 --- /dev/null +++ b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/server.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCz25KjqUkbyjyn +DR9tUZW7S086WNJZxVJcf26IkHCVktCeE3hIVIWiF8jgrxTW0ENOfE8tSj9XUr3L +OguJEMYyTBWPr7j6ETa51LZ49NC/mhox4n3Cuz7lz1Pik62WreksB2bThkwHWdus +TKxx5W50dGRBsYfMenpUbb0XDsZrpimrIdCKFGYSRNTtQJg0D5WTtoM90xL+SHB7 +JOldhmKZor1/QQJTf9U54bd8fpKnZfoU3i1SP9oVcSOxGZTkbZOViJIhQwXYk92F +KKF+TWElUAusc9NTfxI4v4bLIFFZ01ysjBXBum+y8+CmvIxI3GemvQArR0WGPCe6 +ki/mEkg5AgMBAAECggEATrbIBIxwDJOD2/BoUqWkDCY3dGevF8697vFuZKIiQ7PP +TX9j4vPq0DfsmDjHvAPFkTHiTQXzlroFik3LAp+uvhCCVzImmHq0IrwvZ9xtB43f +7Pkc5P6h1l3Ybo8HJ6zRIY3TuLtLxuPSuiOMTQSGRL0zq3SQ5DKuGwkz+kVjHXUN +MR2TECFwMHKQ5VLrC+7PMpsJYyOMlDAWhRfUalxC55xOXTpaN8TxNnwQ8K2ISVY5 +212Jz/a4hn4LdwxSz3Tiu95PN072K87HLWx3EdT6vW4Ge5P/A3y+smIuNAlanMnu +plHBRtpATLiTxZt/n6npyrfQVbYjSH7KWhB8hBHtaQKBgQDh9Cq1c/KtqDtE0Ccr +/r9tZNTUwBE6VP+3OJeKdEdtsfuxjOCkS1oAjgBJiSDOiWPh1DdoDeVZjPKq6pIu +Mq12OE3Doa8znfCXGbkSzEKOb2unKZMJxzrz99kXt40W5DtrqKPNb24CNqTiY8Aa +CjtcX+3weat82VRXvph6U8ltMwKBgQDLxjiQQzNoY7qvg7CwJCjf9qq8jmLK766g +1FHXopqS+dTxDLM8eJSRrpmxGWJvNeNc1uPhsKsKgotqAMdBUQTf7rSTbt4MyoH5 +bUcRLtr+0QTK9hDWMOOvleqNXha68vATkohWYfCueNsC60qD44o8RZAS6UNy3ENq +cM1cxqe84wKBgQDKkHutWnooJtajlTxY27O/nZKT/HA1bDgniMuKaz4R4Gr1PIez +on3YW3V0d0P7BP6PWRIm7bY79vkiMtLEKdiKUGWeyZdo3eHvhDb/3DCawtau8L2K +GZsHVp2//mS1Lfz7Qh8/L/NedqCQ+L4iWiPnZ3THjjwn3CoZ05ucpvrAMwKBgB54 +nay039MUVq44Owub3KDg+dcIU62U+cAC/9oG7qZbxYPmKkc4oL7IJSNecGHA5SbU +2268RFdl/gLz6tfRjbEOuOHzCjFPdvAdbysanpTMHLNc6FefJ+zxtgk9sJh0C4Jh +vxFrw9nTKKzfEl12gQ1SOaEaUIO0fEBGbe8ZpauRAoGAMAlGV+2/K4ebvAJKOVTa +dKAzQ+TD2SJmeR1HZmKDYddNqwtZlzg3v4ZhCk4eaUmGeC1Bdh8MDuB3QQvXz4Dr +vOIP4UVaOr+uM+7TgAgVnP4/K6IeJGzUDhX93pmpWhODfdu/oojEKVcpCojmEmS1 +KCBtmIrQLqzMpnBpLNuSY+Q= +-----END PRIVATE KEY----- diff --git a/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/users.d/set_distributed_defaults.xml b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/users.d/set_distributed_defaults.xml new file mode 100644 index 00000000000..2fa19d41b76 --- /dev/null +++ b/dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/users.d/set_distributed_defaults.xml @@ -0,0 +1,35 @@ + + + + 3 + 1000 + 1 + + + 5 + 3000 + 1 + + + + + + + + ::/0 + + default + default + + + + + ::/0 + + delays + default + + + + + diff --git a/dbms/tests/integration/test_distributed_respect_user_timeouts/test.py b/dbms/tests/integration/test_distributed_respect_user_timeouts/test.py index bbab53edeba..72c3001ee91 100644 --- a/dbms/tests/integration/test_distributed_respect_user_timeouts/test.py +++ b/dbms/tests/integration/test_distributed_respect_user_timeouts/test.py @@ -10,11 +10,7 @@ from helpers.test_tools import TSV cluster = ClickHouseCluster(__file__) -NODES = {'node' + str(i): cluster.add_instance( - 'node' + str(i), - main_configs=['configs/remote_servers.xml'], - user_configs=['configs/set_distributed_defaults.xml'], -) for i in (1, 2)} +NODES = {'node' + str(i): None for i in (1, 2)} CREATE_TABLES_SQL = ''' CREATE DATABASE test; @@ -55,6 +51,16 @@ EXPECTED_BEHAVIOR = { }, } +TIMEOUT_DIFF_UPPER_BOUND = { + 'default': { + 'distributed': 5.5, + 'remote': 2.5, + }, + 'ready_to_wait': { + 'distributed': 3, + 'remote': 1.5, + }, +} def _check_exception(exception, expected_tries=3): lines = exception.split('\n') @@ -80,8 +86,13 @@ def _check_exception(exception, expected_tries=3): assert lines[3 + expected_tries] == '', 'Wrong number of connect attempts' -@pytest.fixture(scope="module") -def started_cluster(): +@pytest.fixture(scope="module", params=["configs", "configs_secure"]) +def started_cluster(request): + + cluster = ClickHouseCluster(__file__) + cluster.__with_ssl_config = request.param == "configs_secure" + for name in NODES: + NODES[name] = cluster.add_instance(name, config_dir=request.param) try: cluster.start() @@ -95,17 +106,18 @@ def started_cluster(): cluster.shutdown() -def _check_timeout_and_exception(node, user, query_base): +def _check_timeout_and_exception(node, user, query_base, query): repeats = EXPECTED_BEHAVIOR[user]['times'] expected_timeout = EXPECTED_BEHAVIOR[user]['timeout'] * repeats start = timeit.default_timer() - exception = node.query_and_get_error(SELECTS_SQL[query_base], user=user) + exception = node.query_and_get_error(query, user=user) # And it should timeout no faster than: measured_timeout = timeit.default_timer() - start - assert measured_timeout >= expected_timeout - TIMEOUT_MEASUREMENT_EPS + assert expected_timeout - measured_timeout <= TIMEOUT_MEASUREMENT_EPS + assert measured_timeout - expected_timeout <= TIMEOUT_DIFF_UPPER_BOUND[user][query_base] # And exception should reflect connection attempts: _check_exception(exception, repeats) @@ -117,9 +129,12 @@ def _check_timeout_and_exception(node, user, query_base): ) def test_reconnect(started_cluster, node_name, first_user, query_base): node = NODES[node_name] + query = SELECTS_SQL[query_base] + if started_cluster.__with_ssl_config: + query = query.replace('remote(', 'remoteSecure(') # Everything is up, select should work: - assert TSV(node.query(SELECTS_SQL[query_base], + assert TSV(node.query(query, user=first_user)) == TSV('node1\nnode2') with PartitionManager() as pm: @@ -127,15 +142,16 @@ def test_reconnect(started_cluster, node_name, first_user, query_base): pm.partition_instances(*NODES.values()) # Now it shouldn't: - _check_timeout_and_exception(node, first_user, query_base) + _check_timeout_and_exception(node, first_user, query_base, query) # Other user should have different timeout and exception _check_timeout_and_exception( node, 'default' if first_user != 'default' else 'ready_to_wait', query_base, + query, ) # select should work again: - assert TSV(node.query(SELECTS_SQL[query_base], + assert TSV(node.query(query, user=first_user)) == TSV('node1\nnode2')