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 693b3cb Close a H2 connection if its stream error rate is high
693b3cb is described below
commit 693b3cb465d2867379a80e882899aca8ea98f389
Author: Masakazu Kitajo <[email protected]>
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.
---
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 7a3236c..01e6d11 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -3712,6 +3712,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.
+
Plug-in Configuration
=====================
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index 8bd392f..3e93abf 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -1331,6 +1331,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}
+ ,
//# Add LOCAL Records Here
{RECT_LOCAL, "proxy.local.incoming_ip_to_bind", RECD_STRING, nullptr,
RECU_NULL, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index 55bd94b..082c7b9 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -62,6 +62,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) {}
@@ -731,6 +732,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;
void
Http2::init()
@@ -748,6 +750,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");
// If any settings is broken, ATS should not start
ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS,
max_concurrent_streams_in}));
@@ -796,6 +799,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 b344743..9289b40 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 void init();
};
diff --git a/proxy/http2/Http2ClientSession.cc
b/proxy/http2/Http2ClientSession.cc
index 7774d01..dafa43e 100644
--- a/proxy/http2/Http2ClientSession.cc
+++ b/proxy/http2/Http2ClientSession.cc
@@ -103,25 +103,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);
@@ -360,13 +372,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 9a2c5a1..06e6262 100644
--- a/proxy/http2/Http2ClientSession.h
+++ b/proxy/http2/Http2ClientSession.h
@@ -44,6 +44,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;
std::unordered_set<std::string> h2_pushed_urls;
};
diff --git a/proxy/http2/Http2ConnectionState.cc
b/proxy/http2/Http2ConnectionState.cc
index 3da2dd7..9db9caf 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -996,7 +996,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);
@@ -1693,6 +1696,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 c2eea39..50c2b39 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
@@ -316,6 +334,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;
+
// NOTE: Id of stream which MUST receive CONTINUATION frame.
// - [RFC 7540] 6.2 HEADERS
// "A HEADERS frame without the END_HEADERS flag set MUST be followed by
a
@@ -328,6 +349,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 fc6e3b4..83da4e1 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -603,7 +603,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);
}
}
}
