From beca3b98341e9cf52a9a9840b3f216a67b454e29 Mon Sep 17 00:00:00 2001
From: Igor Nikonov <igor@clickhouse.com>
Date: Tue, 9 Jul 2024 19:31:33 +0000
Subject: [PATCH] Proper placing for DistributedConnectionFailTry

+ better logs
---
 src/Client/ConnectionEstablisher.cpp      | 3 +++
 src/Client/HedgedConnectionsFactory.cpp   | 2 --
 src/Common/PoolWithFailoverBase.h         | 2 --
 src/QueryPipeline/RemoteQueryExecutor.cpp | 6 +++++-
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/Client/ConnectionEstablisher.cpp b/src/Client/ConnectionEstablisher.cpp
index 303105751ad..05839b44452 100644
--- a/src/Client/ConnectionEstablisher.cpp
+++ b/src/Client/ConnectionEstablisher.cpp
@@ -8,6 +8,7 @@ namespace ProfileEvents
     extern const Event DistributedConnectionUsable;
     extern const Event DistributedConnectionMissingTable;
     extern const Event DistributedConnectionStaleReplica;
+    extern const Event DistributedConnectionFailTry;
 }
 
 namespace DB
@@ -97,6 +98,8 @@ void ConnectionEstablisher::run(ConnectionEstablisher::TryResult & result, std::
     }
     catch (const Exception & e)
     {
+        ProfileEvents::increment(ProfileEvents::DistributedConnectionFailTry);
+
         if (e.code() != ErrorCodes::NETWORK_ERROR && e.code() != ErrorCodes::SOCKET_TIMEOUT
             && e.code() != ErrorCodes::ATTEMPT_TO_READ_AFTER_EOF && e.code() != ErrorCodes::DNS_ERROR)
             throw;
diff --git a/src/Client/HedgedConnectionsFactory.cpp b/src/Client/HedgedConnectionsFactory.cpp
index 0fa2bc12924..be7397b0fad 100644
--- a/src/Client/HedgedConnectionsFactory.cpp
+++ b/src/Client/HedgedConnectionsFactory.cpp
@@ -7,7 +7,6 @@
 namespace ProfileEvents
 {
     extern const Event HedgedRequestsChangeReplica;
-    extern const Event DistributedConnectionFailTry;
     extern const Event DistributedConnectionFailAtAll;
 }
 
@@ -327,7 +326,6 @@ HedgedConnectionsFactory::State HedgedConnectionsFactory::processFinishedConnect
     {
         ShuffledPool & shuffled_pool = shuffled_pools[index];
         LOG_INFO(log, "Connection failed at try №{}, reason: {}", (shuffled_pool.error_count + 1), fail_message);
-        ProfileEvents::increment(ProfileEvents::DistributedConnectionFailTry);
 
         shuffled_pool.error_count = std::min(pool->getMaxErrorCup(), shuffled_pool.error_count + 1);
         shuffled_pool.slowdown_count = 0;
diff --git a/src/Common/PoolWithFailoverBase.h b/src/Common/PoolWithFailoverBase.h
index 2359137012c..3d4de773a36 100644
--- a/src/Common/PoolWithFailoverBase.h
+++ b/src/Common/PoolWithFailoverBase.h
@@ -28,7 +28,6 @@ namespace ErrorCodes
 
 namespace ProfileEvents
 {
-    extern const Event DistributedConnectionFailTry;
     extern const Event DistributedConnectionFailAtAll;
     extern const Event DistributedConnectionSkipReadOnlyReplica;
 }
@@ -285,7 +284,6 @@ PoolWithFailoverBase<TNestedPool>::getMany(
             else
             {
                 LOG_WARNING(log, "Connection failed at try №{}, reason: {}", (shuffled_pool.error_count + 1), fail_message);
-                ProfileEvents::increment(ProfileEvents::DistributedConnectionFailTry);
 
                 shuffled_pool.error_count = std::min(max_error_cap, shuffled_pool.error_count + 1);
 
diff --git a/src/QueryPipeline/RemoteQueryExecutor.cpp b/src/QueryPipeline/RemoteQueryExecutor.cpp
index 61a6bf82270..14457d2df43 100644
--- a/src/QueryPipeline/RemoteQueryExecutor.cpp
+++ b/src/QueryPipeline/RemoteQueryExecutor.cpp
@@ -106,7 +106,11 @@ RemoteQueryExecutor::RemoteQueryExecutor(
         }
         else
         {
-            LOG_DEBUG(log, "Failed to connect to replica {}. {}", pool->getAddress(), fail_message);
+            chassert(!fail_message.empty());
+            if (result.entry.isNull())
+                LOG_DEBUG(log, "Failed to connect to replica {}. {}", pool->getAddress(), fail_message);
+            else
+                LOG_DEBUG(log, "Replica is not usable for remote query execution: {}. {}", pool->getAddress(), fail_message);
         }
 
         auto res = std::make_unique<MultiplexedConnections>(std::move(connection_entries), context, throttler);