diff --git a/include/envoy/common/BUILD b/include/envoy/common/BUILD index 105f8b4374b7c..47dd8e1549ef3 100644 --- a/include/envoy/common/BUILD +++ b/include/envoy/common/BUILD @@ -19,6 +19,11 @@ envoy_basic_cc_library( include_prefix = "envoy/common", ) +envoy_cc_library( + name = "conn_pool_interface", + hdrs = ["conn_pool.h"], +) + envoy_cc_library( name = "mutex_tracer", hdrs = ["mutex_tracer.h"], diff --git a/include/envoy/common/conn_pool.h b/include/envoy/common/conn_pool.h new file mode 100644 index 0000000000000..c8a988b547943 --- /dev/null +++ b/include/envoy/common/conn_pool.h @@ -0,0 +1,18 @@ +#pragma once + +namespace Envoy { +namespace ConnectionPool { + +enum class PoolFailureReason { + // A resource overflowed and policy prevented a new connection from being created. + Overflow, + // A local connection failure took place while creating a new connection. + LocalConnectionFailure, + // A remote connection failure took place while creating a new connection. + RemoteConnectionFailure, + // A timeout occurred while creating a new connection. + Timeout, +}; + +} // namespace ConnectionPool +} // namespace Envoy diff --git a/include/envoy/http/BUILD b/include/envoy/http/BUILD index 2c67a90ace3ba..e0bc196ac5b8e 100644 --- a/include/envoy/http/BUILD +++ b/include/envoy/http/BUILD @@ -50,6 +50,7 @@ envoy_cc_library( hdrs = ["conn_pool.h"], deps = [ ":codec_interface", + "//include/envoy/common:conn_pool_interface", "//include/envoy/event:deferred_deletable", "//include/envoy/upstream:upstream_interface", ], diff --git a/include/envoy/http/conn_pool.h b/include/envoy/http/conn_pool.h index 1eb40df565617..c41fe57641201 100644 --- a/include/envoy/http/conn_pool.h +++ b/include/envoy/http/conn_pool.h @@ -3,6 +3,7 @@ #include #include +#include "envoy/common/conn_pool.h" #include "envoy/common/pure.h" #include "envoy/event/deferred_deletable.h" #include "envoy/http/codec.h" @@ -25,15 +26,7 @@ class Cancellable { virtual void cancel() PURE; }; -/** - * Reason that a pool stream could not be obtained. - */ -enum class PoolFailureReason { - // A resource overflowed and policy prevented a new stream from being created. - Overflow, - // A connection failure took place and the stream could not be bound. - ConnectionFailure -}; +using PoolFailureReason = ::Envoy::ConnectionPool::PoolFailureReason; /** * Pool callbacks invoked in the context of a newStream() call, either synchronously or diff --git a/include/envoy/tcp/BUILD b/include/envoy/tcp/BUILD index 3716bc5cb4f64..991ccbd75e139 100644 --- a/include/envoy/tcp/BUILD +++ b/include/envoy/tcp/BUILD @@ -13,6 +13,7 @@ envoy_cc_library( hdrs = ["conn_pool.h"], deps = [ "//include/envoy/buffer:buffer_interface", + "//include/envoy/common:conn_pool_interface", "//include/envoy/event:deferred_deletable", "//include/envoy/upstream:upstream_interface", ], diff --git a/include/envoy/tcp/conn_pool.h b/include/envoy/tcp/conn_pool.h index 0d16c9afbfe1d..5cdcd617daf7e 100644 --- a/include/envoy/tcp/conn_pool.h +++ b/include/envoy/tcp/conn_pool.h @@ -4,6 +4,7 @@ #include #include "envoy/buffer/buffer.h" +#include "envoy/common/conn_pool.h" #include "envoy/common/pure.h" #include "envoy/event/deferred_deletable.h" #include "envoy/upstream/upstream.h" @@ -42,20 +43,6 @@ class Cancellable { virtual void cancel(CancelPolicy cancel_policy) PURE; }; -/** - * Reason that a pool connection could not be obtained. - */ -enum class PoolFailureReason { - // A resource overflowed and policy prevented a new connection from being created. - Overflow, - // A local connection failure took place while creating a new connection. - LocalConnectionFailure, - // A remote connection failure took place while creating a new connection. - RemoteConnectionFailure, - // A timeout occurred while creating a new connection. - Timeout, -}; - /* * UpstreamCallbacks for connection pool upstream connection callbacks and data. Note that * onEvent(Connected) is never triggered since the event always occurs before a ConnectionPool @@ -131,6 +118,7 @@ class ConnectionData { }; using ConnectionDataPtr = std::unique_ptr; +using PoolFailureReason = ::Envoy::ConnectionPool::PoolFailureReason; /** * Pool callbacks invoked in the context of a newConnection() call, either synchronously or diff --git a/source/common/http/conn_pool_base.cc b/source/common/http/conn_pool_base.cc index da9736a4e6079..b59c45a5beef1 100644 --- a/source/common/http/conn_pool_base.cc +++ b/source/common/http/conn_pool_base.cc @@ -274,6 +274,15 @@ void ConnPoolImplBase::onConnectionEvent(ConnPoolImplBase::ActiveClient& client, host_->cluster().stats().upstream_cx_connect_fail_.inc(); host_->stats().cx_connect_fail_.inc(); + ConnectionPool::PoolFailureReason reason; + if (client.timed_out_) { + reason = ConnectionPool::PoolFailureReason::Timeout; + } else if (event == Network::ConnectionEvent::RemoteClose) { + reason = ConnectionPool::PoolFailureReason::RemoteConnectionFailure; + } else { + reason = ConnectionPool::PoolFailureReason::LocalConnectionFailure; + } + // Raw connect failures should never happen under normal circumstances. If we have an upstream // that is behaving badly, requests can get stuck here in the pending state. If we see a // connect failure, we purge all pending requests so that calling code can determine what to @@ -281,7 +290,7 @@ void ConnPoolImplBase::onConnectionEvent(ConnPoolImplBase::ActiveClient& client, // NOTE: We move the existing pending requests to a temporary list. This is done so that // if retry logic submits a new request to the pool, we don't fail it inline. purgePendingRequests(client.real_host_description_, - client.codec_client_->connectionFailureReason()); + client.codec_client_->connectionFailureReason(), reason); } // We need to release our resourceManager() resources before checking below for @@ -342,7 +351,7 @@ ConnPoolImplBase::newPendingRequest(ResponseDecoder& decoder, void ConnPoolImplBase::purgePendingRequests( const Upstream::HostDescriptionConstSharedPtr& host_description, - absl::string_view failure_reason) { + absl::string_view failure_reason, ConnectionPool::PoolFailureReason reason) { // NOTE: We move the existing pending requests to a temporary list. This is done so that // if retry logic submits a new request to the pool, we don't fail it inline. pending_requests_to_purge_ = std::move(pending_requests_); @@ -350,8 +359,7 @@ void ConnPoolImplBase::purgePendingRequests( PendingRequestPtr request = pending_requests_to_purge_.front()->removeFromList(pending_requests_to_purge_); host_->cluster().stats().upstream_rq_pending_failure_eject_.inc(); - request->callbacks_.onPoolFailure(ConnectionPool::PoolFailureReason::ConnectionFailure, - failure_reason, host_description); + request->callbacks_.onPoolFailure(reason, failure_reason, host_description); } } @@ -428,6 +436,7 @@ void ConnPoolImplBase::ActiveClient::releaseResources() { void ConnPoolImplBase::ActiveClient::onConnectTimeout() { ENVOY_CONN_LOG(debug, "connect timeout", *codec_client_); parent_.host_->cluster().stats().upstream_cx_connect_timeout_.inc(); + timed_out_ = true; close(); } diff --git a/source/common/http/conn_pool_base.h b/source/common/http/conn_pool_base.h index a81434cead9c2..e702f2a057da8 100644 --- a/source/common/http/conn_pool_base.h +++ b/source/common/http/conn_pool_base.h @@ -89,6 +89,7 @@ class ConnPoolImplBase : public ConnectionPool::Instance, Stats::TimespanPtr conn_length_; Event::TimerPtr connect_timer_; bool resources_released_{false}; + bool timed_out_{false}; }; using ActiveClientPtr = std::unique_ptr; @@ -126,7 +127,8 @@ class ConnPoolImplBase : public ConnectionPool::Instance, // Fails all pending requests, calling onPoolFailure on the associated callbacks. void purgePendingRequests(const Upstream::HostDescriptionConstSharedPtr& host_description, - absl::string_view failure_reason); + absl::string_view failure_reason, + ConnectionPool::PoolFailureReason pool_failure_reason); // Closes any idle connections. void closeIdleConnections(); diff --git a/source/common/http/conn_pool_base_legacy.cc b/source/common/http/conn_pool_base_legacy.cc index 9476a846a321d..d50cb871bff3a 100644 --- a/source/common/http/conn_pool_base_legacy.cc +++ b/source/common/http/conn_pool_base_legacy.cc @@ -62,7 +62,7 @@ ConnPoolImplBase::newPendingRequest(ResponseDecoder& decoder, void ConnPoolImplBase::purgePendingRequests( const Upstream::HostDescriptionConstSharedPtr& host_description, - absl::string_view failure_reason) { + absl::string_view failure_reason, bool was_remote_close) { // NOTE: We move the existing pending requests to a temporary list. This is done so that // if retry logic submits a new request to the pool, we don't fail it inline. pending_requests_to_purge_ = std::move(pending_requests_); @@ -70,8 +70,10 @@ void ConnPoolImplBase::purgePendingRequests( PendingRequestPtr request = pending_requests_to_purge_.front()->removeFromList(pending_requests_to_purge_); host_->cluster().stats().upstream_rq_pending_failure_eject_.inc(); - request->callbacks_.onPoolFailure(ConnectionPool::PoolFailureReason::ConnectionFailure, - failure_reason, host_description); + request->callbacks_.onPoolFailure( + was_remote_close ? ConnectionPool::PoolFailureReason::RemoteConnectionFailure + : ConnectionPool::PoolFailureReason::LocalConnectionFailure, + failure_reason, host_description); } } diff --git a/source/common/http/conn_pool_base_legacy.h b/source/common/http/conn_pool_base_legacy.h index edcd2529156d7..7c96ec3a1aafd 100644 --- a/source/common/http/conn_pool_base_legacy.h +++ b/source/common/http/conn_pool_base_legacy.h @@ -63,7 +63,7 @@ class ConnPoolImplBase : protected Logger::Loggable { // Fails all pending requests, calling onPoolFailure on the associated callbacks. void purgePendingRequests(const Upstream::HostDescriptionConstSharedPtr& host_description, - absl::string_view failure_reason); + absl::string_view failure_reason, bool was_remote); // Must be implemented by sub class. Attempts to drain inactive clients. virtual void checkForDrained() PURE; diff --git a/source/common/http/http1/conn_pool_legacy.cc b/source/common/http/http1/conn_pool_legacy.cc index 1c5942b219c41..da834c2c104a3 100644 --- a/source/common/http/http1/conn_pool_legacy.cc +++ b/source/common/http/http1/conn_pool_legacy.cc @@ -163,7 +163,8 @@ void ConnPoolImpl::onConnectionEvent(ActiveClient& client, Network::ConnectionEv ENVOY_CONN_LOG(debug, "purge pending, failure reason: {}", *client.codec_client_, client.codec_client_->connectionFailureReason()); purgePendingRequests(client.real_host_description_, - client.codec_client_->connectionFailureReason()); + client.codec_client_->connectionFailureReason(), + event == Network::ConnectionEvent::RemoteClose); } dispatcher_.deferredDelete(std::move(removed)); diff --git a/source/common/http/http2/conn_pool_legacy.cc b/source/common/http/http2/conn_pool_legacy.cc index c949459d548b9..d9834e1893baa 100644 --- a/source/common/http/http2/conn_pool_legacy.cc +++ b/source/common/http/http2/conn_pool_legacy.cc @@ -166,8 +166,8 @@ void ConnPoolImpl::onConnectionEvent(ActiveClient& client, Network::ConnectionEv // do with the request. // NOTE: We move the existing pending requests to a temporary list. This is done so that // if retry logic submits a new request to the pool, we don't fail it inline. - purgePendingRequests(client.real_host_description_, - client.client_->connectionFailureReason()); + purgePendingRequests(client.real_host_description_, client.client_->connectionFailureReason(), + event == Network::ConnectionEvent::RemoteClose); } if (&client == primary_client_.get()) { diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index b8d11e18f21be..471eaca442f30 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -289,17 +289,21 @@ void UpstreamRequest::onPerTryTimeout() { } } -void UpstreamRequest::onPoolFailure(Http::ConnectionPool::PoolFailureReason reason, +void UpstreamRequest::onPoolFailure(ConnectionPool::PoolFailureReason reason, absl::string_view transport_failure_reason, Upstream::HostDescriptionConstSharedPtr host) { Http::StreamResetReason reset_reason = Http::StreamResetReason::ConnectionFailure; switch (reason) { - case Http::ConnectionPool::PoolFailureReason::Overflow: + case ConnectionPool::PoolFailureReason::Overflow: reset_reason = Http::StreamResetReason::Overflow; break; - case Http::ConnectionPool::PoolFailureReason::ConnectionFailure: + case ConnectionPool::PoolFailureReason::RemoteConnectionFailure: + FALLTHRU; + case ConnectionPool::PoolFailureReason::LocalConnectionFailure: reset_reason = Http::StreamResetReason::ConnectionFailure; break; + case ConnectionPool::PoolFailureReason::Timeout: + reset_reason = Http::StreamResetReason::LocalReset; } // Mimic an upstream reset. @@ -485,7 +489,7 @@ bool HttpConnPool::cancelAnyPendingRequest() { absl::optional HttpConnPool::protocol() const { return conn_pool_.protocol(); } -void HttpConnPool::onPoolFailure(Http::ConnectionPool::PoolFailureReason reason, +void HttpConnPool::onPoolFailure(ConnectionPool::PoolFailureReason reason, absl::string_view transport_failure_reason, Upstream::HostDescriptionConstSharedPtr host) { callbacks_->onPoolFailure(reason, transport_failure_reason, host); diff --git a/source/common/router/upstream_request.h b/source/common/router/upstream_request.h index 0cf36565bed47..0e85a21ddae1b 100644 --- a/source/common/router/upstream_request.h +++ b/source/common/router/upstream_request.h @@ -52,7 +52,7 @@ class GenericConnectionPoolCallbacks { public: virtual ~GenericConnectionPoolCallbacks() = default; - virtual void onPoolFailure(Http::ConnectionPool::PoolFailureReason reason, + virtual void onPoolFailure(ConnectionPool::PoolFailureReason reason, absl::string_view transport_failure_reason, Upstream::HostDescriptionConstSharedPtr host) PURE; virtual void onPoolReady(std::unique_ptr&& upstream, @@ -97,7 +97,7 @@ class UpstreamRequest : public Logger::Loggable, void enableDataFromDownstreamForFlowControl(); // GenericConnPool - void onPoolFailure(Http::ConnectionPool::PoolFailureReason reason, + void onPoolFailure(ConnectionPool::PoolFailureReason reason, absl::string_view transport_failure_reason, Upstream::HostDescriptionConstSharedPtr host) override; void onPoolReady(std::unique_ptr&& upstream, @@ -186,7 +186,7 @@ class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callba absl::optional protocol() const override; // Http::ConnectionPool::Callbacks - void onPoolFailure(Http::ConnectionPool::PoolFailureReason reason, + void onPoolFailure(ConnectionPool::PoolFailureReason reason, absl::string_view transport_failure_reason, Upstream::HostDescriptionConstSharedPtr host) override; void onPoolReady(Http::RequestEncoder& callbacks_encoder, diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index dafba6aa0ea27..539ea9dde24e9 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -28,21 +28,6 @@ namespace Envoy { namespace TcpProxy { -namespace { - -Tcp::ConnectionPool::PoolFailureReason -httpToTcpFailure(Http::ConnectionPool::PoolFailureReason reason) { - switch (reason) { - case Http::ConnectionPool::PoolFailureReason::Overflow: - return Tcp::ConnectionPool::PoolFailureReason::Overflow; - case Http::ConnectionPool::PoolFailureReason::ConnectionFailure: - // TODO(alyssawilk) It's unclear which this is. - return Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure; - } - return Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure; -} - -} // namespace const std::string& PerConnectionCluster::key() { CONSTRUCT_ON_FIRST_USE(std::string, "envoy.tcp_proxy.cluster"); @@ -464,7 +449,7 @@ Network::FilterStatus Filter::initializeUpstreamConnection() { return Network::FilterStatus::StopIteration; } -void Filter::onPoolFailure(Tcp::ConnectionPool::PoolFailureReason reason, +void Filter::onPoolFailure(ConnectionPool::PoolFailureReason reason, Upstream::HostDescriptionConstSharedPtr host) { upstream_handle_.reset(); @@ -472,16 +457,16 @@ void Filter::onPoolFailure(Tcp::ConnectionPool::PoolFailureReason reason, getStreamInfo().onUpstreamHostSelected(host); switch (reason) { - case Tcp::ConnectionPool::PoolFailureReason::Overflow: - case Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure: + case ConnectionPool::PoolFailureReason::Overflow: + case ConnectionPool::PoolFailureReason::LocalConnectionFailure: upstream_callbacks_->onEvent(Network::ConnectionEvent::LocalClose); break; - case Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure: + case ConnectionPool::PoolFailureReason::RemoteConnectionFailure: upstream_callbacks_->onEvent(Network::ConnectionEvent::RemoteClose); break; - case Tcp::ConnectionPool::PoolFailureReason::Timeout: + case ConnectionPool::PoolFailureReason::Timeout: onConnectTimeout(); break; @@ -514,9 +499,9 @@ void Filter::onPoolReady(Tcp::ConnectionPool::ConnectionDataPtr&& conn_data, latched_data->connection().streamInfo().filterState()); } -void Filter::onPoolFailure(Http::ConnectionPool::PoolFailureReason failure, absl::string_view, +void Filter::onPoolFailure(ConnectionPool::PoolFailureReason failure, absl::string_view, Upstream::HostDescriptionConstSharedPtr host) { - onPoolFailure(httpToTcpFailure(failure), host); + onPoolFailure(failure, host); } void Filter::onPoolReady(Http::RequestEncoder& request_encoder, diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index 25dd3353ec64c..950f8f654dbe0 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -245,13 +245,13 @@ class Filter : public Network::ReadFilter, void initializeReadFilterCallbacks(Network::ReadFilterCallbacks& callbacks) override; // Tcp::ConnectionPool::Callbacks - void onPoolFailure(Tcp::ConnectionPool::PoolFailureReason reason, + void onPoolFailure(ConnectionPool::PoolFailureReason reason, Upstream::HostDescriptionConstSharedPtr host) override; void onPoolReady(Tcp::ConnectionPool::ConnectionDataPtr&& conn_data, Upstream::HostDescriptionConstSharedPtr host) override; // Http::ConnectionPool::Callbacks, - void onPoolFailure(Http::ConnectionPool::PoolFailureReason reason, + void onPoolFailure(ConnectionPool::PoolFailureReason reason, absl::string_view transport_failure_reason, Upstream::HostDescriptionConstSharedPtr host) override; void onPoolReady(Http::RequestEncoder& request_encoder, diff --git a/source/extensions/filters/network/dubbo_proxy/router/router_impl.cc b/source/extensions/filters/network/dubbo_proxy/router/router_impl.cc index ec26e3fb70e37..5bd9aab946770 100644 --- a/source/extensions/filters/network/dubbo_proxy/router/router_impl.cc +++ b/source/extensions/filters/network/dubbo_proxy/router/router_impl.cc @@ -112,8 +112,7 @@ void Router::onUpstreamData(Buffer::Instance& data, bool end_stream) { if (end_stream) { // Response is incomplete, but no more data is coming. ENVOY_STREAM_LOG(debug, "dubbo router: response underflow", *callbacks_); - upstream_request_->onResetStream( - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + upstream_request_->onResetStream(ConnectionPool::PoolFailureReason::RemoteConnectionFailure); upstream_request_->onResponseComplete(); cleanup(); } @@ -133,12 +132,10 @@ void Router::onEvent(Network::ConnectionEvent event) { switch (event) { case Network::ConnectionEvent::RemoteClose: - upstream_request_->onResetStream( - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + upstream_request_->onResetStream(ConnectionPool::PoolFailureReason::RemoteConnectionFailure); break; case Network::ConnectionEvent::LocalClose: - upstream_request_->onResetStream( - Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure); + upstream_request_->onResetStream(ConnectionPool::PoolFailureReason::LocalConnectionFailure); break; default: // Connected is consumed by the connection pool. @@ -205,7 +202,7 @@ void Router::UpstreamRequest::encodeData(Buffer::Instance& data) { conn_data_->connection().write(data, false); } -void Router::UpstreamRequest::onPoolFailure(Tcp::ConnectionPool::PoolFailureReason reason, +void Router::UpstreamRequest::onPoolFailure(ConnectionPool::PoolFailureReason reason, Upstream::HostDescriptionConstSharedPtr host) { conn_pool_handle_ = nullptr; @@ -219,9 +216,9 @@ void Router::UpstreamRequest::onPoolFailure(Tcp::ConnectionPool::PoolFailureReas // the error asynchronously and the upper layer needs to be notified to continue decoding. // If it is a non-connection error, it is returned synchronously from the connection pool // and is still in the callback at the current Filter, nothing to do. - if (reason == Tcp::ConnectionPool::PoolFailureReason::Timeout || - reason == Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure || - reason == Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure) { + if (reason == ConnectionPool::PoolFailureReason::Timeout || + reason == ConnectionPool::PoolFailureReason::LocalConnectionFailure || + reason == ConnectionPool::PoolFailureReason::RemoteConnectionFailure) { parent_.callbacks_->continueDecoding(); } } @@ -264,7 +261,7 @@ void Router::UpstreamRequest::onUpstreamHostSelected(Upstream::HostDescriptionCo upstream_host_ = host; } -void Router::UpstreamRequest::onResetStream(Tcp::ConnectionPool::PoolFailureReason reason) { +void Router::UpstreamRequest::onResetStream(ConnectionPool::PoolFailureReason reason) { if (metadata_->message_type() == MessageType::Oneway) { // For oneway requests, we should not attempt a response. Reset the downstream to signal // an error. @@ -276,13 +273,13 @@ void Router::UpstreamRequest::onResetStream(Tcp::ConnectionPool::PoolFailureReas // When the filter's callback does not end, the sendLocalReply function call // triggers the release of the current stream at the end of the filter's callback. switch (reason) { - case Tcp::ConnectionPool::PoolFailureReason::Overflow: + case ConnectionPool::PoolFailureReason::Overflow: parent_.callbacks_->sendLocalReply( AppException(ResponseStatus::ServerError, fmt::format("dubbo upstream request: too many connections")), false); break; - case Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure: + case ConnectionPool::PoolFailureReason::LocalConnectionFailure: // Should only happen if we closed the connection, due to an error condition, in which case // we've already handled any possible downstream response. parent_.callbacks_->sendLocalReply( @@ -291,14 +288,14 @@ void Router::UpstreamRequest::onResetStream(Tcp::ConnectionPool::PoolFailureReas upstream_host_->address()->asString())), false); break; - case Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure: + case ConnectionPool::PoolFailureReason::RemoteConnectionFailure: parent_.callbacks_->sendLocalReply( AppException(ResponseStatus::ServerError, fmt::format("dubbo upstream request: remote connection failure '{}'", upstream_host_->address()->asString())), false); break; - case Tcp::ConnectionPool::PoolFailureReason::Timeout: + case ConnectionPool::PoolFailureReason::Timeout: parent_.callbacks_->sendLocalReply( AppException(ResponseStatus::ServerError, fmt::format("dubbo upstream request: connection failure '{}' due to timeout", diff --git a/source/extensions/filters/network/dubbo_proxy/router/router_impl.h b/source/extensions/filters/network/dubbo_proxy/router/router_impl.h index 9fc078a5aa4be..8eb73cc122ffa 100644 --- a/source/extensions/filters/network/dubbo_proxy/router/router_impl.h +++ b/source/extensions/filters/network/dubbo_proxy/router/router_impl.h @@ -54,7 +54,7 @@ class Router : public Tcp::ConnectionPool::UpstreamCallbacks, void encodeData(Buffer::Instance& data); // Tcp::ConnectionPool::Callbacks - void onPoolFailure(Tcp::ConnectionPool::PoolFailureReason reason, + void onPoolFailure(ConnectionPool::PoolFailureReason reason, Upstream::HostDescriptionConstSharedPtr host) override; void onPoolReady(Tcp::ConnectionPool::ConnectionDataPtr&& conn, Upstream::HostDescriptionConstSharedPtr host) override; @@ -63,7 +63,7 @@ class Router : public Tcp::ConnectionPool::UpstreamCallbacks, void onRequestComplete(); void onResponseComplete(); void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host); - void onResetStream(Tcp::ConnectionPool::PoolFailureReason reason); + void onResetStream(ConnectionPool::PoolFailureReason reason); Router& parent_; Tcp::ConnectionPool::Instance& conn_pool_; diff --git a/source/extensions/filters/network/thrift_proxy/router/router_impl.cc b/source/extensions/filters/network/thrift_proxy/router/router_impl.cc index 80dcb91d5fb52..885538c6298a1 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_impl.cc +++ b/source/extensions/filters/network/thrift_proxy/router/router_impl.cc @@ -344,8 +344,7 @@ void Router::onUpstreamData(Buffer::Instance& data, bool end_stream) { // Response is incomplete, but no more data is coming. ENVOY_STREAM_LOG(debug, "response underflow", *callbacks_); upstream_request_->onResponseComplete(); - upstream_request_->onResetStream( - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + upstream_request_->onResetStream(ConnectionPool::PoolFailureReason::RemoteConnectionFailure); cleanup(); } } @@ -356,13 +355,11 @@ void Router::onEvent(Network::ConnectionEvent event) { switch (event) { case Network::ConnectionEvent::RemoteClose: ENVOY_STREAM_LOG(debug, "upstream remote close", *callbacks_); - upstream_request_->onResetStream( - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + upstream_request_->onResetStream(ConnectionPool::PoolFailureReason::RemoteConnectionFailure); break; case Network::ConnectionEvent::LocalClose: ENVOY_STREAM_LOG(debug, "upstream local close", *callbacks_); - upstream_request_->onResetStream( - Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure); + upstream_request_->onResetStream(ConnectionPool::PoolFailureReason::LocalConnectionFailure); break; default: // Connected is consumed by the connection pool. @@ -434,7 +431,7 @@ void Router::UpstreamRequest::releaseConnection(const bool close) { void Router::UpstreamRequest::resetStream() { releaseConnection(true); } -void Router::UpstreamRequest::onPoolFailure(Tcp::ConnectionPool::PoolFailureReason reason, +void Router::UpstreamRequest::onPoolFailure(ConnectionPool::PoolFailureReason reason, Upstream::HostDescriptionConstSharedPtr host) { conn_pool_handle_ = nullptr; @@ -494,7 +491,7 @@ void Router::UpstreamRequest::onUpstreamHostSelected(Upstream::HostDescriptionCo upstream_host_ = host; } -void Router::UpstreamRequest::onResetStream(Tcp::ConnectionPool::PoolFailureReason reason) { +void Router::UpstreamRequest::onResetStream(ConnectionPool::PoolFailureReason reason) { if (metadata_->messageType() == MessageType::Oneway) { // For oneway requests, we should not attempt a response. Reset the downstream to signal // an error. @@ -503,20 +500,20 @@ void Router::UpstreamRequest::onResetStream(Tcp::ConnectionPool::PoolFailureReas } switch (reason) { - case Tcp::ConnectionPool::PoolFailureReason::Overflow: + case ConnectionPool::PoolFailureReason::Overflow: parent_.callbacks_->sendLocalReply( AppException( AppExceptionType::InternalError, fmt::format("too many connections to '{}'", upstream_host_->address()->asString())), true); break; - case Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure: + case ConnectionPool::PoolFailureReason::LocalConnectionFailure: // Should only happen if we closed the connection, due to an error condition, in which case // we've already handled any possible downstream response. parent_.callbacks_->resetDownstreamConnection(); break; - case Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure: - case Tcp::ConnectionPool::PoolFailureReason::Timeout: + case ConnectionPool::PoolFailureReason::RemoteConnectionFailure: + case ConnectionPool::PoolFailureReason::Timeout: // TODO(zuercher): distinguish between these cases where appropriate (particularly timeout) if (!response_started_) { parent_.callbacks_->sendLocalReply( diff --git a/source/extensions/filters/network/thrift_proxy/router/router_impl.h b/source/extensions/filters/network/thrift_proxy/router/router_impl.h index edec657782f13..c41793a8066cc 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_impl.h +++ b/source/extensions/filters/network/thrift_proxy/router/router_impl.h @@ -223,7 +223,7 @@ class Router : public Tcp::ConnectionPool::UpstreamCallbacks, void releaseConnection(bool close); // Tcp::ConnectionPool::Callbacks - void onPoolFailure(Tcp::ConnectionPool::PoolFailureReason reason, + void onPoolFailure(ConnectionPool::PoolFailureReason reason, Upstream::HostDescriptionConstSharedPtr host) override; void onPoolReady(Tcp::ConnectionPool::ConnectionDataPtr&& conn, Upstream::HostDescriptionConstSharedPtr host) override; @@ -232,7 +232,7 @@ class Router : public Tcp::ConnectionPool::UpstreamCallbacks, void onRequestComplete(); void onResponseComplete(); void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host); - void onResetStream(Tcp::ConnectionPool::PoolFailureReason reason); + void onResetStream(ConnectionPool::PoolFailureReason reason); Router& parent_; Tcp::ConnectionPool::Instance& conn_pool_; diff --git a/test/common/http/common.h b/test/common/http/common.h index 45fb50dab9f95..7eacfa3ad03bb 100644 --- a/test/common/http/common.h +++ b/test/common/http/common.h @@ -45,12 +45,14 @@ struct ConnPoolCallbacks : public Http::ConnectionPool::Callbacks { pool_ready_.ready(); } - void onPoolFailure(Http::ConnectionPool::PoolFailureReason, absl::string_view, + void onPoolFailure(ConnectionPool::PoolFailureReason reason, absl::string_view, Upstream::HostDescriptionConstSharedPtr host) override { host_ = host; + reason_ = reason; pool_failure_.ready(); } + ConnectionPool::PoolFailureReason reason_; ReadyWatcher pool_failure_; ReadyWatcher pool_ready_; Http::RequestEncoder* outer_encoder_{}; diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index 6491520d5e432..beb7344b53335 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -357,6 +357,7 @@ TEST_F(Http1ConnPoolImplTest, MaxPendingRequests) { EXPECT_CALL(callbacks2.pool_failure_, ready()); Http::ConnectionPool::Cancellable* handle2 = conn_pool_.newStream(outer_decoder2, callbacks2); EXPECT_EQ(nullptr, handle2); + EXPECT_EQ(callbacks2.reason_, ConnectionPool::PoolFailureReason::Overflow); EXPECT_EQ(1U, cluster_->circuit_breakers_stats_.rq_pending_open_.value()); diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index 651d41606b303..c8ced6e33f5e9 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -106,7 +106,7 @@ class Http2ConnPoolImplTest : public testing::Test { // Resets the connection belonging to the provided index, asserting that the // provided request receives onPoolFailure. - void expectClientReset(size_t index, ActiveTestRequest& r); + void expectClientReset(size_t index, ActiveTestRequest& r, bool local_failure); // Asserts that the provided requests receives onPoolFailure. void expectStreamReset(ActiveTestRequest& r); @@ -171,10 +171,17 @@ void Http2ConnPoolImplTest::expectStreamConnect(size_t index, ActiveTestRequest& EXPECT_CALL(r.callbacks_.pool_ready_, ready()); } -void Http2ConnPoolImplTest::expectClientReset(size_t index, ActiveTestRequest& r) { +void Http2ConnPoolImplTest::expectClientReset(size_t index, ActiveTestRequest& r, + bool local_failure) { expectStreamReset(r); EXPECT_CALL(*test_clients_[0].connect_timer_, disableTimer()); - test_clients_[index].connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); + if (local_failure) { + test_clients_[index].connection_->raiseEvent(Network::ConnectionEvent::LocalClose); + EXPECT_EQ(r.callbacks_.reason_, ConnectionPool::PoolFailureReason::LocalConnectionFailure); + } else { + test_clients_[index].connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); + EXPECT_EQ(r.callbacks_.reason_, ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + } } void Http2ConnPoolImplTest::expectStreamReset(ActiveTestRequest& r) { @@ -514,7 +521,7 @@ TEST_F(Http2ConnPoolImplTest, PendingRequestsFailure) { // Note that these occur in reverse order due to the order we purge pending requests in. expectStreamReset(r3); expectStreamReset(r2); - expectClientReset(0, r1); + expectClientReset(0, r1, false); expectClientCreate(); // Since we have no active connection, subsequence requests will queue until @@ -531,6 +538,26 @@ TEST_F(Http2ConnPoolImplTest, PendingRequestsFailure) { EXPECT_EQ(2U, cluster_->stats_.upstream_cx_destroy_remote_.value()); } +// Verifies resets due to local connection closes are tracked correctly. +TEST_F(Http2ConnPoolImplTest, LocalFailure) { + InSequence s; + cluster_->max_requests_per_connection_ = 10; + + // Create three requests. These should be queued up. + expectClientCreate(); + ActiveTestRequest r1(*this, 0, false); + ActiveTestRequest r2(*this, 0, false); + ActiveTestRequest r3(*this, 0, false); + + // The connection now becomes ready. This should cause all the queued requests to be sent. + // Note that these occur in reverse order due to the order we purge pending requests in. + expectStreamReset(r3); + expectStreamReset(r2); + expectClientReset(0, r1, true); + + EXPECT_CALL(*this, onClientDestroy()); +} + // Verifies that requests are queued up in the conn pool and respect max request circuit breaking // when the connection is established. TEST_F(Http2ConnPoolImplTest, PendingRequestsRequestOverflow) { @@ -877,6 +904,7 @@ TEST_F(Http2ConnPoolImplTest, ConnectTimeout) { ActiveTestRequest r1(*this, 0, false); EXPECT_CALL(r1.callbacks_.pool_failure_, ready()); test_clients_[0].connect_timer_->invokeCallback(); + EXPECT_EQ(r1.callbacks_.reason_, ConnectionPool::PoolFailureReason::Timeout); EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 36e5eab6ddc72..03831607220a1 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -466,7 +466,7 @@ TEST_F(RouterTest, PoolFailureWithPriority) { EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) .WillOnce(Invoke([&](Http::StreamDecoder&, Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { - callbacks.onPoolFailure(Http::ConnectionPool::PoolFailureReason::ConnectionFailure, + callbacks.onPoolFailure(ConnectionPool::PoolFailureReason::RemoteConnectionFailure, absl::string_view(), cm_.conn_pool_.host_); return nullptr; })); @@ -1109,7 +1109,7 @@ TEST_F(RouterTest, EnvoyAttemptCountInResponsePresentWithLocalReply) { EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) .WillOnce(Invoke([&](Http::StreamDecoder&, Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { - callbacks.onPoolFailure(Http::ConnectionPool::PoolFailureReason::ConnectionFailure, + callbacks.onPoolFailure(ConnectionPool::PoolFailureReason::RemoteConnectionFailure, absl::string_view(), cm_.conn_pool_.host_); return nullptr; })); @@ -3075,7 +3075,7 @@ TEST_F(RouterTest, HedgingRetryImmediatelyReset) { EXPECT_CALL(*router_.retry_state_, onHostAttempted(_)); EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectFailed, _)); - callbacks.onPoolFailure(Http::ConnectionPool::PoolFailureReason::ConnectionFailure, + callbacks.onPoolFailure(ConnectionPool::PoolFailureReason::RemoteConnectionFailure, absl::string_view(), cm_.conn_pool_.host_); return nullptr; })); @@ -3312,7 +3312,7 @@ TEST_F(RouterTest, RetryUpstreamConnectionFailure) { router_.retry_state_->expectResetRetry(); - conn_pool_callbacks->onPoolFailure(Http::ConnectionPool::PoolFailureReason::ConnectionFailure, + conn_pool_callbacks->onPoolFailure(ConnectionPool::PoolFailureReason::RemoteConnectionFailure, absl::string_view(), nullptr); // Pool failure, so no upstream request was made. EXPECT_EQ(0U, diff --git a/test/common/tcp/conn_pool_test.cc b/test/common/tcp/conn_pool_test.cc index 32e0cf5c033e5..1470abc69c37c 100644 --- a/test/common/tcp/conn_pool_test.cc +++ b/test/common/tcp/conn_pool_test.cc @@ -50,7 +50,7 @@ struct ConnPoolCallbacks : public Tcp::ConnectionPool::Callbacks { pool_ready_.ready(); } - void onPoolFailure(Tcp::ConnectionPool::PoolFailureReason reason, + void onPoolFailure(ConnectionPool::PoolFailureReason reason, Upstream::HostDescriptionConstSharedPtr host) override { reason_ = reason; host_ = host; diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 34a92a7798cbf..a0f460d214e26 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -946,7 +946,7 @@ class TcpProxyTest : public testing::Test { } void raiseEventUpstreamConnectFailed(uint32_t conn_index, - Tcp::ConnectionPool::PoolFailureReason reason) { + ConnectionPool::PoolFailureReason reason) { conn_pool_callbacks_.at(conn_index)->onPoolFailure(reason, upstream_hosts_.at(conn_index)); } @@ -1059,8 +1059,7 @@ TEST_F(TcpProxyTest, DEPRECATED_FEATURE_TEST(ConnectAttemptsUpstreamLocalFail)) setup(2, config); - raiseEventUpstreamConnectFailed(0, - Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure); + raiseEventUpstreamConnectFailed(0, ConnectionPool::PoolFailureReason::LocalConnectionFailure); raiseEventUpstreamConnected(1); EXPECT_EQ(0U, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_store_ @@ -1077,8 +1076,8 @@ TEST_F(TcpProxyTest, DEPRECATED_FEATURE_TEST(ConnectAttemptsUpstreamLocalFailRee // This simulates a connection failure from under the stack of newStream. new_connection_functions_.push_back( [&](Tcp::ConnectionPool::Cancellable*) -> Tcp::ConnectionPool::Cancellable* { - raiseEventUpstreamConnectFailed( - 0, Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure); + raiseEventUpstreamConnectFailed(0, + ConnectionPool::PoolFailureReason::LocalConnectionFailure); return nullptr; }); @@ -1099,8 +1098,7 @@ TEST_F(TcpProxyTest, DEPRECATED_FEATURE_TEST(ConnectAttemptsUpstreamRemoteFail)) config.mutable_max_connect_attempts()->set_value(2); setup(2, config); - raiseEventUpstreamConnectFailed(0, - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + raiseEventUpstreamConnectFailed(0, ConnectionPool::PoolFailureReason::RemoteConnectionFailure); raiseEventUpstreamConnected(1); EXPECT_EQ(0U, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_store_ @@ -1114,7 +1112,7 @@ TEST_F(TcpProxyTest, DEPRECATED_FEATURE_TEST(ConnectAttemptsUpstreamTimeout)) { config.mutable_max_connect_attempts()->set_value(2); setup(2, config); - raiseEventUpstreamConnectFailed(0, Tcp::ConnectionPool::PoolFailureReason::Timeout); + raiseEventUpstreamConnectFailed(0, ConnectionPool::PoolFailureReason::Timeout); raiseEventUpstreamConnected(1); EXPECT_EQ(0U, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_store_ @@ -1139,11 +1137,9 @@ TEST_F(TcpProxyTest, DEPRECATED_FEATURE_TEST(ConnectAttemptsLimit)) { EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush)); // Try both failure modes - raiseEventUpstreamConnectFailed(0, Tcp::ConnectionPool::PoolFailureReason::Timeout); - raiseEventUpstreamConnectFailed(1, - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); - raiseEventUpstreamConnectFailed(2, - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + raiseEventUpstreamConnectFailed(0, ConnectionPool::PoolFailureReason::Timeout); + raiseEventUpstreamConnectFailed(1, ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + raiseEventUpstreamConnectFailed(2, ConnectionPool::PoolFailureReason::RemoteConnectionFailure); filter_.reset(); EXPECT_EQ(access_log_data_, "UF,URX"); @@ -1157,12 +1153,11 @@ TEST_F(TcpProxyTest, OutlierDetection) { EXPECT_CALL(upstream_hosts_.at(0)->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginTimeout, _)); - raiseEventUpstreamConnectFailed(0, Tcp::ConnectionPool::PoolFailureReason::Timeout); + raiseEventUpstreamConnectFailed(0, ConnectionPool::PoolFailureReason::Timeout); EXPECT_CALL(upstream_hosts_.at(1)->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectFailed, _)); - raiseEventUpstreamConnectFailed(1, - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + raiseEventUpstreamConnectFailed(1, ConnectionPool::PoolFailureReason::RemoteConnectionFailure); EXPECT_CALL(upstream_hosts_.at(2)->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectSuccessFinal, _)); @@ -1229,7 +1224,7 @@ TEST_F(TcpProxyTest, DEPRECATED_FEATURE_TEST(UpstreamConnectTimeout)) { setup(1, accessLogConfig("%RESPONSE_FLAGS%")); EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush)); - raiseEventUpstreamConnectFailed(0, Tcp::ConnectionPool::PoolFailureReason::Timeout); + raiseEventUpstreamConnectFailed(0, ConnectionPool::PoolFailureReason::Timeout); filter_.reset(); EXPECT_EQ(access_log_data_, "UF,URX"); @@ -1392,8 +1387,7 @@ TEST_F(TcpProxyTest, DEPRECATED_FEATURE_TEST(UpstreamConnectFailure)) { setup(1, accessLogConfig("%RESPONSE_FLAGS%")); EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush)); - raiseEventUpstreamConnectFailed(0, - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + raiseEventUpstreamConnectFailed(0, ConnectionPool::PoolFailureReason::RemoteConnectionFailure); filter_.reset(); EXPECT_EQ(access_log_data_, "UF,URX"); diff --git a/test/extensions/filters/network/dubbo_proxy/router_test.cc b/test/extensions/filters/network/dubbo_proxy/router_test.cc index e21148d3d7436..ad5f1d5b90049 100644 --- a/test/extensions/filters/network/dubbo_proxy/router_test.cc +++ b/test/extensions/filters/network/dubbo_proxy/router_test.cc @@ -247,7 +247,7 @@ TEST_F(DubboRouterTest, PoolRemoteConnectionFailure) { startRequest(MessageType::Request); context_.cluster_manager_.tcp_conn_pool_.poolFailure( - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + ConnectionPool::PoolFailureReason::RemoteConnectionFailure); } TEST_F(DubboRouterTest, PoolTimeout) { @@ -262,8 +262,7 @@ TEST_F(DubboRouterTest, PoolTimeout) { })); startRequest(MessageType::Request); - context_.cluster_manager_.tcp_conn_pool_.poolFailure( - Tcp::ConnectionPool::PoolFailureReason::Timeout); + context_.cluster_manager_.tcp_conn_pool_.poolFailure(ConnectionPool::PoolFailureReason::Timeout); } TEST_F(DubboRouterTest, PoolOverflowFailure) { @@ -278,8 +277,7 @@ TEST_F(DubboRouterTest, PoolOverflowFailure) { })); startRequest(MessageType::Request); - context_.cluster_manager_.tcp_conn_pool_.poolFailure( - Tcp::ConnectionPool::PoolFailureReason::Overflow); + context_.cluster_manager_.tcp_conn_pool_.poolFailure(ConnectionPool::PoolFailureReason::Overflow); } TEST_F(DubboRouterTest, ClusterMaintenanceMode) { @@ -334,7 +332,7 @@ TEST_F(DubboRouterTest, PoolConnectionFailureWithOnewayMessage) { EXPECT_EQ(FilterStatus::StopIteration, router_->onMessageDecoded(metadata_, message_context_)); context_.cluster_manager_.tcp_conn_pool_.poolFailure( - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + ConnectionPool::PoolFailureReason::RemoteConnectionFailure); destroyRouter(); } diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index ca3d8198a5bd2..ff7e57a5e14e2 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -368,7 +368,7 @@ TEST_F(ThriftRouterTest, PoolRemoteConnectionFailure) { EXPECT_TRUE(end_stream); })); context_.cluster_manager_.tcp_conn_pool_.poolFailure( - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + ConnectionPool::PoolFailureReason::RemoteConnectionFailure); } TEST_F(ThriftRouterTest, PoolLocalConnectionFailure) { @@ -377,7 +377,7 @@ TEST_F(ThriftRouterTest, PoolLocalConnectionFailure) { startRequest(MessageType::Call); context_.cluster_manager_.tcp_conn_pool_.poolFailure( - Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure); + ConnectionPool::PoolFailureReason::LocalConnectionFailure); } TEST_F(ThriftRouterTest, PoolTimeout) { @@ -392,8 +392,7 @@ TEST_F(ThriftRouterTest, PoolTimeout) { EXPECT_THAT(app_ex.what(), ContainsRegex(".*connection failure.*")); EXPECT_TRUE(end_stream); })); - context_.cluster_manager_.tcp_conn_pool_.poolFailure( - Tcp::ConnectionPool::PoolFailureReason::Timeout); + context_.cluster_manager_.tcp_conn_pool_.poolFailure(ConnectionPool::PoolFailureReason::Timeout); } TEST_F(ThriftRouterTest, PoolOverflowFailure) { @@ -408,8 +407,7 @@ TEST_F(ThriftRouterTest, PoolOverflowFailure) { EXPECT_THAT(app_ex.what(), ContainsRegex(".*too many connections.*")); EXPECT_TRUE(end_stream); })); - context_.cluster_manager_.tcp_conn_pool_.poolFailure( - Tcp::ConnectionPool::PoolFailureReason::Overflow); + context_.cluster_manager_.tcp_conn_pool_.poolFailure(ConnectionPool::PoolFailureReason::Overflow); } TEST_F(ThriftRouterTest, PoolConnectionFailureWithOnewayMessage) { @@ -419,7 +417,7 @@ TEST_F(ThriftRouterTest, PoolConnectionFailureWithOnewayMessage) { EXPECT_CALL(callbacks_, sendLocalReply(_, _)).Times(0); EXPECT_CALL(callbacks_, resetDownstreamConnection()); context_.cluster_manager_.tcp_conn_pool_.poolFailure( - Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure); + ConnectionPool::PoolFailureReason::RemoteConnectionFailure); destroyRouter(); } diff --git a/test/integration/tcp_conn_pool_integration_test.cc b/test/integration/tcp_conn_pool_integration_test.cc index 03dbc5f9530b8..5f1d14711e903 100644 --- a/test/integration/tcp_conn_pool_integration_test.cc +++ b/test/integration/tcp_conn_pool_integration_test.cc @@ -46,7 +46,7 @@ class TestFilter : public Network::ReadFilter { Request(TestFilter& parent, Buffer::Instance& data) : parent_(parent) { data_.move(data); } // Tcp::ConnectionPool::Callbacks - void onPoolFailure(Tcp::ConnectionPool::PoolFailureReason, + void onPoolFailure(ConnectionPool::PoolFailureReason, Upstream::HostDescriptionConstSharedPtr) override { ASSERT(false); }