Merge branch 'master' into optimize-in-single-value

.clang-tidy

@@ -96,7 +96,6 @@ Checks: [
   '-modernize-use-default-member-init',
   '-modernize-use-emplace',
   '-modernize-use-nodiscard',
-  '-modernize-use-override',
   '-modernize-use-trailing-return-type',
 
   '-performance-inefficient-string-concatenation',
@@ -120,7 +119,6 @@ Checks: [
   '-readability-named-parameter',
   '-readability-redundant-declaration',
   '-readability-simplify-boolean-expr',
-  '-readability-static-accessed-through-instance',
   '-readability-suspicious-call-argument',
   '-readability-uppercase-literal-suffix',
   '-readability-use-anyofallof',

.github/workflows/master.yml

@@ -23,6 +23,10 @@ jobs:
           clear-repository: true # to ensure correct digests
           fetch-depth: 0 # to get version
           filter: tree:0
+      - name: Check sync PR
+        run: |
+          cd "$GITHUB_WORKSPACE/tests/ci"
+          python3 sync_pr.py || :
       - name: Python unit tests
         run: |
           cd "$GITHUB_WORKSPACE/tests/ci"

.github/workflows/pull_request.yml

@@ -157,7 +157,7 @@ jobs:
   ################################# Stage Final #################################
   #
   FinishCheck:
-    if: ${{ !failure() && !cancelled() }}
+    if: ${{ !failure() && !cancelled() && github.event_name != 'merge_group' }}
     needs: [Tests_1, Tests_2]
     runs-on: [self-hosted, style-checker]
     steps:

CHANGELOG.md

@@ -123,7 +123,6 @@
 * Something was wrong with Apache Hive, which is experimental and not supported. [#60262](https://github.com/ClickHouse/ClickHouse/pull/60262) ([shanfengp](https://github.com/Aed-p)).
 * An improvement for experimental parallel replicas: force reanalysis if parallel replicas changed [#60362](https://github.com/ClickHouse/ClickHouse/pull/60362) ([Raúl Marín](https://github.com/Algunenano)).
 * Fix usage of plain metadata type with new disks configuration option [#60396](https://github.com/ClickHouse/ClickHouse/pull/60396) ([Kseniia Sumarokova](https://github.com/kssenii)).
-* Don't allow to set max_parallel_replicas to 0 as it doesn't make sense [#60430](https://github.com/ClickHouse/ClickHouse/pull/60430) ([Kruglov Pavel](https://github.com/Avogar)).
 * Try to fix logical error 'Cannot capture column because it has incompatible type' in mapContainsKeyLike [#60451](https://github.com/ClickHouse/ClickHouse/pull/60451) ([Kruglov Pavel](https://github.com/Avogar)).
 * Avoid calculation of scalar subqueries for CREATE TABLE. [#60464](https://github.com/ClickHouse/ClickHouse/pull/60464) ([Nikolai Kochetov](https://github.com/KochetovNicolai)).
 * Fix deadlock in parallel parsing when lots of rows are skipped due to errors [#60516](https://github.com/ClickHouse/ClickHouse/pull/60516) ([Kruglov Pavel](https://github.com/Avogar)).

base/base/scope_guard.h

@@ -29,11 +29,13 @@ class [[nodiscard]] BasicScopeGuard
     requires std::is_convertible_v<G, F>
     constexpr BasicScopeGuard & operator=(BasicScopeGuard<G> && src) // NOLINT(cppcoreguidelines-rvalue-reference-param-not-moved, cppcoreguidelines-noexcept-move-operations)
     {
-        if (this != &src)
+        if constexpr (std::is_same_v<G, F>)
         {
-            invoke();
-            function = src.release();
+            if (this == &src)
+                return *this;
         }
+        invoke();
+        function = src.release();
         return *this;
     }
 

base/base/wide_integer_impl.h

@@ -13,15 +13,14 @@
 #include <tuple>
 #include <limits>
 
-#include <boost/math/special_functions/fpclassify.hpp>
-
 // NOLINTBEGIN(*)
 
 /// Use same extended double for all platforms
 #if (LDBL_MANT_DIG == 64)
 #define CONSTEXPR_FROM_DOUBLE constexpr
 using FromDoubleIntermediateType = long double;
 #else
+#include <boost/math/special_functions/fpclassify.hpp>
 #include <boost/multiprecision/cpp_bin_float.hpp>
 /// `wide_integer_from_builtin` can't be constexpr with non-literal `cpp_bin_float_double_extended`
 #define CONSTEXPR_FROM_DOUBLE
@@ -309,6 +308,13 @@ struct integer<Bits, Signed>::_impl
         constexpr uint64_t max_int = std::numeric_limits<uint64_t>::max();
         static_assert(std::is_same_v<T, double> || std::is_same_v<T, FromDoubleIntermediateType>);
         /// Implementation specific behaviour on overflow (if we don't check here, stack overflow will triggered in bigint_cast).
+#if (LDBL_MANT_DIG == 64)
+        if (!std::isfinite(t))
+        {
+            self = 0;
+            return;
+        }
+#else
         if constexpr (std::is_same_v<T, double>)
         {
             if (!std::isfinite(t))
@@ -325,6 +331,7 @@ struct integer<Bits, Signed>::_impl
                 return;
             }
         }
+#endif
 
         const T alpha = t / static_cast<T>(max_int);
 

base/poco/Foundation/src/pcre_compile.c

@@ -4835,7 +4835,7 @@ for (;; ptr++)
 
     If the class contains characters outside the 0-255 range, a different
     opcode is compiled. It may optionally have a bit map for characters < 256,
-    but those above are are explicitly listed afterwards. A flag byte tells
+    but those above are explicitly listed afterwards. A flag byte tells
     whether the bitmap is present, and whether this is a negated class or not.
 
     In JavaScript compatibility mode, an isolated ']' causes an error. In

base/poco/Net/include/Poco/Net/HTTPClientSession.h

@@ -213,6 +213,19 @@ namespace Net
         Poco::Timespan getKeepAliveTimeout() const;
         /// Returns the connection timeout for HTTP connections.
 
+        void setKeepAliveMaxRequests(int max_requests);
+
+        int getKeepAliveMaxRequests() const;
+
+        int getKeepAliveRequest() const;
+
+        bool isKeepAliveExpired(double reliability = 1.0) const;
+        /// Returns if the connection is expired with some margin as fraction of timeout as reliability
+
+        double getKeepAliveReliability() const;
+        /// Returns the current fraction of keep alive timeout when connection is considered safe to use
+        /// It helps to avoid situation when a client uses nearly expired connection and receives NoMessageException
+
         virtual std::ostream & sendRequest(HTTPRequest & request);
         /// Sends the header for the given HTTP request to
         /// the server.
@@ -345,6 +358,8 @@ namespace Net
 
         void assign(HTTPClientSession & session);
 
+        void setKeepAliveRequest(int request);
+
         HTTPSessionFactory _proxySessionFactory;
         /// Factory to create HTTPClientSession to proxy.
     private:
@@ -353,6 +368,8 @@ namespace Net
         Poco::UInt16 _port;
         ProxyConfig _proxyConfig;
         Poco::Timespan _keepAliveTimeout;
+        int _keepAliveCurrentRequest = 0;
+        int _keepAliveMaxRequests = 1000;
         Poco::Timestamp _lastRequest;
         bool _reconnect;
         bool _mustReconnect;
@@ -361,6 +378,7 @@ namespace Net
         Poco::SharedPtr<std::ostream> _pRequestStream;
         Poco::SharedPtr<std::istream> _pResponseStream;
 
+        static const double _defaultKeepAliveReliabilityLevel;
         static ProxyConfig _globalProxyConfig;
 
         HTTPClientSession(const HTTPClientSession &);
@@ -450,9 +468,19 @@ namespace Net
         return _lastRequest;
     }
 
-    inline void HTTPClientSession::setLastRequest(Poco::Timestamp time)
+    inline double HTTPClientSession::getKeepAliveReliability() const
+    {
+        return _defaultKeepAliveReliabilityLevel;
+    }
+
+    inline int HTTPClientSession::getKeepAliveMaxRequests() const
+    {
+        return _keepAliveMaxRequests;
+    }
+
+    inline int HTTPClientSession::getKeepAliveRequest() const
     {
-        _lastRequest = time;
+        return _keepAliveCurrentRequest;
     }
 
 }

base/poco/Net/include/Poco/Net/HTTPMessage.h

@@ -120,6 +120,10 @@ namespace Net
         /// The value is set to "Keep-Alive" if keepAlive is
         /// true, or to "Close" otherwise.
 
+        void setKeepAliveTimeout(int timeout, int max_requests);
+        int getKeepAliveTimeout() const;
+        int getKeepAliveMaxRequests() const;
+
         bool getKeepAlive() const;
         /// Returns true if
         ///   * the message has a Connection header field and its value is "Keep-Alive"

base/poco/Net/include/Poco/Net/HTTPServerParams.h

@@ -44,7 +44,7 @@ namespace Net
         ///   - timeout:              60 seconds
         ///   - keepAlive:            true
         ///   - maxKeepAliveRequests: 0
-        ///   - keepAliveTimeout:     10 seconds
+        ///   - keepAliveTimeout:     15 seconds
 
         void setServerName(const std::string & serverName);
         /// Sets the name and port (name:port) that the server uses to identify itself.

base/poco/Net/include/Poco/Net/HTTPServerSession.h

@@ -56,6 +56,8 @@ namespace Net
         SocketAddress serverAddress();
         /// Returns the server's address.
 
+        void setKeepAliveTimeout(Poco::Timespan keepAliveTimeout);
+
     private:
         bool _firstRequest;
         Poco::Timespan _keepAliveTimeout;

base/poco/Net/src/HTTPClientSession.cpp

@@ -37,6 +37,7 @@ namespace Net {
 
 
 HTTPClientSession::ProxyConfig HTTPClientSession::_globalProxyConfig;
+const double HTTPClientSession::_defaultKeepAliveReliabilityLevel = 0.9;
 
 
 HTTPClientSession::HTTPClientSession():
@@ -220,7 +221,41 @@ void HTTPClientSession::setGlobalProxyConfig(const ProxyConfig& config)
 
 void HTTPClientSession::setKeepAliveTimeout(const Poco::Timespan& timeout)
 {
-	_keepAliveTimeout = timeout;
+    if (connected())
+    {
+        throw Poco::IllegalStateException("cannot change keep alive timeout on initiated connection, "
+                                          "That value is managed privately after connection is established.");
+    }
+    _keepAliveTimeout = timeout;
+}
+
+
+void HTTPClientSession::setKeepAliveMaxRequests(int max_requests)
+{
+    if (connected())
+    {
+        throw Poco::IllegalStateException("cannot change keep alive max requests on initiated connection, "
+                                          "That value is managed privately after connection is established.");
+    }
+    _keepAliveMaxRequests = max_requests;
+}
+
+
+void HTTPClientSession::setKeepAliveRequest(int request)
+{
+    _keepAliveCurrentRequest = request;
+}
+
+
+
+void HTTPClientSession::setLastRequest(Poco::Timestamp time)
+{
+    if (connected())
+    {
+        throw Poco::IllegalStateException("cannot change last request on initiated connection, "
+                                          "That value is managed privately after connection is established.");
+    }
+    _lastRequest = time;
 }
 
 
@@ -231,6 +266,8 @@ std::ostream& HTTPClientSession::sendRequest(HTTPRequest& request)
 	clearException();
 	_responseReceived = false;
 
+    _keepAliveCurrentRequest += 1;
+
 	bool keepAlive = getKeepAlive();
 	if (((connected() && !keepAlive) || mustReconnect()) && !_host.empty())
 	{
@@ -241,8 +278,10 @@ std::ostream& HTTPClientSession::sendRequest(HTTPRequest& request)
 	{
 		if (!connected())
 			reconnect();
-		if (!keepAlive)
-			request.setKeepAlive(false);
+        if (!request.has(HTTPMessage::CONNECTION))
+            request.setKeepAlive(keepAlive);
+        if (keepAlive && !request.has(HTTPMessage::CONNECTION_KEEP_ALIVE) && _keepAliveTimeout.totalSeconds() > 0)
+            request.setKeepAliveTimeout(_keepAliveTimeout.totalSeconds(), _keepAliveMaxRequests);
 		if (!request.has(HTTPRequest::HOST) && !_host.empty())
 			request.setHost(_host, _port);
 		if (!_proxyConfig.host.empty() && !bypassProxy())
@@ -324,6 +363,17 @@ std::istream& HTTPClientSession::receiveResponse(HTTPResponse& response)
 
 	_mustReconnect = getKeepAlive() && !response.getKeepAlive();
 
+    if (!_mustReconnect)
+    {
+        /// when server sends its keep alive timeout, client has to follow that value
+        auto timeout = response.getKeepAliveTimeout();
+        if (timeout > 0)
+            _keepAliveTimeout = std::min(_keepAliveTimeout, Poco::Timespan(timeout, 0));
+        auto max_requests = response.getKeepAliveMaxRequests();
+        if (max_requests > 0)
+            _keepAliveMaxRequests = std::min(_keepAliveMaxRequests, max_requests);
+    }
+
 	if (!_expectResponseBody || response.getStatus() < 200 || response.getStatus() == HTTPResponse::HTTP_NO_CONTENT || response.getStatus() == HTTPResponse::HTTP_NOT_MODIFIED)
 		_pResponseStream = new HTTPFixedLengthInputStream(*this, 0);
 	else if (response.getChunkedTransferEncoding())
@@ -430,15 +480,18 @@ std::string HTTPClientSession::proxyRequestPrefix() const
 	return result;
 }
 
+bool HTTPClientSession::isKeepAliveExpired(double reliability) const
+{
+    Poco::Timestamp now;
+    return Timespan(Timestamp::TimeDiff(reliability *_keepAliveTimeout.totalMicroseconds())) <= now - _lastRequest
+            || _keepAliveCurrentRequest > _keepAliveMaxRequests;
+}
 
 bool HTTPClientSession::mustReconnect() const
 {
 	if (!_mustReconnect)
-	{
-		Poco::Timestamp now;
-		return _keepAliveTimeout <= now - _lastRequest;
-	}
-	else return true;
+        return isKeepAliveExpired(_defaultKeepAliveReliabilityLevel);
+    return true;
 }
 
 
@@ -511,14 +564,21 @@ void HTTPClientSession::assign(Poco::Net::HTTPClientSession & session)
     if (buffered())
         throw Poco::LogicException("assign to a session with not empty buffered data");
 
-    attachSocket(session.detachSocket());
-    setLastRequest(session.getLastRequest());
+    poco_assert(!connected());
+
     setResolvedHost(session.getResolvedHost());
-    setKeepAlive(session.getKeepAlive());
+    setProxyConfig(session.getProxyConfig());
 
     setTimeout(session.getConnectionTimeout(), session.getSendTimeout(), session.getReceiveTimeout());
+    setKeepAlive(session.getKeepAlive());
+
+    setLastRequest(session.getLastRequest());
     setKeepAliveTimeout(session.getKeepAliveTimeout());
-    setProxyConfig(session.getProxyConfig());
+
+    _keepAliveMaxRequests = session._keepAliveMaxRequests;
+    _keepAliveCurrentRequest = session._keepAliveCurrentRequest;
+
+    attachSocket(session.detachSocket());
 
     session.reset();
 }