trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zw...@apache.org
Subject [trafficserver] branch 8.0.x updated: Close a H2 connection if its stream error rate is high
Date Tue, 23 Jul 2019 15:49:59 GMT
This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 8.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/8.0.x by this push:
     new eadf654  Close a H2 connection if its stream error rate is high
eadf654 is described below

commit eadf654a8673bb44ecfac262878874bcc0b850f7
Author: Masakazu Kitajo <maskit@apache.org>
AuthorDate: Tue Feb 12 10:01:44 2019 +0900

    Close a H2 connection if its stream error rate is high
    
        This adds a new setting for H2:
        proxy.config.http2.stream_error_rate_threshold
    
        This adds a new metric for a number of session closes because of the error rate
        proxy.process.http2.session_die_high_error_rate
    
        A conection starts graceful close when its stream error rate (error per ssn) exceeds
the configured threshold.
    
    (cherry picked from commit 693b3cb465d2867379a80e882899aca8ea98f389)
    
     Conflicts:
    	doc/admin-guide/files/records.config.en.rst
    	mgmt/RecordsConfig.cc
    	proxy/http2/HTTP2.cc
    	proxy/http2/HTTP2.h
    	proxy/http2/Http2ConnectionState.h
---
 doc/admin-guide/files/records.config.en.rst |  7 +++
 mgmt/RecordsConfig.cc                       |  2 +
 proxy/http2/HTTP2.cc                        |  5 +++
 proxy/http2/HTTP2.h                         |  2 +
 proxy/http2/Http2ClientSession.cc           | 70 +++++++++++++++++++----------
 proxy/http2/Http2ClientSession.h            | 16 ++++---
 proxy/http2/Http2ConnectionState.cc         |  6 ++-
 proxy/http2/Http2ConnectionState.h          | 26 ++++++++++-
 proxy/http2/Http2Stream.cc                  |  2 +-
 9 files changed, 104 insertions(+), 32 deletions(-)

diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index b2ca394..2e3c944 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -3518,6 +3518,13 @@ HTTP/2 Configuration
    HTTP/2 connection to avoid duplicate pushes on the same connection. If the
    maximum number is reached, new entries are not remembered.
 
+.. ts:cv:: CONFIG proxy.config.http2.stream_error_rate_threshold FLOAT 0.1
+   :reloadable:
+
+   This is the maximum stream error rate |TS| allows on an HTTP/2 connection.
+   |TS| gracefully closes connections that have stream error rates above this
+   setting by sending GOAWAY frames.
+
 .. ts:cv:: CONFIG proxy.config.http2.max_settings_per_frame INT 7
    :reloadable:
 
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index 5f781f8..b922f72 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -1324,6 +1324,8 @@ static const RecordElement RecordsConfig[] =
   ,
   {RECT_CONFIG, "proxy.config.http2.zombie_debug_timeout_in", RECD_INT, "0", RECU_DYNAMIC,
RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
   ,
+  {RECT_CONFIG, "proxy.config.http2.stream_error_rate_threshold", RECD_FLOAT, "0.1", RECU_DYNAMIC,
RR_NULL, RECC_NULL, nullptr, RECA_NULL}
+  ,
   {RECT_CONFIG, "proxy.config.http2.max_settings_per_frame", RECD_INT, "7", RECU_DYNAMIC,
RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
   ,
   {RECT_CONFIG, "proxy.config.http2.max_settings_per_minute", RECD_INT, "14", RECU_DYNAMIC,
RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index cd7a20d..675f621 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -63,6 +63,7 @@ static const char *const HTTP2_STAT_SESSION_DIE_ACTIVE_NAME            
  = "pro
 static const char *const HTTP2_STAT_SESSION_DIE_INACTIVE_NAME             = "proxy.process.http2.session_die_inactive";
 static const char *const HTTP2_STAT_SESSION_DIE_EOS_NAME                  = "proxy.process.http2.session_die_eos";
 static const char *const HTTP2_STAT_SESSION_DIE_ERROR_NAME                = "proxy.process.http2.session_die_error";
+static const char *const HTTP2_STAT_SESSION_DIE_HIGH_ERROR_RATE_NAME      = "proxy.process.http2.session_die_high_error_rate";
 
 union byte_pointer {
   byte_pointer(void *p) : ptr(p) {}
@@ -732,6 +733,7 @@ uint32_t Http2::no_activity_timeout_in     = 120;
 uint32_t Http2::active_timeout_in          = 0;
 uint32_t Http2::push_diary_size            = 256;
 uint32_t Http2::zombie_timeout_in          = 0;
+float Http2::stream_error_rate_threshold   = 0.1;
 uint32_t Http2::max_settings_per_frame     = 7;
 uint32_t Http2::max_settings_per_minute    = 14;
 
@@ -751,6 +753,7 @@ Http2::init()
   REC_EstablishStaticConfigInt32U(active_timeout_in, "proxy.config.http2.active_timeout_in");
   REC_EstablishStaticConfigInt32U(push_diary_size, "proxy.config.http2.push_diary_size");
   REC_EstablishStaticConfigInt32U(zombie_timeout_in, "proxy.config.http2.zombie_debug_timeout_in");
+  REC_EstablishStaticConfigFloat(stream_error_rate_threshold, "proxy.config.http2.stream_error_rate_threshold");
   REC_EstablishStaticConfigInt32U(max_settings_per_frame, "proxy.config.http2.max_settings_per_frame");
   REC_EstablishStaticConfigInt32U(max_settings_per_minute, "proxy.config.http2.max_settings_per_minute");
 
@@ -803,6 +806,8 @@ Http2::init()
                      static_cast<int>(HTTP2_STAT_SESSION_DIE_INACTIVE), RecRawStatSyncSum);
   RecRegisterRawStat(http2_rsb, RECT_PROCESS, HTTP2_STAT_SESSION_DIE_ERROR_NAME, RECD_INT,
RECP_PERSISTENT,
                      static_cast<int>(HTTP2_STAT_SESSION_DIE_ERROR), RecRawStatSyncSum);
+  RecRegisterRawStat(http2_rsb, RECT_PROCESS, HTTP2_STAT_SESSION_DIE_HIGH_ERROR_RATE_NAME,
RECD_INT, RECP_PERSISTENT,
+                     static_cast<int>(HTTP2_STAT_SESSION_DIE_HIGH_ERROR_RATE), RecRawStatSyncSum);
 }
 
 #if TS_HAS_TESTS
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
index 72a626c..d7203d2 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -83,6 +83,7 @@ enum {
   HTTP2_STAT_SESSION_DIE_INACTIVE,
   HTTP2_STAT_SESSION_DIE_EOS,
   HTTP2_STAT_SESSION_DIE_ERROR,
+  HTTP2_STAT_SESSION_DIE_HIGH_ERROR_RATE,
 
   HTTP2_N_STATS // Terminal counter, NOT A STAT INDEX.
 };
@@ -377,6 +378,7 @@ public:
   static uint32_t active_timeout_in;
   static uint32_t push_diary_size;
   static uint32_t zombie_timeout_in;
+  static float stream_error_rate_threshold;
   static uint32_t max_settings_per_frame;
   static uint32_t max_settings_per_minute;
 
diff --git a/proxy/http2/Http2ClientSession.cc b/proxy/http2/Http2ClientSession.cc
index d727b10..6ee435e 100644
--- a/proxy/http2/Http2ClientSession.cc
+++ b/proxy/http2/Http2ClientSession.cc
@@ -98,25 +98,37 @@ Http2ClientSession::free()
   // Update stats on how we died.  May want to eliminate this.  Was useful for
   // tracking down which cases we were having problems cleaning up.  But for general
   // use probably not worth the effort
-  switch (dying_event) {
-  case VC_EVENT_NONE:
-    HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_DEFAULT, this_ethread());
-    break;
-  case VC_EVENT_ACTIVE_TIMEOUT:
-    HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_ACTIVE, this_ethread());
-    break;
-  case VC_EVENT_INACTIVITY_TIMEOUT:
-    HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_INACTIVE, this_ethread());
-    break;
-  case VC_EVENT_ERROR:
-    HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_ERROR, this_ethread());
-    break;
-  case VC_EVENT_EOS:
-    HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_EOS, this_ethread());
-    break;
-  default:
-    HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_OTHER, this_ethread());
-    break;
+  if (cause_of_death != Http2SessionCod::NOT_PROVIDED) {
+    switch (cause_of_death) {
+    case Http2SessionCod::HIGH_ERROR_RATE:
+      HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_HIGH_ERROR_RATE, this_ethread());
+      break;
+    case Http2SessionCod::NOT_PROVIDED:
+      // Can't happen but this case is here to not have default case.
+      HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_OTHER, this_ethread());
+      break;
+    }
+  } else {
+    switch (dying_event) {
+    case VC_EVENT_NONE:
+      HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_DEFAULT, this_ethread());
+      break;
+    case VC_EVENT_ACTIVE_TIMEOUT:
+      HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_ACTIVE, this_ethread());
+      break;
+    case VC_EVENT_INACTIVITY_TIMEOUT:
+      HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_INACTIVE, this_ethread());
+      break;
+    case VC_EVENT_ERROR:
+      HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_ERROR, this_ethread());
+      break;
+    case VC_EVENT_EOS:
+      HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_EOS, this_ethread());
+      break;
+    default:
+      HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_SESSION_DIE_OTHER, this_ethread());
+      break;
+    }
   }
 
   ink_release_assert(this->client_vc == nullptr);
@@ -351,13 +363,25 @@ Http2ClientSession::main_event_handler(int event, void *edata)
     break;
   }
 
-  if (!this->is_draining()) {
+  if (!this->is_draining() && this->connection_state.get_shutdown_reason()
== Http2ErrorCode::HTTP2_ERROR_MAX) {
     this->connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NONE);
   }
 
-  // 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_NONE) {
+    if (this->is_draining()) { // For a case we already checked Connection header and
it didn't exist
+      Http2SsnDebug("Preparing for graceful shutdown because of draining state");
+      this->connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED);
+    } else if (this->connection_state.get_stream_error_rate() >
+               Http2::stream_error_rate_threshold) { // For a case many stream errors happened
+      ip_port_text_buffer ipb;
+      const char *client_ip = ats_ip_ntop(get_client_addr(), ipb, sizeof(ipb));
+      Error("HTTP/2 session error client_ip=%s session_id=%" PRId64
+            " closing a connection, because its stream error rate (%f) exceeded the threshold
(%f)",
+            client_ip, connection_id(), this->connection_state.get_stream_error_rate(),
Http2::stream_error_rate_threshold);
+      Http2SsnDebug("Preparing for graceful shutdown because of a high stream error rate");
+      cause_of_death = Http2SessionCod::HIGH_ERROR_RATE;
+      this->connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED, Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM);
+    }
   }
 
   if (this->connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NOT_INITIATED) {
diff --git a/proxy/http2/Http2ClientSession.h b/proxy/http2/Http2ClientSession.h
index cd660bc..d917dae 100644
--- a/proxy/http2/Http2ClientSession.h
+++ b/proxy/http2/Http2ClientSession.h
@@ -43,6 +43,11 @@
 #define HTTP2_SESSION_EVENT_SHUTDOWN_INIT (HTTP2_SESSION_EVENTS_START + 5)
 #define HTTP2_SESSION_EVENT_SHUTDOWN_CONT (HTTP2_SESSION_EVENTS_START + 6)
 
+enum class Http2SessionCod : int {
+  NOT_PROVIDED,
+  HIGH_ERROR_RATE,
+};
+
 size_t const HTTP2_HEADER_BUFFER_SIZE_INDEX = CLIENT_CONNECTION_FIRST_READ_BUFFER_SIZE_INDEX;
 
 // To support Upgrade: h2c
@@ -342,11 +347,12 @@ private:
   // For Upgrade: h2c
   Http2UpgradeContext upgrade_context;
 
-  VIO *write_vio        = nullptr;
-  int dying_event       = 0;
-  bool kill_me          = false;
-  bool half_close_local = false;
-  int recursion         = 0;
+  VIO *write_vio                 = nullptr;
+  int dying_event                = 0;
+  bool kill_me                   = false;
+  Http2SessionCod cause_of_death = Http2SessionCod::NOT_PROVIDED;
+  bool half_close_local          = false;
+  int recursion                  = 0;
 
   InkHashTable *h2_pushed_urls = nullptr;
   uint32_t h2_pushed_urls_size = 0;
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 243e038..6aad485 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1000,7 +1000,10 @@ Http2ConnectionState::main_event_handler(int event, void *edata)
     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);
+    if (shutdown_reason == Http2ErrorCode::HTTP2_ERROR_MAX) {
+      shutdown_reason = Http2ErrorCode::HTTP2_ERROR_NO_ERROR;
+    }
+    send_goaway_frame(latest_streamid_in, shutdown_reason);
     // Stop creating new streams
     SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
     this->ua_session->set_half_close_local_flag(true);
@@ -1696,6 +1699,7 @@ Http2ConnectionState::send_rst_stream_frame(Http2StreamId id, Http2ErrorCode
ec)
 
   if (ec != Http2ErrorCode::HTTP2_ERROR_NO_ERROR) {
     HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_STREAM_ERRORS_COUNT, this_ethread());
+    ++stream_error_count;
   }
 
   Http2Frame rst_stream(HTTP2_FRAME_TYPE_RST_STREAM, id, 0);
diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h
index 7e6a840..16dc1ae 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -221,6 +221,23 @@ public:
     return client_streams_in_count;
   }
 
+  double
+  get_stream_error_rate() const
+  {
+    int total = get_stream_requests();
+    if (total > 0) {
+      return (double)stream_error_count / (double)total;
+    } else {
+      return 0;
+    }
+  }
+
+  Http2ErrorCode
+  get_shutdown_reason() const
+  {
+    return shutdown_reason;
+  }
+
   // Connection level window size
   ssize_t client_rwnd = HTTP2_INITIAL_WINDOW_SIZE;
   ssize_t server_rwnd = Http2::initial_window_size;
@@ -267,9 +284,10 @@ public:
   }
 
   void
-  set_shutdown_state(Http2ShutdownState state)
+  set_shutdown_state(Http2ShutdownState state, Http2ErrorCode reason = Http2ErrorCode::HTTP2_ERROR_NO_ERROR)
   {
-    shutdown_state = state;
+    shutdown_state  = state;
+    shutdown_reason = reason;
   }
 
   // noncopyable
@@ -319,6 +337,9 @@ private:
   // Counter for current active streams and streams in the process of shutting down
   std::atomic<uint32_t> total_client_streams_count = 0;
 
+  // Counter for stream errors ATS sent
+  uint32_t stream_error_count = 0;
+
   // Counter for settings received within last 60 seconds
   // Each item holds a count for 30 seconds.
   uint16_t settings_count[2]            = {0};
@@ -336,6 +357,7 @@ private:
   bool fini_received                = false;
   int recursion                     = 0;
   Http2ShutdownState shutdown_state = HTTP2_SHUTDOWN_NONE;
+  Http2ErrorCode shutdown_reason    = Http2ErrorCode::HTTP2_ERROR_MAX;
   Event *shutdown_cont_event        = nullptr;
   Event *fini_event                 = nullptr;
   Event *zombie_event               = nullptr;
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 429b55f..839e7f0 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -592,7 +592,7 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t
write_len,
         if (memcmp(HTTP_VALUE_CLOSE, value, HTTP_LEN_CLOSE) == 0) {
           SCOPED_MUTEX_LOCK(lock, parent->connection_state.mutex, this_ethread());
           if (parent->connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NONE) {
-            parent->connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED);
+            parent->connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED,
Http2ErrorCode::HTTP2_ERROR_NO_ERROR);
           }
         }
       }


Mime
View raw message