http/2 graceful drain support

docs/configuration/http_conn_man/http_conn_man.rst

@@ -21,6 +21,7 @@ HTTP connection manager
       "http_codec_options": "...",
       "server_name": "...",
       "idle_timeout_s": "...",
+      "drain_timeout_ms": "...",
       "access_log": [],
       "use_remote_address": "..."
     }
@@ -100,8 +101,19 @@ idle_timeout_s
   *(optional, integer)* The idle timeout in seconds for connections managed by the connection
   manager. The idle timeout is defined as the period in which there are no active requests. If not
   set, there is no idle timeout. When the idle timeout is reached the connection will be closed. If
-  the connection is an HTTP/2 connection a GOAWAY frame will be sent prior to closing
-  the connection.
+  the connection is an HTTP/2 connection a drain sequence will occur prior to closing the
+  connection. See :ref:`drain_timeout_s <config_http_conn_man_drain_timeout_ms>`.
+
+.. _config_http_conn_man_drain_timeout_ms:
+
+drain_timeout_ms
+  *(optional, integer)* The time in milliseconds that Envoy will wait between sending an HTTP/2
+  "shutdown notification" (GOAWAY frame with max stream ID) and a final GOAWAY frame. This is used
+  so that Envoy provides a grace period for new streams that race with the final GOAWAY frame.
+  During this grace period, Envoy will continue to accept new streams. After the grace period, a
+  final GOAWAY frame is sent and Envoy will start refusing new streams. Draining occurs both
+  when a connection hits the idle timeout or during general server draining. The default grace
+  period is 5000 milliseconds (5 seconds) if this option is not specified.
 
 :ref:`access_log <config_http_conn_man_access_log>`
   *(optional, array)* Configuration for :ref:`HTTP access logs <arch_overview_http_access_logs>`
@@ -111,7 +123,7 @@ idle_timeout_s
 
 use_remote_address
   *(optional, boolean)* If set to true, the connection manager will use the real remote address
-  of the client connection when determining internal versus external origin and manipulating 
+  of the client connection when determining internal versus external origin and manipulating
   various headers. If set to false or absent, the connection manager will use the
   :ref:`config_http_conn_man_headers_x-forwarded-for` HTTP header. See the documentation for
   :ref:`config_http_conn_man_headers_x-forwarded-for`,

include/envoy/http/codec.h

@@ -188,6 +188,12 @@ class Connection {
    */
   virtual const std::string& protocolString() PURE;
 
+  /**
+   * Indicate a "shutdown notice" to the remote. This is a hint that the remote should not send
+   * any new streams, but if streams do arrive that will not be reset.
+   */
+  virtual void shutdownNotice() PURE;
+
   /**
    * @return bool whether the codec has data that it wants to write but cannot due to protocol
    *              reasons (e.g, needing window updates).

source/common/http/conn_manager_impl.h

@@ -101,6 +101,12 @@ class ConnectionManagerConfig {
                                           const Buffer::Instance& data,
                                           ServerConnectionCallbacks& callbacks) PURE;
 
+  /**
+   * @return the time in milliseconds the connection manager will wait betwen issuing a "shutdown
+   *         notice" to the time it will issue a full GOAWAY and not accept any new streams.
+   */
+  virtual std::chrono::milliseconds drainTimeout() PURE;
+
   /**
    * @return FilterChainFactory& the HTTP level filter factory to build the connection's filter
    *         chain.
@@ -375,19 +381,21 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
    */
   void destroyStream(ActiveStream& stream);
 
-  /**
-   * Connection idle timer event.
-   */
   void onIdleTimeout();
+  void onDrainTimeout();
+  void startDrainSequence();
+
+  enum class DrainState { NotDraining, Draining, Closing };
 
   ConnectionManagerConfig& config_;
   ServerConnectionPtr codec_;
   std::list<ActiveStreamPtr> streams_;
   Stats::TimespanPtr conn_length_;
   Network::DrainDecision& drain_close_;
-  bool close_when_drained_{};
+  DrainState drain_state_{DrainState::NotDraining};
   UserAgent user_agent_;
   Event::TimerPtr idle_timer_;
+  Event::TimerPtr drain_timer_;
   Runtime::RandomGenerator& random_generator_;
   Tracing::HttpTracer& tracer_;
   Runtime::Loader& runtime_;

source/common/http/http1/codec_impl.h

@@ -127,6 +127,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
   uint64_t features() override { return 0; }
   void goAway() override {} // Called during connection manager drain flow
   const std::string& protocolString() override { return Http1::PROTOCOL_STRING; }
+  void shutdownNotice() override {} // Called during connection manager drain flow
   bool wantsToWrite() override { return false; }
 
 protected:

source/common/http/http2/codec_impl.cc

@@ -260,10 +260,22 @@ void ConnectionImpl::goAway() {
   sendPendingFrames();
 }
 
+void ConnectionImpl::shutdownNotice() {
+  int rc = nghttp2_submit_shutdown_notice(session_);
+  ASSERT(rc == 0);
+  UNREFERENCED_PARAMETER(rc);
+
+  sendPendingFrames();
+}
+
 int ConnectionImpl::onFrameReceived(const nghttp2_frame* frame) {
   conn_log_trace("recv frame type={}", connection_, static_cast<uint64_t>(frame->hd.type));
-  if (frame->hd.type == NGHTTP2_GOAWAY) {
+
+  // Only raise GOAWAY once, since we don't currently expose stream information. Shutdown
+  // notifications are the same as a normal GOAWAY.
+  if (frame->hd.type == NGHTTP2_GOAWAY && !raised_goaway_) {
     ASSERT(frame->hd.stream_id == 0);
+    raised_goaway_ = true;
     callbacks().onGoAway();
     return 0;
   }

source/common/http/http2/codec_impl.h

@@ -66,6 +66,7 @@ class ConnectionImpl : public virtual Connection, Logger::Loggable<Logger::Id::h
   uint64_t features() override { return CodecFeatures::Multiplexing; }
   void goAway() override;
   const std::string& protocolString() override { return PROTOCOL_STRING; }
+  void shutdownNotice() override;
   bool wantsToWrite() override { return nghttp2_session_want_write(session_); }
 
   static const uint64_t MAX_CONCURRENT_STREAMS = 1024;
@@ -190,6 +191,7 @@ class ConnectionImpl : public virtual Connection, Logger::Loggable<Logger::Id::h
 
   Network::Connection& connection_;
   bool dispatching_{};
+  bool raised_goaway_{};
 };
 
 /**

source/server/config/network/http_connection_manager.cc

@@ -74,7 +74,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(const Json::Object& con
       stats_(Http::ConnectionManagerImpl::generateStats(stats_prefix_, server.stats())),
       codec_options_(Http::Utility::parseCodecOptions(config)),
       route_config_(new Router::ConfigImpl(config.getObject("route_config"), server.runtime(),
-                                           server.clusterManager())) {
+                                           server.clusterManager())),
+      drain_timeout_(config.getInteger("drain_timeout_ms", 5000)) {
 
   if (config.hasObject("use_remote_address")) {
     use_remote_address_ = config.getBoolean("use_remote_address");

source/server/config/network/http_connection_manager.h

@@ -62,6 +62,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   Http::ServerConnectionPtr createCodec(Network::Connection& connection,
                                         const Buffer::Instance& data,
                                         Http::ServerConnectionCallbacks& callbacks) override;
+  std::chrono::milliseconds drainTimeout() override { return drain_timeout_; }
   FilterChainFactory& filterFactory() override { return *this; }
   const Optional<std::chrono::milliseconds>& idleTimeout() override { return idle_timeout_; }
   const Router::Config& routeConfig() override { return *route_config_; }
@@ -101,6 +102,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   Optional<std::string> user_agent_;
   Optional<std::chrono::milliseconds> idle_timeout_;
   Router::ConfigPtr route_config_;
+  std::chrono::milliseconds drain_timeout_;
 };
 
 /**

source/server/http/admin.h

@@ -42,6 +42,7 @@ class AdminImpl : public Admin,
   Http::ServerConnectionPtr createCodec(Network::Connection& connection,
                                         const Buffer::Instance& data,
                                         Http::ServerConnectionCallbacks& callbacks) override;
+  std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); }
   Http::FilterChainFactory& filterFactory() override { return *this; }
   const Optional<std::chrono::milliseconds>& idleTimeout() override { return idle_timeout_; }
   const Router::Config& routeConfig() override { return *route_config_; }

test/common/http/conn_manager_impl_test.cc

@@ -67,6 +67,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi
                                   ServerConnectionCallbacks&) override {
     return ServerConnectionPtr{codec_};
   }
+  std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); }
   FilterChainFactory& filterFactory() override { return filter_factory_; }
   const Optional<std::chrono::milliseconds>& idleTimeout() override { return idle_timeout_; }
   const Router::Config& routeConfig() override { return route_config_; }
@@ -219,10 +220,16 @@ TEST_F(HttpConnectionManagerImplTest, DrainClose) {
   conn_manager_->onData(fake_input);
 
   Http::HeaderMapPtr response_headers{new HeaderMapImpl{{":status", "300"}}};
+  Event::MockTimer* drain_timer = new Event::MockTimer(&filter_callbacks_.connection_.dispatcher_);
+  EXPECT_CALL(*drain_timer, enableTimer(_));
   EXPECT_CALL(drain_close_, drainClose()).WillOnce(Return(true));
+  EXPECT_CALL(*codec_, shutdownNotice());
+  filter->callbacks_->encodeHeaders(std::move(response_headers), true);
+
   EXPECT_CALL(*codec_, goAway());
   EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWrite));
-  filter->callbacks_->encodeHeaders(std::move(response_headers), true);
+  EXPECT_CALL(*drain_timer, disableTimer());
+  drain_timer->callback_();
 
   EXPECT_EQ(1U, stats_.named_.downstream_cx_drain_close_.value());
   EXPECT_EQ(1U, stats_.named_.downstream_rq_3xx_.value());
@@ -372,6 +379,22 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamProtocolError) {
   conn_manager_->onData(fake_input);
 }
 
+TEST_F(HttpConnectionManagerImplTest, IdleTimeoutNoCodec) {
+  // Not used in the test.
+  delete codec_;
+
+  idle_timeout_.value(std::chrono::milliseconds(10));
+  Event::MockTimer* idle_timer = new Event::MockTimer(&filter_callbacks_.connection_.dispatcher_);
+  EXPECT_CALL(*idle_timer, enableTimer(_));
+  setup(false, "");
+
+  EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWrite));
+  EXPECT_CALL(*idle_timer, disableTimer());
+  idle_timer->callback_();
+
+  EXPECT_EQ(1U, stats_.named_.downstream_cx_idle_timeout_.value());
+}
+
 TEST_F(HttpConnectionManagerImplTest, IdleTimeout) {
   idle_timeout_.value(std::chrono::milliseconds(10));
   Event::MockTimer* idle_timer = new Event::MockTimer(&filter_callbacks_.connection_.dispatcher_);
@@ -411,10 +434,15 @@ TEST_F(HttpConnectionManagerImplTest, IdleTimeout) {
   Http::HeaderMapPtr response_headers{new HeaderMapImpl{{":status", "200"}}};
   filter->callbacks_->encodeHeaders(std::move(response_headers), true);
 
+  Event::MockTimer* drain_timer = new Event::MockTimer(&filter_callbacks_.connection_.dispatcher_);
+  EXPECT_CALL(*drain_timer, enableTimer(_));
+  idle_timer->callback_();
+
   EXPECT_CALL(*codec_, goAway());
   EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWrite));
   EXPECT_CALL(*idle_timer, disableTimer());
-  idle_timer->callback_();
+  EXPECT_CALL(*drain_timer, disableTimer());
+  drain_timer->callback_();
 
   EXPECT_EQ(1U, stats_.named_.downstream_cx_idle_timeout_.value());
 }

test/common/http/http2/codec_impl_test.cc

@@ -60,6 +60,32 @@ class Http2CodecImplTest : public testing::TestWithParam<uint64_t> {
   ConnectionWrapper server_wrapper_;
 };
 
+TEST_P(Http2CodecImplTest, ShutdownNotice) {
+  MockStreamDecoder response_decoder;
+  StreamEncoder& request_encoder = client_.newStream(response_decoder);
+
+  MockStreamDecoder request_decoder;
+  StreamEncoder* response_encoder;
+  EXPECT_CALL(server_callbacks_, newStream(_))
+      .WillOnce(Invoke([&](StreamEncoder& encoder) -> StreamDecoder& {
+        response_encoder = &encoder;
+        return request_decoder;
+      }));
+
+  HeaderMapImpl request_headers;
+  HttpTestUtility::addDefaultHeaders(request_headers);
+  EXPECT_CALL(request_decoder, decodeHeaders_(_, true));
+  request_encoder.encodeHeaders(request_headers, true);
+
+  EXPECT_CALL(client_callbacks_, onGoAway());
+  server_.shutdownNotice();
+  server_.goAway();
+
+  HeaderMapImpl response_headers{{":status", "200"}};
+  EXPECT_CALL(response_decoder, decodeHeaders_(_, true));
+  response_encoder->encodeHeaders(response_headers, true);
+}
+
 TEST_P(Http2CodecImplTest, RefusedStreamReset) {
   MockStreamDecoder response_decoder;
   StreamEncoder& request_encoder = client_.newStream(response_decoder);

test/config/integration/server.json

@@ -21,6 +21,7 @@
       "name": "http_connection_manager",
       "config": {
         "codec_type": "http1",
+        "drain_timeout_ms": 10,
         "access_log": [
         {
           "path": "/dev/null",
@@ -101,6 +102,7 @@
       "name": "http_connection_manager",
       "config": {
         "codec_type": "http1",
+        "drain_timeout_ms": 10,
         "access_log": [
         {
           "path": "/dev/null",

test/config/integration/server_http2.json

@@ -14,6 +14,7 @@
       "name": "http_connection_manager",
       "config": {
         "codec_type": "http2",
+        "drain_timeout_ms": 10,
         "access_log": [
         {
           "path": "/dev/null",
@@ -83,6 +84,7 @@
       "name": "http_connection_manager",
       "config": {
         "codec_type": "http2",
+        "drain_timeout_ms": 10,
         "access_log": [
         {
           "path": "/dev/null",

test/config/integration/server_http2_upstream.json

@@ -8,6 +8,7 @@
       "name": "http_connection_manager",
       "config": {
         "codec_type": "http2",
+        "drain_timeout_ms": 10,
         "access_log": [
         {
           "path": "/dev/null",
@@ -77,6 +78,7 @@
       "name": "http_connection_manager",
       "config": {
         "codec_type": "http2",
+        "drain_timeout_ms": 10,
         "access_log": [
         {
           "path": "/dev/null",

test/mocks/http/mocks.h

@@ -63,7 +63,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig {
   MOCK_METHOD0(accessLogs, const std::list<AccessLog::InstancePtr>&());
   MOCK_METHOD3(createCodec_, ServerConnection*(Network::Connection&, const Buffer::Instance&,
                                                ServerConnectionCallbacks&));
-
+  MOCK_METHOD0(drainTimeout, std::chrono::milliseconds());
   MOCK_METHOD0(filterFactory, FilterChainFactory&());
   MOCK_METHOD0(idleTimeout, const Optional<std::chrono::milliseconds>&());
   MOCK_METHOD0(routeConfig, Router::Config&());
@@ -158,6 +158,7 @@ class MockServerConnection : public ServerConnection {
   MOCK_METHOD0(features, uint64_t());
   MOCK_METHOD0(goAway, void());
   MOCK_METHOD0(protocolString, const std::string&());
+  MOCK_METHOD0(shutdownNotice, void());
   MOCK_METHOD0(wantsToWrite, bool());
 
   const std::string protocol_{"HTTP/1.1"};
@@ -173,6 +174,7 @@ class MockClientConnection : public ClientConnection {
   MOCK_METHOD0(features, uint64_t());
   MOCK_METHOD0(goAway, void());
   MOCK_METHOD0(protocolString, const std::string&());
+  MOCK_METHOD0(shutdownNotice, void());
   MOCK_METHOD0(wantsToWrite, bool());
 
   // Http::ClientConnection