From 015695b3bf354c3b8190db22563b9f330796382f Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <a3at.mail@gmail.com>
Date: Thu, 23 Sep 2021 08:58:19 +0300
Subject: [PATCH] Fix connection timeouts (send_timeout/receive_timeout)

Query is not executed within sendQuery() function.

INSERT query consist from multiple parts:
- sendQuery()
- write()/writePrepared() -- send Data blocks
- writeSuffix() -- send empty Data block and receive EndOfStream
(see RemoteBlockOutputStream for more info)
So as you can see it is not executed completely from the sendQuery() function.

SELECT query (and others) also send Data blocks to the client after.

And what this means that temporary timeout (via TimeoutSetter) is not
enough, since next query can use timeout from the previous query.

Usually you do not see it, because:
- You don't use custom send_timeout/receive_timeout
- SELECT is fast enough

However it is not the case with INSERT, especially with
insert_distributed_sync=1, since
DistributedBlockOutputStream::writeSuffix() may stack because remote
server still INSERT'ing the data (i.e. creating znodes in zookeeper for
ReplicatedMergeTree, syncing blocks, and similar) that was issued from
DistributedBlockOutputStream::write(), and it will lead to the following
exception:

    Code: 209, e.displayText() = DB::NetException: Timeout exceeded while reading from socket (127.1:9000): while receiving packet from ch-47-drt.dpa.semrush.net:9000: Insertion status:
    Wrote X blocks and Y rows on shard Z replica 0, 127.1:9000 (average 0 ms per block, the slowest block N ms)
    0. DB::Exception::Exception()
    1. DB::ReadBufferFromPocoSocket::nextImpl()
    2. void DB::readVarUIntImpl<false>(unsigned long&, DB::ReadBuffer&)
    3. DB::Connection::receivePacket()
    4. DB::RemoteBlockOutputStream::writeSuffix()
    ...
---
 src/Client/Connection.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/Client/Connection.cpp b/src/Client/Connection.cpp
index fd02bde57bc..dffac97d8ce 100644
--- a/src/Client/Connection.cpp
+++ b/src/Client/Connection.cpp
@@ -419,7 +419,12 @@ void Connection::sendQuery(
     if (!connected)
         connect(timeouts);
 
-    TimeoutSetter timeout_setter(*socket, timeouts.send_timeout, timeouts.receive_timeout, true);
+    /// Query is not executed within sendQuery() function.
+    ///
+    /// And what this means that temporary timeout (via TimeoutSetter) is not
+    /// enough, since next query can use timeout from the previous query in this case.
+    socket->setReceiveTimeout(timeouts.receive_timeout);
+    socket->setSendTimeout(timeouts.send_timeout);
 
     if (settings)
     {