Combine H1 and H2 conn_pool implementations

api/envoy/api/v2/core/protocol.proto

@@ -117,6 +117,10 @@ message Http2ProtocolOptions {
   // `Maximum concurrent streams <https://httpwg.org/specs/rfc7540.html#rfc.section.5.1.2>`_
   // allowed for peer on one HTTP/2 connection. Valid values range from 1 to 2147483647 (2^31 - 1)
   // and defaults to 2147483647.
+  //
+  // For upstream connections, this also limits how many streams Envoy will initiate concurrently
+  // on a single connection. If the limit is reached, Envoy may queue requests or establish
+  // additional connections (as allowed per circuit breaker limits).
   google.protobuf.UInt32Value max_concurrent_streams = 2
       [(validate.rules).uint32 = {lte: 2147483647 gte: 1}];
 

api/envoy/config/core/v3/protocol.proto

@@ -136,6 +136,10 @@ message Http2ProtocolOptions {
   // `Maximum concurrent streams <https://httpwg.org/specs/rfc7540.html#rfc.section.5.1.2>`_
   // allowed for peer on one HTTP/2 connection. Valid values range from 1 to 2147483647 (2^31 - 1)
   // and defaults to 2147483647.
+  //
+  // For upstream connections, this also limits how many streams Envoy will initiate concurrently
+  // on a single connection. If the limit is reached, Envoy may queue requests or establish
+  // additional connections (as allowed per circuit breaker limits).
   google.protobuf.UInt32Value max_concurrent_streams = 2
       [(validate.rules).uint32 = {lte: 2147483647 gte: 1}];
 

docs/root/intro/arch_overview/upstream/circuit_breaking.rst

@@ -12,23 +12,20 @@ configure and code each application independently. Envoy supports various types
 .. _arch_overview_circuit_break_cluster_maximum_connections:
 
 * **Cluster maximum connections**: The maximum number of connections that Envoy will establish to
-  all hosts in an upstream cluster. In practice this is only applicable to HTTP/1.1 clusters since
-  HTTP/2 uses a single connection to each host. If this circuit breaker overflows the :ref:`upstream_cx_overflow
+  all hosts in an upstream cluster. If this circuit breaker overflows the :ref:`upstream_cx_overflow
   <config_cluster_manager_cluster_stats>` counter for the cluster will increment.
 * **Cluster maximum pending requests**: The maximum number of requests that will be queued while
-  waiting for a ready connection pool connection. Since HTTP/2 requests are sent over a single
-  connection, this circuit breaker only comes into play as the initial connection is created,
-  as requests will be multiplexed immediately afterwards. For HTTP/1.1, requests are added to the list
+  waiting for a ready connection pool connection. Requests are added to the list
   of pending requests whenever there aren't enough upstream connections available to immediately dispatch
-  the request, so this circuit breaker will remain in play for the lifetime of the process.
-  If this circuit breaker overflows the
+  the request. For HTTP/2 connections, if :ref:`max concurrent streams <envoy_api_field_core.Http2ProtocolOptions.max_concurrent_streams>`
+  and :ref:`max requests per connection <envoy_api_field_Cluster.max_requests_per_connection>` are not
+  configured, all requests will be multiplexed over the same connection so this circuit breaker
+  will only be hit when no connection is already established. If this circuit breaker overflows the
   :ref:`upstream_rq_pending_overflow <config_cluster_manager_cluster_stats>` counter for the cluster will
   increment.
 * **Cluster maximum requests**: The maximum number of requests that can be outstanding to all hosts
-  in a cluster at any given time. In practice this is applicable to HTTP/2 clusters since HTTP/1.1
-  clusters are governed by the maximum connections circuit breaker. If this circuit breaker
-  overflows the :ref:`upstream_rq_pending_overflow <config_cluster_manager_cluster_stats>` counter
-  for the cluster will increment.
+  in a cluster at any given time. If this circuit breaker overflows the :ref:`upstream_rq_pending_overflow <config_cluster_manager_cluster_stats>`
+  counter for the cluster will increment.
 * **Cluster maximum active retries**: The maximum number of retries that can be outstanding to all
   hosts in a cluster at any given time. In general we recommend using :ref:`retry budgets <envoy_api_field_cluster.CircuitBreakers.Thresholds.retry_budget>`; however, if static circuit breaking is preferred it should aggressively circuit break
   retries. This is so that retries for sporadic failures are allowed, but the overall retry volume cannot

docs/root/intro/arch_overview/upstream/connection_pooling.rst

@@ -20,9 +20,14 @@ downstream request must be reset if the upstream connection is severed.
 HTTP/2
 ------
 
-The HTTP/2 connection pool acquires a single connection to an upstream host. All requests are
-multiplexed over this connection. If a GOAWAY frame is received or if the connection reaches the
-maximum stream limit, the connection pool will create a new connection and drain the existing one.
+The HTTP/2 connection pool multiplexes multiple requests over a single connection, up to the limits
+imposed by :ref:`max concurrent streams <envoy_api_field_core.Http2ProtocolOptions.max_concurrent_streams>`
+and :ref:`max requests per connection <envoy_api_field_Cluster.max_requests_per_connection>`.
+The HTTP/2 connection pool establishes only as many connections as are needed to serve the current
+requests. With no limits, this will be only a single connection. If a GOAWAY frame is received or
+if the connection reaches the maximum stream limit, the connection pool will drain the existing one.
+New connections are established anytime there is a pending request without a connection that it can
+be dispatched to (up to circuit breaker limits for connections).
 HTTP/2 is the preferred communication protocol as connections rarely if ever get severed.
 
 .. _arch_overview_conn_pool_health_checking:

docs/root/intro/deprecated.rst

@@ -12,6 +12,9 @@ Deprecated items below are listed in chronological order.
 
 1.14.0 (Pending)
 ================
+* The previous behavior for upstream connection pool circuit breaking described
+  `here <https://www.envoyproxy.io/docs/envoy/v1.13.0/intro/arch_overview/upstream/circuit_breaking>`_ has
+  been deprecated in favor of the new behavior described :ref:`here <arch_overview_circuit_break>`.
 
 1.13.0 (January 20, 2020)
 =========================

docs/root/intro/version_history.rst

@@ -5,6 +5,14 @@ Version history
 ================
 * config: use type URL to select an extension whenever the config type URL (or its previous versions) uniquely identify a typed extension, see :ref:`extension configuration <config_overview_extension_configuration>`.
 * retry: added a retry predicate that :ref:`rejects hosts based on metadata. <envoy_api_field_route.RetryPolicy.retry_host_predicate>`
+* upstream: combined HTTP/1 and HTTP/2 connection pool code. This means that circuit breaker
+  limits for both requests and connections apply to both pool types. Also, HTTP/2 now has
+  the option to limit concurrent requests on a connection, and allow multiple draining
+  connections. The old behavior is deprecated, but can be used during the deprecation
+  period by disabling runtime feature "envoy.reloadable_features.new_http1_connection_pool_behavior" or
+  "envoy.reloadable_features.new_http2_connection_pool_behavior" and then re-configure your clusters or
+  restart Envoy. The behavior will not switch until the connection pools are recreated. The new
+  circuit breaker behavior is described :ref:`here <arch_overview_circuit_break>`.
 * upstream: changed load distribution algorithm when all priorities enter :ref:`panic mode<arch_overview_load_balancing_panic_threshold>`. 
 
 1.13.0 (January 20, 2020)

source/common/common/linked_object.h

@@ -28,15 +28,15 @@ template <class T> class LinkedObject {
   bool inserted() { return inserted_; }
 
   /**
-   * Move a linked item between 2 lists.
-   * @param list1 supplies the first list.
-   * @param list2 supplies the second list.
+   * Move a linked item from src list to dst list.
+   * @param src supplies the list that the item is currently in.
+   * @param dst supplies the destination list for the item.
    */
-  void moveBetweenLists(ListType& list1, ListType& list2) {
+  void moveBetweenLists(ListType& src, ListType& dst) {
     ASSERT(inserted_);
-    ASSERT(std::find(list1.begin(), list1.end(), *entry_) != list1.end());
+    ASSERT(std::find(src.begin(), src.end(), *entry_) != src.end());
 
-    list2.splice(list2.begin(), list1, entry_);
+    dst.splice(dst.begin(), src, entry_);
   }
 
   /**

source/common/http/BUILD

@@ -114,6 +114,20 @@ envoy_cc_library(
     name = "conn_pool_base_lib",
     srcs = ["conn_pool_base.cc"],
     hdrs = ["conn_pool_base.h"],
+    deps = [
+        ":codec_client_lib",
+        "//include/envoy/http:conn_pool_interface",
+        "//include/envoy/stats:timespan_interface",
+        "//source/common/common:linked_object",
+        "//source/common/stats:timespan_lib",
+        "//source/common/upstream:upstream_lib",
+    ],
+)
+
+envoy_cc_library(
+    name = "conn_pool_base_legacy_lib",
+    srcs = ["conn_pool_base_legacy.cc"],
+    hdrs = ["conn_pool_base_legacy.h"],
     deps = [
         "//include/envoy/http:conn_pool_interface",
         "//include/envoy/stats:timespan_interface",