conn pool: fixed a bug with mixed connection pool capacity (envoyproxy#39349)

Fixed an issue that could lead to too many connections when using
:ref:`AutoHttpConfig
<envoy_v3_api_msg_extensions.upstreams.http.v3.HttpProtocolOptions.AutoHttpConfig>`
if the
established connection is ``http/2`` and Envoy predicted it would have
lower concurrent capacity.

Signed-off-by: Greg Greenway <[email protected]>

Author: ggreenway

changelogs/current.yaml

@@ -36,6 +36,11 @@ minor_behavior_changes:
 
 bug_fixes:
 # *Changes expected to improve the state of the world and are unlikely to have negative effects*
+- area: conn_pool
+  change: |
+    Fixed an issue that could lead to too many connections when using
+    :ref:`AutoHttpConfig <envoy_v3_api_msg_extensions.upstreams.http.v3.HttpProtocolOptions.AutoHttpConfig>` if the
+    established connection is ``http/2`` and Envoy predicted it would have lower concurrent capacity.
 - area: conn_pool
   change: |
     Fixed an issue that could lead to insufficient connections for current pending requests. If a connection starts draining while it

source/common/http/mixed_conn_pool.cc

@@ -86,8 +86,8 @@ void HttpConnPoolImplMixed::onConnected(Envoy::ConnectionPool::ActiveClient& cli
   // it to reflect any difference between the TCP stream limits and HTTP/2
   // stream limits.
   if (new_client->effectiveConcurrentStreamLimit() > old_effective_limit) {
-    state_.incrConnectingAndConnectedStreamCapacity(new_client->effectiveConcurrentStreamLimit() -
-                                                    old_effective_limit);
+    incrConnectingAndConnectedStreamCapacity(
+        new_client->effectiveConcurrentStreamLimit() - old_effective_limit, client);
   }
   new_client->setState(ActiveClient::State::Connecting);
   LinkedList::moveIntoList(std::move(new_client), owningList(new_client->state()));

test/common/http/mixed_conn_pool_test.cc

@@ -114,6 +114,18 @@ TEST_F(MixedConnPoolImplTest, HandshakeWithCachedLimit) {
   testAlpnHandshake({});
 }
 
+// Test that increasing the limit upon connect, versus what was in the cache before connection,
+// works correctly.
+TEST_F(MixedConnPoolImplTest, HandshakeWithCachedLimitAndEffectiveIncrease) {
+  expected_capacity_ = 1;
+
+  // This simulates a previous connection being http 1.
+  EXPECT_CALL(mock_cache_, getConcurrentStreams(_)).WillOnce(Return(1));
+
+  // This makes the new connection http 2, which has more than 1 stream available.
+  testAlpnHandshake(Protocol::Http2);
+}
+
 TEST_F(MixedConnPoolImplTest, HandshakeWithCachedLimitCapped) {
   EXPECT_CALL(mock_cache_, getConcurrentStreams(_))
       .WillOnce(Return(std::numeric_limits<uint32_t>::max()));