diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 1987acbadeb8a..ec3b38d8353b7 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -72,6 +72,7 @@ Version history * tracing: added :ref:`verbose ` to support logging annotations on spans. * upstream: added support for host weighting and :ref:`locality weighting ` in the :ref:`ring hash load balancer `, and added a :ref:`maximum_ring_size` config parameter to strictly bound the ring size. * upstream: added configuration option to select any host when the fallback policy fails. +* upstream: stopped incrementing upstream_rq_total for HTTP/1 conn pool when request is circuit broken. 1.9.0 (Dec 20, 2018) ==================== diff --git a/source/common/http/http1/conn_pool.cc b/source/common/http/http1/conn_pool.cc index 139c814448495..119711fc1505a 100644 --- a/source/common/http/http1/conn_pool.cc +++ b/source/common/http/http1/conn_pool.cc @@ -66,6 +66,8 @@ bool ConnPoolImpl::hasActiveConnections() const { void ConnPoolImpl::attachRequestToClient(ActiveClient& client, StreamDecoder& response_decoder, ConnectionPool::Callbacks& callbacks) { ASSERT(!client.stream_wrapper_); + host_->cluster().stats().upstream_rq_total_.inc(); + host_->stats().rq_total_.inc(); client.stream_wrapper_ = std::make_unique(response_decoder, client); callbacks.onPoolReady(*client.stream_wrapper_, client.real_host_description_); } @@ -90,8 +92,6 @@ void ConnPoolImpl::createNewConnection() { ConnectionPool::Cancellable* ConnPoolImpl::newStream(StreamDecoder& response_decoder, ConnectionPool::Callbacks& callbacks) { - host_->cluster().stats().upstream_rq_total_.inc(); - host_->stats().rq_total_.inc(); if (!ready_clients_.empty()) { ready_clients_.front()->moveBetweenLists(ready_clients_, busy_clients_); ENVOY_CONN_LOG(debug, "using existing connection", *busy_clients_.front()->codec_client_); diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index 3d5ba3d40c73b..c66a2d9a41d55 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -169,7 +169,9 @@ struct ActiveTestRequest { parent.conn_pool_.test_clients_[client_index_].connection_->raiseEvent( Network::ConnectionEvent::Connected); } - EXPECT_EQ(current_rq_total + 1, parent_.cluster_->stats_.upstream_rq_total_.value()); + if (type != Type::Pending) { + EXPECT_EQ(current_rq_total + 1, parent_.cluster_->stats_.upstream_rq_total_.value()); + } } void completeResponse(bool with_body) { @@ -368,7 +370,7 @@ TEST_F(Http1ConnPoolImplTest, ConnectTimeout) { EXPECT_CALL(conn_pool_, onClientDestroy()).Times(2); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(2U, cluster_->stats_.upstream_rq_total_.value()); + EXPECT_EQ(0U, cluster_->stats_.upstream_rq_total_.value()); EXPECT_EQ(2U, cluster_->stats_.upstream_cx_connect_fail_.value()); EXPECT_EQ(2U, cluster_->stats_.upstream_cx_connect_timeout_.value()); } @@ -630,6 +632,7 @@ TEST_F(Http1ConnPoolImplTest, ConcurrentConnections) { r1.completeResponse(false); conn_pool_.expectAndRunUpstreamReady(); r3.startRequest(); + EXPECT_EQ(3U, cluster_->stats_.upstream_rq_total_.value()); r2.completeResponse(false); r3.completeResponse(false); @@ -651,6 +654,7 @@ TEST_F(Http1ConnPoolImplTest, DrainCallback) { ActiveTestRequest r1(*this, 0, ActiveTestRequest::Type::CreateConnection); ActiveTestRequest r2(*this, 0, ActiveTestRequest::Type::Pending); r2.handle_->cancel(); + EXPECT_EQ(1U, cluster_->stats_.upstream_rq_total_.value()); EXPECT_CALL(drained, ready()); r1.startRequest(); @@ -756,6 +760,7 @@ TEST_F(Http1ConnPoolImplTest, PendingRequestIsConsideredActive) { EXPECT_CALL(conn_pool_, onClientDestroy()); r1.handle_->cancel(); + EXPECT_EQ(0U, cluster_->stats_.upstream_rq_total_.value()); conn_pool_.drainConnections(); conn_pool_.test_clients_[0].connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); dispatcher_.clearDeferredDeleteList();