From 907052421744eaf3c949b04db147fb7d7b212164 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=96=B9=E4=BF=8A=E6=AD=A6?= Date: Wed, 17 May 2017 19:17:38 +0800 Subject: [PATCH 1/6] cluster max_concurrent_streams config --- include/envoy/upstream/upstream.h | 6 ++++++ source/common/http/http2/conn_pool.cc | 12 +++++++++++- source/common/http/http2/conn_pool.h | 4 ++-- source/common/json/config_schemas.cc | 5 +++++ source/common/upstream/upstream_impl.cc | 1 + source/common/upstream/upstream_impl.h | 2 ++ test/common/upstream/upstream_impl_test.cc | 2 ++ 7 files changed, 29 insertions(+), 3 deletions(-) diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index fbc2ce337d2e5..d46f6b1d43256 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -268,6 +268,12 @@ class ClusterInfo { */ virtual uint64_t maxRequestsPerConnection() const PURE; + /** + * @return int32_t the maximum number of concurrent http2 streams that a connection pool will + * hold on each upstream connection. 0 indicates default (1024), -1 no maximum. + */ + virtual int32_t maxConcurrentStreams() const PURE; + /** * @return the human readable name of the cluster. */ diff --git a/source/common/http/http2/conn_pool.cc b/source/common/http/http2/conn_pool.cc index 21a276c807e46..091214f10f35b 100644 --- a/source/common/http/http2/conn_pool.cc +++ b/source/common/http/http2/conn_pool.cc @@ -247,7 +247,17 @@ CodecClientPtr ProdConnPoolImpl::createCodecClient(Upstream::Host::CreateConnect return codec; } -uint64_t ProdConnPoolImpl::maxConcurrentStreams() { return ConnectionImpl::MAX_CONCURRENT_STREAMS; } +uint32_t ProdConnPoolImpl::maxConcurrentStreams() { + int32_t max_concurrent_streams = host_->cluster().maxConcurrentStreams(); + switch (max_concurrent_streams) { + case -1: + return maxTotalStreams(); + case 0: + return ConnectionImpl::MAX_CONCURRENT_STREAMS; + default: + return max_concurrent_streams; + } +} uint32_t ProdConnPoolImpl::maxTotalStreams() { return MAX_STREAMS; } diff --git a/source/common/http/http2/conn_pool.h b/source/common/http/http2/conn_pool.h index f188744260dc2..4abec7f6f3ee9 100644 --- a/source/common/http/http2/conn_pool.h +++ b/source/common/http/http2/conn_pool.h @@ -66,7 +66,7 @@ class ConnPoolImpl : Logger::Loggable, public ConnectionPool:: void checkForDrained(); virtual CodecClientPtr createCodecClient(Upstream::Host::CreateConnectionData& data) PURE; - virtual uint64_t maxConcurrentStreams() PURE; + virtual uint32_t maxConcurrentStreams() PURE; virtual uint32_t maxTotalStreams() PURE; void movePrimaryClientToDraining(); void onConnectionEvent(ActiveClient& client, uint32_t events); @@ -93,7 +93,7 @@ class ProdConnPoolImpl : public ConnPoolImpl { private: CodecClientPtr createCodecClient(Upstream::Host::CreateConnectionData& data) override; - uint64_t maxConcurrentStreams() override; + uint32_t maxConcurrentStreams() override; uint32_t maxTotalStreams() override; // All streams are 2^31. Client streams are half that, minus stream 0. Just to be on the safe diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index 48e758386345c..de61d485a9b80 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -1191,6 +1191,11 @@ const std::string Json::Schema::CLUSTER_SCHEMA(R"EOF( "minimum" : 0, "exclusiveMinimum" : true }, + "max_concurrent_streams" : { + "type" : "integer", + "minimum" : -1, + "exclusiveMinimum" : false + }, "circuit_breakers" : { "type" : "object", "properties" : { diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 72a0b5342a211..cc8ab3770995e 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -69,6 +69,7 @@ ClusterInfoImpl::ClusterInfoImpl(const Json::Object& config, Runtime::Loader& ru Stats::Store& stats, Ssl::ContextManager& ssl_context_manager) : runtime_(runtime), name_(config.getString("name")), max_requests_per_connection_(config.getInteger("max_requests_per_connection", 0)), + max_concurrent_streams_(config.getInteger("max_concurrent_streams", 0)), connect_timeout_(std::chrono::milliseconds(config.getInteger("connect_timeout_ms"))), per_connection_buffer_limit_bytes_( config.getInteger("per_connection_buffer_limit_bytes", 1024 * 1024)), diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index c340cd0a88b5f..5d5adeb20c163 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -180,6 +180,7 @@ class ClusterInfoImpl : public ClusterInfo { LoadBalancerType lbType() const override { return lb_type_; } bool maintenanceMode() const override; uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; } + int32_t maxConcurrentStreams() const override { return max_concurrent_streams_; } const std::string& name() const override { return name_; } ResourceManager& resourceManager(ResourcePriority priority) const override; Ssl::ClientContext* sslContext() const override { return ssl_ctx_.get(); } @@ -203,6 +204,7 @@ class ClusterInfoImpl : public ClusterInfo { Runtime::Loader& runtime_; const std::string name_; const uint64_t max_requests_per_connection_; + const int32_t max_concurrent_streams_; const std::chrono::milliseconds connect_timeout_; const uint32_t per_connection_buffer_limit_bytes_; Stats::ScopePtr stats_scope_; diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 19d0a5b7ce9a3..0fd3fffd31e10 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -127,6 +127,7 @@ TEST(StrictDnsClusterImplTest, Basic) { } }, "max_requests_per_connection": 3, + "max_concurrent_streams": 4, "http_codec_options": "no_compression", "hosts": [{"url": "tcp://localhost1:11001"}, {"url": "tcp://localhost2:11002"}] @@ -146,6 +147,7 @@ TEST(StrictDnsClusterImplTest, Basic) { EXPECT_EQ(3U, cluster.info()->resourceManager(ResourcePriority::High).requests().max()); EXPECT_EQ(4U, cluster.info()->resourceManager(ResourcePriority::High).retries().max()); EXPECT_EQ(3U, cluster.info()->maxRequestsPerConnection()); + EXPECT_EQ(4U, cluster.info()->maxConcurrentStreams()); EXPECT_EQ(static_cast(Http::CodecOptions::NoCompression), cluster.info()->httpCodecOptions()); From 16ce6022d0eaae0da13b61aa668bc98c8a8ca83a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=96=B9=E4=BF=8A=E6=AD=A6?= Date: Thu, 18 May 2017 11:16:38 +0800 Subject: [PATCH 2/6] Revert "cluster max_concurrent_streams config" This reverts commit 907052421744eaf3c949b04db147fb7d7b212164. --- include/envoy/upstream/upstream.h | 6 ------ source/common/http/http2/conn_pool.cc | 12 +----------- source/common/http/http2/conn_pool.h | 4 ++-- source/common/json/config_schemas.cc | 5 ----- source/common/upstream/upstream_impl.cc | 1 - source/common/upstream/upstream_impl.h | 2 -- test/common/upstream/upstream_impl_test.cc | 2 -- 7 files changed, 3 insertions(+), 29 deletions(-) diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index d46f6b1d43256..fbc2ce337d2e5 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -268,12 +268,6 @@ class ClusterInfo { */ virtual uint64_t maxRequestsPerConnection() const PURE; - /** - * @return int32_t the maximum number of concurrent http2 streams that a connection pool will - * hold on each upstream connection. 0 indicates default (1024), -1 no maximum. - */ - virtual int32_t maxConcurrentStreams() const PURE; - /** * @return the human readable name of the cluster. */ diff --git a/source/common/http/http2/conn_pool.cc b/source/common/http/http2/conn_pool.cc index 091214f10f35b..21a276c807e46 100644 --- a/source/common/http/http2/conn_pool.cc +++ b/source/common/http/http2/conn_pool.cc @@ -247,17 +247,7 @@ CodecClientPtr ProdConnPoolImpl::createCodecClient(Upstream::Host::CreateConnect return codec; } -uint32_t ProdConnPoolImpl::maxConcurrentStreams() { - int32_t max_concurrent_streams = host_->cluster().maxConcurrentStreams(); - switch (max_concurrent_streams) { - case -1: - return maxTotalStreams(); - case 0: - return ConnectionImpl::MAX_CONCURRENT_STREAMS; - default: - return max_concurrent_streams; - } -} +uint64_t ProdConnPoolImpl::maxConcurrentStreams() { return ConnectionImpl::MAX_CONCURRENT_STREAMS; } uint32_t ProdConnPoolImpl::maxTotalStreams() { return MAX_STREAMS; } diff --git a/source/common/http/http2/conn_pool.h b/source/common/http/http2/conn_pool.h index 4abec7f6f3ee9..f188744260dc2 100644 --- a/source/common/http/http2/conn_pool.h +++ b/source/common/http/http2/conn_pool.h @@ -66,7 +66,7 @@ class ConnPoolImpl : Logger::Loggable, public ConnectionPool:: void checkForDrained(); virtual CodecClientPtr createCodecClient(Upstream::Host::CreateConnectionData& data) PURE; - virtual uint32_t maxConcurrentStreams() PURE; + virtual uint64_t maxConcurrentStreams() PURE; virtual uint32_t maxTotalStreams() PURE; void movePrimaryClientToDraining(); void onConnectionEvent(ActiveClient& client, uint32_t events); @@ -93,7 +93,7 @@ class ProdConnPoolImpl : public ConnPoolImpl { private: CodecClientPtr createCodecClient(Upstream::Host::CreateConnectionData& data) override; - uint32_t maxConcurrentStreams() override; + uint64_t maxConcurrentStreams() override; uint32_t maxTotalStreams() override; // All streams are 2^31. Client streams are half that, minus stream 0. Just to be on the safe diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index de61d485a9b80..48e758386345c 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -1191,11 +1191,6 @@ const std::string Json::Schema::CLUSTER_SCHEMA(R"EOF( "minimum" : 0, "exclusiveMinimum" : true }, - "max_concurrent_streams" : { - "type" : "integer", - "minimum" : -1, - "exclusiveMinimum" : false - }, "circuit_breakers" : { "type" : "object", "properties" : { diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index cc8ab3770995e..72a0b5342a211 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -69,7 +69,6 @@ ClusterInfoImpl::ClusterInfoImpl(const Json::Object& config, Runtime::Loader& ru Stats::Store& stats, Ssl::ContextManager& ssl_context_manager) : runtime_(runtime), name_(config.getString("name")), max_requests_per_connection_(config.getInteger("max_requests_per_connection", 0)), - max_concurrent_streams_(config.getInteger("max_concurrent_streams", 0)), connect_timeout_(std::chrono::milliseconds(config.getInteger("connect_timeout_ms"))), per_connection_buffer_limit_bytes_( config.getInteger("per_connection_buffer_limit_bytes", 1024 * 1024)), diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 5d5adeb20c163..c340cd0a88b5f 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -180,7 +180,6 @@ class ClusterInfoImpl : public ClusterInfo { LoadBalancerType lbType() const override { return lb_type_; } bool maintenanceMode() const override; uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; } - int32_t maxConcurrentStreams() const override { return max_concurrent_streams_; } const std::string& name() const override { return name_; } ResourceManager& resourceManager(ResourcePriority priority) const override; Ssl::ClientContext* sslContext() const override { return ssl_ctx_.get(); } @@ -204,7 +203,6 @@ class ClusterInfoImpl : public ClusterInfo { Runtime::Loader& runtime_; const std::string name_; const uint64_t max_requests_per_connection_; - const int32_t max_concurrent_streams_; const std::chrono::milliseconds connect_timeout_; const uint32_t per_connection_buffer_limit_bytes_; Stats::ScopePtr stats_scope_; diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 0fd3fffd31e10..19d0a5b7ce9a3 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -127,7 +127,6 @@ TEST(StrictDnsClusterImplTest, Basic) { } }, "max_requests_per_connection": 3, - "max_concurrent_streams": 4, "http_codec_options": "no_compression", "hosts": [{"url": "tcp://localhost1:11001"}, {"url": "tcp://localhost2:11002"}] @@ -147,7 +146,6 @@ TEST(StrictDnsClusterImplTest, Basic) { EXPECT_EQ(3U, cluster.info()->resourceManager(ResourcePriority::High).requests().max()); EXPECT_EQ(4U, cluster.info()->resourceManager(ResourcePriority::High).retries().max()); EXPECT_EQ(3U, cluster.info()->maxRequestsPerConnection()); - EXPECT_EQ(4U, cluster.info()->maxConcurrentStreams()); EXPECT_EQ(static_cast(Http::CodecOptions::NoCompression), cluster.info()->httpCodecOptions()); From 3c58b912a88d80c81ebf09035579ad4cb280580e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=96=B9=E4=BF=8A=E6=AD=A6?= Date: Thu, 18 May 2017 11:18:57 +0800 Subject: [PATCH 3/6] Remove MAX_STREAMS check on client side connection pool. (#963) The reason for this is that we already have protection via circuit breakers. There is no point in having additional protection (other than potentially for push, which we don't support currently, and can deal with later). If we overflow the other side, the code will correctly handle the REFUSED_STREAM reset that happens. --- source/common/http/http2/conn_pool.cc | 27 +++++++++------------------ source/common/http/http2/conn_pool.h | 2 -- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/source/common/http/http2/conn_pool.cc b/source/common/http/http2/conn_pool.cc index 21a276c807e46..42b26904e5e30 100644 --- a/source/common/http/http2/conn_pool.cc +++ b/source/common/http/http2/conn_pool.cc @@ -82,22 +82,15 @@ ConnectionPool::Cancellable* ConnPoolImpl::newStream(Http::StreamDecoder& respon primary_client_.reset(new ActiveClient(*this)); } - if (primary_client_->client_->numActiveRequests() >= maxConcurrentStreams() || - !host_->cluster().resourceManager(priority_).requests().canCreate()) { - log_debug("max requests overflow"); - callbacks.onPoolFailure(ConnectionPool::PoolFailureReason::Overflow, nullptr); - host_->cluster().stats().upstream_rq_pending_overflow_.inc(); - } else { - conn_log_debug("creating stream", *primary_client_->client_); - primary_client_->total_streams_++; - host_->stats().rq_total_.inc(); - host_->stats().rq_active_.inc(); - host_->cluster().stats().upstream_rq_total_.inc(); - host_->cluster().stats().upstream_rq_active_.inc(); - host_->cluster().resourceManager(priority_).requests().inc(); - callbacks.onPoolReady(primary_client_->client_->newStream(response_decoder), - primary_client_->real_host_description_); - } + conn_log_debug("creating stream", *primary_client_->client_); + primary_client_->total_streams_++; + host_->stats().rq_total_.inc(); + host_->stats().rq_active_.inc(); + host_->cluster().stats().upstream_rq_total_.inc(); + host_->cluster().stats().upstream_rq_active_.inc(); + host_->cluster().resourceManager(priority_).requests().inc(); + callbacks.onPoolReady(primary_client_->client_->newStream(response_decoder), + primary_client_->real_host_description_); return nullptr; } @@ -247,8 +240,6 @@ CodecClientPtr ProdConnPoolImpl::createCodecClient(Upstream::Host::CreateConnect return codec; } -uint64_t ProdConnPoolImpl::maxConcurrentStreams() { return ConnectionImpl::MAX_CONCURRENT_STREAMS; } - uint32_t ProdConnPoolImpl::maxTotalStreams() { return MAX_STREAMS; } } // Http2 diff --git a/source/common/http/http2/conn_pool.h b/source/common/http/http2/conn_pool.h index f188744260dc2..bb66b5430a4fb 100644 --- a/source/common/http/http2/conn_pool.h +++ b/source/common/http/http2/conn_pool.h @@ -66,7 +66,6 @@ class ConnPoolImpl : Logger::Loggable, public ConnectionPool:: void checkForDrained(); virtual CodecClientPtr createCodecClient(Upstream::Host::CreateConnectionData& data) PURE; - virtual uint64_t maxConcurrentStreams() PURE; virtual uint32_t maxTotalStreams() PURE; void movePrimaryClientToDraining(); void onConnectionEvent(ActiveClient& client, uint32_t events); @@ -93,7 +92,6 @@ class ProdConnPoolImpl : public ConnPoolImpl { private: CodecClientPtr createCodecClient(Upstream::Host::CreateConnectionData& data) override; - uint64_t maxConcurrentStreams() override; uint32_t maxTotalStreams() override; // All streams are 2^31. Client streams are half that, minus stream 0. Just to be on the safe From 6aeda69fdfacffaf3e84ecd9c26e14d3a1061c10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=96=B9=E4=BF=8A=E6=AD=A6?= Date: Thu, 18 May 2017 11:18:57 +0800 Subject: [PATCH 4/6] Remove MAX_STREAMS check on client side connection pool. (#963) The reason for this is that we already have protection via circuit breakers. There is no point in having additional protection (other than potentially for push, which we don't support currently, and can deal with later). If we overflow the other side, the code will correctly handle the REFUSED_STREAM reset that happens. --- source/common/http/http2/conn_pool.cc | 27 ++++++++---------------- source/common/http/http2/conn_pool.h | 2 -- test/common/http/http2/conn_pool_test.cc | 1 - 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/source/common/http/http2/conn_pool.cc b/source/common/http/http2/conn_pool.cc index 21a276c807e46..42b26904e5e30 100644 --- a/source/common/http/http2/conn_pool.cc +++ b/source/common/http/http2/conn_pool.cc @@ -82,22 +82,15 @@ ConnectionPool::Cancellable* ConnPoolImpl::newStream(Http::StreamDecoder& respon primary_client_.reset(new ActiveClient(*this)); } - if (primary_client_->client_->numActiveRequests() >= maxConcurrentStreams() || - !host_->cluster().resourceManager(priority_).requests().canCreate()) { - log_debug("max requests overflow"); - callbacks.onPoolFailure(ConnectionPool::PoolFailureReason::Overflow, nullptr); - host_->cluster().stats().upstream_rq_pending_overflow_.inc(); - } else { - conn_log_debug("creating stream", *primary_client_->client_); - primary_client_->total_streams_++; - host_->stats().rq_total_.inc(); - host_->stats().rq_active_.inc(); - host_->cluster().stats().upstream_rq_total_.inc(); - host_->cluster().stats().upstream_rq_active_.inc(); - host_->cluster().resourceManager(priority_).requests().inc(); - callbacks.onPoolReady(primary_client_->client_->newStream(response_decoder), - primary_client_->real_host_description_); - } + conn_log_debug("creating stream", *primary_client_->client_); + primary_client_->total_streams_++; + host_->stats().rq_total_.inc(); + host_->stats().rq_active_.inc(); + host_->cluster().stats().upstream_rq_total_.inc(); + host_->cluster().stats().upstream_rq_active_.inc(); + host_->cluster().resourceManager(priority_).requests().inc(); + callbacks.onPoolReady(primary_client_->client_->newStream(response_decoder), + primary_client_->real_host_description_); return nullptr; } @@ -247,8 +240,6 @@ CodecClientPtr ProdConnPoolImpl::createCodecClient(Upstream::Host::CreateConnect return codec; } -uint64_t ProdConnPoolImpl::maxConcurrentStreams() { return ConnectionImpl::MAX_CONCURRENT_STREAMS; } - uint32_t ProdConnPoolImpl::maxTotalStreams() { return MAX_STREAMS; } } // Http2 diff --git a/source/common/http/http2/conn_pool.h b/source/common/http/http2/conn_pool.h index f188744260dc2..bb66b5430a4fb 100644 --- a/source/common/http/http2/conn_pool.h +++ b/source/common/http/http2/conn_pool.h @@ -66,7 +66,6 @@ class ConnPoolImpl : Logger::Loggable, public ConnectionPool:: void checkForDrained(); virtual CodecClientPtr createCodecClient(Upstream::Host::CreateConnectionData& data) PURE; - virtual uint64_t maxConcurrentStreams() PURE; virtual uint32_t maxTotalStreams() PURE; void movePrimaryClientToDraining(); void onConnectionEvent(ActiveClient& client, uint32_t events); @@ -93,7 +92,6 @@ class ProdConnPoolImpl : public ConnPoolImpl { private: CodecClientPtr createCodecClient(Upstream::Host::CreateConnectionData& data) override; - uint64_t maxConcurrentStreams() override; uint32_t maxTotalStreams() override; // All streams are 2^31. Client streams are half that, minus stream 0. Just to be on the safe diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index 1ca1eb0973e8f..7e5e0721098f2 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -43,7 +43,6 @@ class TestConnPoolImpl : public ConnPoolImpl { MOCK_METHOD1(createCodecClient_, CodecClient*(Upstream::Host::CreateConnectionData& data)); - uint64_t maxConcurrentStreams() override { return max_concurrent_streams_; } uint32_t maxTotalStreams() override { return max_streams_; } uint64_t max_concurrent_streams_{std::numeric_limits::max()}; From 9e6cad885725ac94b590174aad0776c341660d7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=96=B9=E4=BF=8A=E6=AD=A6?= Date: Thu, 18 May 2017 15:07:31 +0800 Subject: [PATCH 5/6] Fix test. it seems the whole TEST_F(Http2ConnPoolImplTest, MaxRequests) is for the max_concurrent_request thing, so i delete it. --- test/common/http/http2/conn_pool_test.cc | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index 50ed7b0d7e816..c94ea6e6c38cd 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -354,25 +354,6 @@ TEST_F(Http2ConnPoolImplTest, ConnectTimeout) { EXPECT_EQ(1U, cluster_->stats_.upstream_rq_pending_failure_eject_.value()); } -TEST_F(Http2ConnPoolImplTest, MaxRequests) { - InSequence s; - - expectClientCreate(); - ActiveTestRequest r1(*this, 0); - EXPECT_CALL(r1.inner_encoder_, encodeHeaders(_, true)); - r1.callbacks_.outer_encoder_->encodeHeaders(HeaderMapImpl{}, true); - expectClientConnect(0); - - ConnPoolCallbacks callbacks; - Http::MockStreamDecoder decoder; - EXPECT_CALL(callbacks.pool_failure_, ready()); - EXPECT_EQ(nullptr, pool_.newStream(decoder, callbacks)); - - test_clients_[0].connection_->raiseEvents(Network::ConnectionEvent::RemoteClose); - EXPECT_CALL(*this, onClientDestroy()); - dispatcher_.clearDeferredDeleteList(); -} - TEST_F(Http2ConnPoolImplTest, MaxGlobalRequests) { InSequence s; cluster_->resource_manager_.reset( From 6b0d74fc87d83bc8bc64996da30789abd2c201ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=96=B9=E4=BF=8A=E6=AD=A6?= Date: Thu, 18 May 2017 18:39:58 +0800 Subject: [PATCH 6/6] add back resource manager check --- source/common/http/http2/conn_pool.cc | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/source/common/http/http2/conn_pool.cc b/source/common/http/http2/conn_pool.cc index 42b26904e5e30..bacef3b336394 100644 --- a/source/common/http/http2/conn_pool.cc +++ b/source/common/http/http2/conn_pool.cc @@ -82,15 +82,21 @@ ConnectionPool::Cancellable* ConnPoolImpl::newStream(Http::StreamDecoder& respon primary_client_.reset(new ActiveClient(*this)); } - conn_log_debug("creating stream", *primary_client_->client_); - primary_client_->total_streams_++; - host_->stats().rq_total_.inc(); - host_->stats().rq_active_.inc(); - host_->cluster().stats().upstream_rq_total_.inc(); - host_->cluster().stats().upstream_rq_active_.inc(); - host_->cluster().resourceManager(priority_).requests().inc(); - callbacks.onPoolReady(primary_client_->client_->newStream(response_decoder), - primary_client_->real_host_description_); + if (!host_->cluster().resourceManager(priority_).requests().canCreate()) { + log_debug("max requests overflow"); + callbacks.onPoolFailure(ConnectionPool::PoolFailureReason::Overflow, nullptr); + host_->cluster().stats().upstream_rq_pending_overflow_.inc(); + } else { + conn_log_debug("creating stream", *primary_client_->client_); + primary_client_->total_streams_++; + host_->stats().rq_total_.inc(); + host_->stats().rq_active_.inc(); + host_->cluster().stats().upstream_rq_total_.inc(); + host_->cluster().stats().upstream_rq_active_.inc(); + host_->cluster().resourceManager(priority_).requests().inc(); + callbacks.onPoolReady(primary_client_->client_->newStream(response_decoder), + primary_client_->real_host_description_); + } return nullptr; }