Allow multiplexed upstream servers to half close the stream before th…

…e downstream (#34461)

Commit Message:
Allow HTTP/2 (and HTTP/3) upstream servers to half close the stream
before the downstream. This enables bidirectional gRPC streams where
server completes streaming before the client. Behavior of HTTP/1 or TCP
proxy upstream servers is unchanged and the stream is reset if the
upstream server completes response before the downstream. The stream is
also reset if the upstream server responds with an error status before
the downstream. This behavior is disabled by default and can be enabled
by setting the
``envoy.reloadable_features.allow_multiplexed_upstream_half_close``
runtime key to true.

Change details:
Presently there are two places where the stream was reset when upstream
half-closed.
1. In the router filter's `Filter::onUpstreamComplete` method, which
covers HTTP upstreams.
2. In the filter manager's `FilterManager::commonEncodePrefix` which
covers local reply's by filters and TCP upstreams.

When the
``envoy.reloadable_features.allow_multiplexed_upstream_half_close`` is
enabled the router filter no longer forces reset in the
`Filter::onUpstreamComplete` and allows fully independent half closes.
To preserve existing half close behavior of HTTP/1 protocol the force
reset is added in the H/1 codec's
`ClientConnectionImpl::onMessageCompleteBase()` method.

When the
``envoy.reloadable_features.allow_multiplexed_upstream_half_close`` is
enabled the filter manager closes stream after both decoder and encoder
filter chain complete, except in two cases:
1. local reply - behaves the same.
2. error (non 1xx or 2xx) response from the server. This case is handled
in the `FilterManager::checkAndCloseStreamIfFullyClosed()` method.

The `state_.decoder_filter_chain_complete_` is added to track completion
of the decoder filter chain.

To preserver behavior for TCP upstream the force reset is moved into the
`TcpUpstream::onUpstreamData` method.

Risk Level: High (flag protected, disabled by default)
Testing: Unit Tests
Docs Changes: No
Release Notes: Yes
Platform Specific Features: N/A
Runtime guard:
envoy.reloadable_features.allow_multiplexed_upstream_half_close
Fixes #30149

---------

Signed-off-by: Yan Avlasov <[email protected]>

changelogs/current.yaml

@@ -10,6 +10,13 @@ behavior_changes:
   change: |
     Change ``OnLogDownstreamStart``, ``OnLogDownstreamPeriodic`` and ``OnLog`` methods so that user can get the request/response's
     headers and trailers when producing access log.
+- area: http
+  change: |
+    Allow HTTP/2 (and HTTP/3) upstream servers to half close the stream before the downstream. This enables bidirectional
+    gRPC streams where server completes streaming before the client. Behavior of HTTP/1 or TCP proxy upstream servers is
+    unchanged and the stream is reset if the upstream server completes response before the downstream. The stream is also
+    reset if the upstream server responds with an error status before the downstream. This behavior is disabled by default
+    and can be enabled by setting the ``envoy.reloadable_features.allow_multiplexed_upstream_half_close`` runtime key to true.
 - area: http
   change: |
     Added HTTP1-safe option for :ref:`max_connection_duration

docs/root/configuration/observability/access_log/usage.rst

@@ -428,6 +428,10 @@ The following command operators are supported:
     * ``us``: Microsecond precision.
     * ``ns``: Nanosecond precision.
 
+    NOTE: enabling independent half-close behavior for H/2 and H/3 protocols can produce
+    ``*_TX_END`` values lower than ``*_RX_END`` values, in cases where upstream peer has half-closed
+    its stream before downstream peer. In these cases ``COMMON_DURATION`` value will become negative.
+
   TCP/UDP
     Not implemented ("-").
 

docs/root/intro/arch_overview/http/http_connection_management.rst

@@ -36,6 +36,26 @@ In the case of HTTP/1.1, the codec translates the serial/pipelining capabilities
 that looks like HTTP/2 to higher layers. This means that the majority of the code does not need to
 understand whether a stream originated on an HTTP/1.1, HTTP/2, or HTTP/3 connection.
 
+HTTP lifecycle
+--------------
+
+Proxying of the request begins when the downstream HTTP codec has successfully decoded request header map.
+
+The point at which the proxying completes and the stream is destroyed depends on the upstream protocol and
+whether independent half close is enabled.
+
+If independent half-close is enabled and the upstream protocol is either HTTP/2 or HTTP/3 protocols the stream
+is destroyed after both request and response are complete i.e. reach their respective end-of-stream,
+by receiving trailers or the header/body with end-stream set in both directions AND response has
+success (2xx) status code.
+
+For HTTP/1 upstream protocol or if independent half-close is disabled the stream is destroyed when the response
+is complete and reaches its end-of-stream, i.e. when trailers or the response header/body with
+end-stream set are received, even if the request has not yet completed. If the request was incomplete at response
+completion, the stream is reset.
+
+Note that proxying can stop early when an error or timeout occurred or when a peer reset HTTP/2 or HTTP/3 stream.
+
 HTTP header sanitizing
 ----------------------
 

docs/root/intro/life_of_a_request.rst

@@ -212,8 +212,11 @@ A brief outline of the life cycle of a request and response using the example co
     :ref:`opposite order <arch_overview_http_filters_ordering>` from the request, starting at the
     codec filter, traversing any upstream HTTP filters, then going through the router filter and passing
     through CustomFilter, before being sent downstream.
-12. When the response is complete, the stream is destroyed. Post-request processing will update
-    stats, write to the access log and finalize trace spans.
+12. If independent half-close is enabled the stream is destroyed after both request and response are
+    complete (END_STREAM for the HTTP/2 stream is observed in both directions) AND response has success
+    (2xx) status code. Otherwise the stream is destroyed when the response is complete, even if the
+    request has not yet completed. Post-request processing will update stats, write to the access log
+    and finalize trace spans.
 
 We elaborate on each of these steps in the sections below.
 
@@ -533,8 +536,15 @@ directions during a request.
 :ref:`Outlier detection <arch_overview_outlier_detection>` status for the endpoint is revised as the
 request progresses.
 
-A request completes when the upstream response reaches its end-of-stream, i.e. when trailers or the
-response header/body with end-stream set are received. This is handled in
+The point at which the proxying completes and the stream is destroyed for HTTP/2 and HTTP/3 protocols
+is determined by the independent half-close option. If independent half-close is enabled the stream
+is destroyed after both request and response are complete i.e. reach their respective end-of-stream,
+by receiving trailers or the header/body with end-stream set in both directions AND response has
+success (2xx) status code. This is handled in ``FilterManager::checkAndCloseStreamIfFullyClosed()``.
+
+For HTTP/1 protocol or if independent half-close is disabled the stream is destroyed when the response
+is complete and reaches its end-of-stream, i.e. when trailers or the response header/body with
+end-stream set are received, even if the request has not yet completed. This is handled in
 ``Router::Filter::onUpstreamComplete()``.
 
 It is possible for a request to terminate early. This may be due to (but not limited to):

envoy/http/stream_reset_handler.h

@@ -36,7 +36,9 @@ enum class StreamResetReason {
   // Received payload did not conform to HTTP protocol.
   ProtocolError,
   // If the stream was locally reset by the Overload Manager.
-  OverloadManager
+  OverloadManager,
+  // If stream was locally reset due to HTTP/1 upstream half closing before downstream.
+  Http1PrematureUpstreamHalfClose,
 };
 
 /**

source/common/http/async_client_impl.cc

@@ -153,13 +153,14 @@ void AsyncStreamImpl::encodeHeaders(ResponseHeaderMapPtr&& headers, bool end_str
   encoded_response_headers_ = true;
   stream_callbacks_.onHeaders(std::move(headers), end_stream);
   closeRemote(end_stream);
-  // At present, the router cleans up stream state as soon as the remote is closed, making a
-  // half-open local stream unsupported and dangerous. Ensure we close locally to trigger completion
-  // and keep things consistent. Another option would be to issue a stream reset here if local isn't
-  // yet closed, triggering cleanup along a more standardized path. However, this would require
-  // additional logic to handle the response completion and subsequent reset, and run the risk of
-  // being interpreted as a failure, when in fact no error has necessarily occurred. Gracefully
-  // closing seems most in-line with behavior elsewhere in Envoy for now.
+  // At present, the AsyncStream is always fully closed when the server half closes the stream.
+  //
+  // Always ensure we close locally to trigger completion. Another option would be to issue a stream
+  // reset here if local isn't yet closed, triggering cleanup along a more standardized path.
+  // However, this would require additional logic to handle the response completion and subsequent
+  // reset, and run the risk of being interpreted as a failure, when in fact no error has
+  // necessarily occurred. Gracefully closing seems most in-line with behavior elsewhere in Envoy
+  // for now.
   closeLocal(end_stream);
 }
 

source/common/http/conn_manager_impl.cc

@@ -126,8 +126,10 @@ ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfigSharedPtr co
                                      /*node_id=*/local_info_.node().id(),
                                      /*server_name=*/config_->serverName(),
                                      /*proxy_status_config=*/config_->proxyStatusConfig())),
-      max_requests_during_dispatch_(runtime_.snapshot().getInteger(
-          ConnectionManagerImpl::MaxRequestsPerIoCycle, UINT32_MAX)) {
+      max_requests_during_dispatch_(
+          runtime_.snapshot().getInteger(ConnectionManagerImpl::MaxRequestsPerIoCycle, UINT32_MAX)),
+      allow_upstream_half_close_(Runtime::runtimeFeatureEnabled(
+          "envoy.reloadable_features.allow_multiplexed_upstream_half_close")) {
   ENVOY_LOG_ONCE_IF(
       trace, accept_new_http_stream_ == nullptr,
       "LoadShedPoint envoy.load_shed_points.http_connection_manager_decode_headers is not "

source/common/http/conn_manager_impl.h

@@ -283,7 +283,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
     OptRef<const Tracing::Config> tracingConfig() const override;
     const ScopeTrackedObject& scope() override;
     OptRef<DownstreamStreamFilterCallbacks> downstreamCallbacks() override { return *this; }
-    bool isHalfCloseEnabled() override { return false; }
+    bool isHalfCloseEnabled() override { return connection_manager_.allow_upstream_half_close_; }
 
     // DownstreamStreamFilterCallbacks
     void setRoute(Router::RouteConstSharedPtr route) override;
@@ -641,6 +641,18 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
   uint32_t requests_during_dispatch_count_{0};
   const uint32_t max_requests_during_dispatch_{UINT32_MAX};
   Event::SchedulableCallbackPtr deferred_request_processing_callback_;
+
+  // If independent half-close is enabled and the upstream protocol is either HTTP/2 or HTTP/3
+  // protocols the stream is destroyed after both request and response are complete i.e. reach their
+  // respective end-of-stream, by receiving trailers or the header/body with end-stream set in both
+  // directions AND response has success (2xx) status code.
+  //
+  // For HTTP/1 upstream protocol or if independent half-close is disabled the stream is destroyed
+  // when the response is complete and reaches its end-of-stream, i.e. when trailers or the response
+  // header/body with end-stream set are received, even if the request has not yet completed. If
+  // request was incomplete at response completion, the stream is reset.
+
+  const bool allow_upstream_half_close_{};
 };
 
 } // namespace Http

source/common/http/conn_pool_base.cc

@@ -177,6 +177,7 @@ void MultiplexedActiveClientBase::onStreamReset(Http::StreamResetReason reason)
   case StreamResetReason::RemoteRefusedStreamReset:
   case StreamResetReason::Overflow:
   case StreamResetReason::ConnectError:
+  case StreamResetReason::Http1PrematureUpstreamHalfClose:
     break;
   }
 }