You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2017/12/13 05:08:27 UTC

[trafficserver] branch master updated: Support graceful shutdown on both HTTP/1.1 and HTTP/2

This is an automated email from the ASF dual-hosted git repository.

maskit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 6ab6a78  Support graceful shutdown on both HTTP/1.1 and HTTP/2
6ab6a78 is described below

commit 6ab6a78c50cf26428c0cd16c94d7aee5df992875
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Thu Jun 8 16:52:58 2017 +0900

    Support graceful shutdown on both HTTP/1.1 and HTTP/2
    
    For HTTP/1.1, add Connection: close header
    For HTTP/2, send GOAWAY frames
---
 doc/admin-guide/files/records.config.en.rst | 13 +++++++++++++
 iocore/net/I_NetProcessor.h                 |  1 +
 iocore/net/P_NetAccept.h                    |  1 +
 iocore/net/UnixNetAccept.cc                 |  9 +++++++++
 iocore/net/UnixNetProcessor.cc              |  8 ++++++++
 iocore/net/UnixNetVConnection.cc            |  5 -----
 mgmt/RecordsConfig.cc                       |  2 ++
 proxy/Main.cc                               | 15 +++++++++++----
 proxy/ProxyClientSession.cc                 |  2 ++
 proxy/ProxyClientSession.h                  |  8 ++++++++
 proxy/http/HttpProxyServerMain.cc           |  7 +++++++
 proxy/http/HttpProxyServerMain.h            |  2 ++
 proxy/http/HttpTransact.cc                  |  4 ++++
 proxy/http/HttpTransactHeaders.cc           | 11 +++++++++++
 proxy/http/HttpTransactHeaders.h            |  1 +
 proxy/http2/HTTP2.cc                        |  2 --
 proxy/http2/HTTP2.h                         |  2 --
 proxy/http2/Http2ClientSession.cc           | 14 ++++++++++----
 proxy/http2/Http2ConnectionState.cc         | 13 +++++++------
 proxy/http2/Http2ConnectionState.h          |  8 ++++++--
 proxy/http2/Http2SessionAccept.cc           |  4 ----
 proxy/http2/Http2Stream.cc                  | 13 +++++++++++++
 proxy/http2/Http2Stream.h                   |  8 ++++++++
 proxy/shared/UglyLogStubs.cc                |  6 ++++++
 24 files changed, 130 insertions(+), 29 deletions(-)

diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index 3718237..6f46747 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -411,6 +411,19 @@ Thread Variables
    This setting specifies the number of active client connections
    for use by :option:`traffic_ctl server restart --drain`.
 
+.. ts:cv:: CONFIG proxy.config.restart.stop_listening INT 0
+   :reloadable:
+
+   This option specifies whether |TS| should close listening sockets while shutting down gracefully.
+
+   ===== ======================================================================
+   Value Description
+   ===== ======================================================================
+   ``0`` Listening sockets will be kept open.
+   ``1`` Listening sockets will be closed when |TS| starts shutting down.
+   ===== ======================================================================
+
+
 .. ts:cv:: CONFIG proxy.config.stop.shutdown_timeout INT 0
    :reloadable:
 
diff --git a/iocore/net/I_NetProcessor.h b/iocore/net/I_NetProcessor.h
index 75ecf06..0d72080 100644
--- a/iocore/net/I_NetProcessor.h
+++ b/iocore/net/I_NetProcessor.h
@@ -152,6 +152,7 @@ public:
 
   */
   virtual Action *main_accept(Continuation *cont, SOCKET listen_socket_in, AcceptOptions const &opt = DEFAULT_ACCEPT_OPTIONS);
+  virtual void stop_accept();
 
   /**
     Open a NetVConnection for connection oriented I/O. Connects
diff --git a/iocore/net/P_NetAccept.h b/iocore/net/P_NetAccept.h
index 73e070a..a9c98ca 100644
--- a/iocore/net/P_NetAccept.h
+++ b/iocore/net/P_NetAccept.h
@@ -98,6 +98,7 @@ struct NetAccept : public Continuation {
   void init_accept_loop(const char *);
   virtual void init_accept(EThread *t = nullptr);
   virtual void init_accept_per_thread();
+  virtual void stop_accept();
   virtual NetAccept *clone() const;
 
   // 0 == success
diff --git a/iocore/net/UnixNetAccept.cc b/iocore/net/UnixNetAccept.cc
index c6d9e95..01aa4e4 100644
--- a/iocore/net/UnixNetAccept.cc
+++ b/iocore/net/UnixNetAccept.cc
@@ -202,6 +202,15 @@ NetAccept::init_accept_per_thread()
   }
 }
 
+void
+NetAccept::stop_accept()
+{
+  if (!action_->cancelled) {
+    action_->cancel();
+  }
+  server.close();
+}
+
 int
 NetAccept::do_listen(bool non_blocking)
 {
diff --git a/iocore/net/UnixNetProcessor.cc b/iocore/net/UnixNetProcessor.cc
index bba56e5..13d37a9 100644
--- a/iocore/net/UnixNetProcessor.cc
+++ b/iocore/net/UnixNetProcessor.cc
@@ -197,6 +197,14 @@ UnixNetProcessor::accept_internal(Continuation *cont, int fd, AcceptOptions cons
   return na->action_.get();
 }
 
+void
+NetProcessor::stop_accept()
+{
+  for (auto na = naVec.begin(); na != naVec.end(); ++na) {
+    (*na)->stop_accept();
+  }
+}
+
 Action *
 UnixNetProcessor::connect_re_internal(Continuation *cont, sockaddr const *target, NetVCOptions *opt)
 {
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index 9dc3be4..b96b8e0 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -1101,11 +1101,6 @@ UnixNetVConnection::acceptEvent(int event, Event *e)
 
   thread = t;
 
-  if (action_.cancelled) {
-    free(thread);
-    return EVENT_DONE;
-  }
-
   // Send this NetVC to NetHandler and start to polling read & write event.
   if (h->startIO(this) < 0) {
     free(t);
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index e6037e9..273f6a8 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -136,6 +136,8 @@ static const RecordElement RecordsConfig[] =
   ,
   {RECT_CONFIG, "proxy.config.restart.active_client_threshold", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
   ,
+  {RECT_CONFIG, "proxy.config.restart.stop_listening", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
+  ,
   {RECT_CONFIG, "proxy.config.stop.shutdown_timeout", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
   ,
 
diff --git a/proxy/Main.cc b/proxy/Main.cc
index 1269f82..3663cd2 100644
--- a/proxy/Main.cc
+++ b/proxy/Main.cc
@@ -76,6 +76,7 @@ extern "C" int plock(int);
 #include "ProxyConfig.h"
 #include "HttpProxyServerMain.h"
 #include "HttpBodyFactory.h"
+#include "ProxyClientSession.h"
 #include "logging/Log.h"
 #include "CacheControl.h"
 #include "IPAllow.h"
@@ -278,10 +279,16 @@ public:
       signal_received[SIGINT]  = false;
 
       RecInt timeout = 0;
-      REC_ReadConfigInteger(timeout, "proxy.config.stop.shutdown_timeout");
-
-      if (timeout) {
-        http2_drain = true;
+      if (RecGetRecordInt("proxy.config.stop.shutdown_timeout", &timeout) == REC_ERR_OKAY && timeout &&
+          !http_client_session_draining) {
+        http_client_session_draining = true;
+        if (!remote_management_flag) {
+          // Close listening sockets here only if TS is running standalone
+          RecInt close_sockets = 0;
+          if (RecGetRecordInt("proxy.config.restart.stop_listening", &close_sockets) == REC_ERR_OKAY && close_sockets) {
+            stop_HttpProxyServer();
+          }
+        }
       }
 
       Debug("server", "received exit signal, shutting down in %" PRId64 "secs", timeout);
diff --git a/proxy/ProxyClientSession.cc b/proxy/ProxyClientSession.cc
index 8cb61eb..348ca22 100644
--- a/proxy/ProxyClientSession.cc
+++ b/proxy/ProxyClientSession.cc
@@ -25,6 +25,8 @@
 #include "HttpDebugNames.h"
 #include "ProxyClientSession.h"
 
+bool http_client_session_draining = false;
+
 static int64_t next_cs_id = 0;
 
 ProxyClientSession::ProxyClientSession() : VConnection(nullptr)
diff --git a/proxy/ProxyClientSession.h b/proxy/ProxyClientSession.h
index 28508e0..6c3883d 100644
--- a/proxy/ProxyClientSession.h
+++ b/proxy/ProxyClientSession.h
@@ -31,6 +31,8 @@
 #include "InkAPIInternal.h"
 #include "http/HttpServerSession.h"
 
+extern bool http_client_session_draining;
+
 // Emit a debug message conditional on whether this particular client session
 // has debugging enabled. This should only be called from within a client session
 // member function.
@@ -120,6 +122,12 @@ public:
     return m_active;
   }
 
+  bool
+  is_draining() const
+  {
+    return http_client_session_draining;
+  }
+
   // Initiate an API hook invocation.
   void do_api_callout(TSHttpHookID id);
 
diff --git a/proxy/http/HttpProxyServerMain.cc b/proxy/http/HttpProxyServerMain.cc
index df1200d..21142ca 100644
--- a/proxy/http/HttpProxyServerMain.cc
+++ b/proxy/http/HttpProxyServerMain.cc
@@ -335,3 +335,10 @@ start_HttpProxyServerBackDoor(int port, int accept_threads)
   // The backdoor only binds the loopback interface
   netProcessor.main_accept(new HttpSessionAccept(ha_opt), NO_FD, opt);
 }
+
+void
+stop_HttpProxyServer()
+{
+  sslNetProcessor.stop_accept();
+  netProcessor.stop_accept();
+}
diff --git a/proxy/http/HttpProxyServerMain.h b/proxy/http/HttpProxyServerMain.h
index dea7cea..c640069 100644
--- a/proxy/http/HttpProxyServerMain.h
+++ b/proxy/http/HttpProxyServerMain.h
@@ -35,6 +35,8 @@ void init_accept_HttpProxyServer(int n_accept_threads = 0);
 */
 void start_HttpProxyServer();
 
+void stop_HttpProxyServer();
+
 void start_HttpProxyServerBackDoor(int port, int accept_threads = 0);
 
 NetProcessor::AcceptOptions make_net_accept_options(const HttpProxyPort *port, unsigned nthreads);
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 8bf5a50..508a56d 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -7772,6 +7772,10 @@ HttpTransact::build_response(State *s, HTTPHdr *base_response, HTTPHdr *outgoing
 
   HttpTransactHeaders::add_server_header_to_response(s->txn_conf, outgoing_response);
 
+  if (s->state_machine->ua_session && s->state_machine->ua_session->get_parent()->is_draining()) {
+    HttpTransactHeaders::add_connection_close(outgoing_response);
+  }
+
   if (!s->cop_test_page && is_debug_tag_set("http_hdrs")) {
     if (base_response) {
       DUMP_HEADER("http_hdrs", base_response, s->state_machine_id, "Base Header for Building Response");
diff --git a/proxy/http/HttpTransactHeaders.cc b/proxy/http/HttpTransactHeaders.cc
index 5844cc6..4a75db1 100644
--- a/proxy/http/HttpTransactHeaders.cc
+++ b/proxy/http/HttpTransactHeaders.cc
@@ -1317,3 +1317,14 @@ HttpTransactHeaders::normalize_accept_encoding(const OverridableHttpConfigParams
     }
   }
 }
+
+void
+HttpTransactHeaders::add_connection_close(HTTPHdr *header)
+{
+  MIMEField *field = header->field_find(MIME_FIELD_CONNECTION, MIME_LEN_CONNECTION);
+  if (!field) {
+    field = header->field_create(MIME_FIELD_CONNECTION, MIME_LEN_CONNECTION);
+    header->field_attach(field);
+  }
+  header->field_value_set(field, HTTP_VALUE_CLOSE, HTTP_LEN_CLOSE);
+}
diff --git a/proxy/http/HttpTransactHeaders.h b/proxy/http/HttpTransactHeaders.h
index dd6d998..3cfaa01 100644
--- a/proxy/http/HttpTransactHeaders.h
+++ b/proxy/http/HttpTransactHeaders.h
@@ -95,6 +95,7 @@ public:
   static void add_server_header_to_response(OverridableHttpConfigParams *http_txn_conf, HTTPHdr *header);
   static void remove_privacy_headers_from_request(HttpConfigParams *http_config_param, OverridableHttpConfigParams *http_txn_conf,
                                                   HTTPHdr *header);
+  static void add_connection_close(HTTPHdr *header);
 
   static int nstrcpy(char *d, const char *as);
 };
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index ef259ed..85c10b5 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -28,8 +28,6 @@
 #include "P_RecCore.h"
 #include "P_RecProcess.h"
 
-bool http2_drain = false;
-
 const char *const HTTP2_CONNECTION_PREFACE = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n";
 
 // Constant strings for pseudo headers
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
index c6e8989..d510e99 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -38,8 +38,6 @@ typedef unsigned Http2StreamId;
 // the flow control window can be come negative so we need to track it with a signed type.
 typedef int32_t Http2WindowSize;
 
-extern bool http2_drain;
-
 extern const char *const HTTP2_CONNECTION_PREFACE;
 const size_t HTTP2_CONNECTION_PREFACE_LEN = 24;
 
diff --git a/proxy/http2/Http2ClientSession.cc b/proxy/http2/Http2ClientSession.cc
index e73d01c..0d063fd 100644
--- a/proxy/http2/Http2ClientSession.cc
+++ b/proxy/http2/Http2ClientSession.cc
@@ -307,10 +307,6 @@ Http2ClientSession::main_event_handler(int event, void *edata)
     schedule_event = nullptr;
   }
 
-  if (http2_drain && this->connection_state.get_shutdown_state() == NOT_INITIATED) {
-    send_connection_event(&this->connection_state, HTTP2_SESSION_EVENT_SHUTDOWN_INIT, this);
-  }
-
   switch (event) {
   case VC_EVENT_READ_COMPLETE:
   case VC_EVENT_READ_READY:
@@ -350,6 +346,16 @@ Http2ClientSession::main_event_handler(int event, void *edata)
     retval = 0;
     break;
   }
+
+  // For a case we already checked Connection header and it didn't exist
+  if (this->is_draining() && this->connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NONE) {
+    this->connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED);
+  }
+
+  if (this->connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NOT_INITIATED) {
+    send_connection_event(&this->connection_state, HTTP2_SESSION_EVENT_SHUTDOWN_INIT, this);
+  }
+
   recursion--;
   if (!connection_state.is_recursing() && this->recursion == 0 && kill_me) {
     this->free();
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index bab5b15..0004c07 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -927,21 +927,22 @@ Http2ConnectionState::main_event_handler(int event, void *edata)
 
   // Initiate a gracefull shutdown
   case HTTP2_SESSION_EVENT_SHUTDOWN_INIT: {
-    ink_assert(shutdown_state == NOT_INITIATED);
-    shutdown_state = INITIATED;
+    ink_assert(shutdown_state == HTTP2_SHUTDOWN_NOT_INITIATED);
+    shutdown_state = HTTP2_SHUTDOWN_INITIATED;
     // [RFC 7540] 6.8.  GOAWAY
     // A server that is attempting to gracefully shut down a
     // connection SHOULD send an initial GOAWAY frame with the last stream
     // identifier set to 2^31-1 and a NO_ERROR code.
     send_goaway_frame(INT32_MAX, Http2ErrorCode::HTTP2_ERROR_NO_ERROR);
     // After allowing time for any in-flight stream creation (at least one round-trip time),
-    this_ethread()->schedule_in((Continuation *)this, HRTIME_SECONDS(2), HTTP2_SESSION_EVENT_SHUTDOWN_CONT);
+    shutdown_cont_event = this_ethread()->schedule_in((Continuation *)this, HRTIME_SECONDS(2), HTTP2_SESSION_EVENT_SHUTDOWN_CONT);
   } break;
 
   // Continue a gracefull shutdown
   case HTTP2_SESSION_EVENT_SHUTDOWN_CONT: {
-    ink_assert(shutdown_state == INITIATED);
-    shutdown_state = IN_PROGRESS;
+    ink_assert(shutdown_state == HTTP2_SHUTDOWN_INITIATED);
+    shutdown_cont_event = nullptr;
+    shutdown_state      = HTTP2_SHUTDOWN_IN_PROGRESS;
     // [RFC 7540] 6.8.  GOAWAY
     // ..., the server can send another GOAWAY frame with an updated last stream identifier
     send_goaway_frame(latest_streamid_in, Http2ErrorCode::HTTP2_ERROR_NO_ERROR);
@@ -1187,7 +1188,7 @@ Http2ConnectionState::release_stream(Http2Stream *stream)
         // We were shutting down, go ahead and terminate the session
         ua_session->destroy();
         ua_session = nullptr;
-      } else if (shutdown_state == IN_PROGRESS) {
+      } else if (shutdown_state == HTTP2_SHUTDOWN_IN_PROGRESS) {
         this_ethread()->schedule_imm_local((Continuation *)this, HTTP2_SESSION_EVENT_FINI);
       }
     }
diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h
index 27f2dc4..2395782 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -39,7 +39,7 @@ enum class Http2SendDataFrameResult {
   DONE,
 };
 
-enum Http2ShutdownState { NOT_INITIATED, INITIATED, IN_PROGRESS };
+enum Http2ShutdownState { HTTP2_SHUTDOWN_NONE, HTTP2_SHUTDOWN_NOT_INITIATED, HTTP2_SHUTDOWN_INITIATED, HTTP2_SHUTDOWN_IN_PROGRESS };
 
 class Http2ConnectionSettings
 {
@@ -139,6 +139,9 @@ public:
   void
   destroy()
   {
+    if (shutdown_cont_event) {
+      shutdown_cont_event->cancel();
+    }
     cleanup_streams();
 
     mutex = nullptr; // magic happens - assigning to nullptr frees the ProxyMutex
@@ -302,7 +305,8 @@ private:
   bool _scheduled                   = false;
   bool fini_received                = false;
   int recursion                     = 0;
-  Http2ShutdownState shutdown_state = NOT_INITIATED;
+  Http2ShutdownState shutdown_state = HTTP2_SHUTDOWN_NONE;
+  Event *shutdown_cont_event        = nullptr;
 };
 
 #endif // __HTTP2_CONNECTION_STATE_H__
diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
index 8504026..5081d04 100644
--- a/proxy/http2/Http2SessionAccept.cc
+++ b/proxy/http2/Http2SessionAccept.cc
@@ -46,10 +46,6 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
     return false;
   }
 
-  if (http2_drain) {
-    return false;
-  }
-
   netvc->attributes = this->options.transport_type;
 
   if (is_debug_tag_set("http2_seq")) {
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index d2d91c0..411b2c5 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -585,6 +585,19 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
       case PARSE_RESULT_DONE: {
         this->response_header_done = true;
 
+        // Schedule session shutdown if response header has "Connection: close"
+        MIMEField *field = this->response_header.field_find(MIME_FIELD_CONNECTION, MIME_LEN_CONNECTION);
+        if (field) {
+          int len;
+          const char *value = field->value_get(&len);
+          if (memcmp(HTTP_VALUE_CLOSE, value, HTTP_LEN_CLOSE) == 0) {
+            SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
+            if (parent->connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NONE) {
+              parent->connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED);
+            }
+          }
+        }
+
         // Send the response header back
         parent->connection_state.send_headers_frame(this);
 
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index 34aec1f..fd5f185 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -154,6 +154,14 @@ public:
   bool update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool send_update);
   void reenable(VIO *vio) override;
   virtual void transaction_done() override;
+  virtual bool
+  ignore_keep_alive() override
+  {
+    // If we return true here, Connection header will always be "close".
+    // It should be handled as the same as HTTP/1.1
+    return false;
+  }
+
   void send_response_body();
   void push_promise(URL &url, const MIMEField *accept_encoding);
 
diff --git a/proxy/shared/UglyLogStubs.cc b/proxy/shared/UglyLogStubs.cc
index c55fb1d..31726f3 100644
--- a/proxy/shared/UglyLogStubs.cc
+++ b/proxy/shared/UglyLogStubs.cc
@@ -161,6 +161,12 @@ NetProcessor::main_accept(Continuation * /* cont ATS_UNUSED */, SOCKET /* fd ATS
   return nullptr;
 }
 
+void
+NetProcessor::stop_accept()
+{
+  ink_release_assert(false);
+}
+
 Action *
 UnixNetProcessor::accept_internal(Continuation * /* cont ATS_UNUSED */, int /* fd ATS_UNUSED */,
                                   AcceptOptions const & /* opt ATS_UNUSED */)

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].