Merge pull request #9026 from ClickHouse/fix_poco_ssl_timeouts

Fix poco secure sockets timeouts

contrib/poco

@@ -1 +1 @@
-Subproject commit d478f62bd93c9cd14eb343756ef73a4ae622ddf5
+Subproject commit d805cf5ca4cf8bdc642261cfcbe7a0a241cb7298

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();

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<bool>(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);

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).

dbms/src/Core/Settings.h

@@ -62,6 +62,7 @@ struct Settings : public SettingsCollection<Settings>
     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) \

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)

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:

dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/config.d/remote_servers.xml

@@ -0,0 +1,21 @@
+<yandex>
+    <tcp_port_secure>9440</tcp_port_secure>
+    <remote_servers>
+        <test_cluster>
+            <shard>
+                <replica>
+                    <host>node1</host>
+                    <port>9440</port>
+                    <secure>1</secure>
+                </replica>
+            </shard>
+            <shard>
+                <replica>
+                    <host>node2</host>
+                    <port>9440</port>
+                    <secure>1</secure>
+                </replica>
+            </shard>
+        </test_cluster>
+    </remote_servers>
+</yandex>

dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/config.d/ssl_conf.xml

@@ -0,0 +1,17 @@
+<yandex>
+    <openSSL>
+        <server>
+            <certificateFile>/etc/clickhouse-server/server.crt</certificateFile>
+            <privateKeyFile>/etc/clickhouse-server/server.key</privateKeyFile>
+            <verificationMode>none</verificationMode>
+            <cacheSessions>true</cacheSessions>
+        </server>
+        <client>
+            <cacheSessions>true</cacheSessions>
+            <verificationMode>none</verificationMode>
+            <invalidCertificateHandler>
+                <name>AcceptCertificateHandler</name>
+            </invalidCertificateHandler>
+        </client>
+    </openSSL>
+</yandex>

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-----

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-----

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-----

dbms/tests/integration/test_distributed_respect_user_timeouts/configs_secure/users.d/set_distributed_defaults.xml

@@ -0,0 +1,35 @@
+<yandex>
+    <profiles>
+        <default>
+            <connections_with_failover_max_tries>3</connections_with_failover_max_tries>
+            <connect_timeout_with_failover_secure_ms>1000</connect_timeout_with_failover_secure_ms>
+            <min_insert_block_size_rows>1</min_insert_block_size_rows>
+        </default>
+        <delays>
+            <connections_with_failover_max_tries>5</connections_with_failover_max_tries>
+            <connect_timeout_with_failover_secure_ms>3000</connect_timeout_with_failover_secure_ms>
+            <min_insert_block_size_rows>1</min_insert_block_size_rows>
+        </delays>
+    </profiles>
+
+    <users>
+        <default>
+            <password></password>
+            <networks incl="networks" replace="replace">
+                <ip>::/0</ip>
+            </networks>
+            <profile>default</profile>
+            <quota>default</quota>
+        </default>
+        <ready_to_wait>
+            <password></password>
+            <networks incl="networks" replace="replace">
+                <ip>::/0</ip>
+            </networks>
+            <profile>delays</profile>
+            <quota>default</quota>
+        </ready_to_wait>
+    </users>
+
+    <quotas><default></default></quotas>
+</yandex>

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(
+NODES = {'node' + str(i): None for i in (1, 2)}
-    'node' + str(i),
-    main_configs=['configs/remote_servers.xml'],
-    user_configs=['configs/set_distributed_defaults.xml'],
-) 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")
+@pytest.fixture(scope="module", params=["configs", "configs_secure"])
-def started_cluster():
+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')