You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2017/11/17 03:03:47 UTC
[trafficserver] branch 7.1.x updated: Revert "Cancel closing if
Http2SendDataFrameResult is NO_WINDOW"
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/7.1.x by this push:
new 85139e5 Revert "Cancel closing if Http2SendDataFrameResult is NO_WINDOW"
85139e5 is described below
commit 85139e5e6cf898e672b4c54871d14381172f8b3b
Author: Leif Hedstrom <zw...@apache.org>
AuthorDate: Fri Nov 17 10:40:17 2017 +0800
Revert "Cancel closing if Http2SendDataFrameResult is NO_WINDOW"
This reverts commit 6b532c9a3de2e050ad3ca6c2e45bb5d485e25181.
The issue here is that we leak (or seemingly leak) active connections,
such that the metrics for active connections go nuts, and eventually
we throttle. This only happens with H2.
---
proxy/http2/Http2ConnectionState.cc | 38 +++++++++++++++++++------------------
proxy/http2/Http2ConnectionState.h | 14 +++++++-------
proxy/http2/Http2Stream.cc | 22 ++++++++-------------
3 files changed, 35 insertions(+), 39 deletions(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index fa1fb29..9084617 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1209,11 +1209,11 @@ Http2ConnectionState::send_data_frames_depends_on_priority()
ink_release_assert(stream != nullptr);
Http2StreamDebug(ua_session, stream->get_id(), "top node, point=%d", node->point);
- size_t len = 0;
- Http2SendDataFrameResult result = send_a_data_frame(stream, len);
+ size_t len = 0;
+ Http2SendADataFrameResult result = send_a_data_frame(stream, len);
switch (result) {
- case Http2SendDataFrameResult::NO_ERROR: {
+ case HTTP2_SEND_A_DATA_FRAME_NO_ERROR: {
// No response body to send
if (len == 0 && !stream->is_body_done()) {
dependency_tree->deactivate(node, len);
@@ -1222,7 +1222,7 @@ Http2ConnectionState::send_data_frames_depends_on_priority()
}
break;
}
- case Http2SendDataFrameResult::DONE: {
+ case HTTP2_SEND_A_DATA_FRAME_DONE: {
dependency_tree->deactivate(node, len);
delete_stream(stream);
break;
@@ -1237,7 +1237,7 @@ Http2ConnectionState::send_data_frames_depends_on_priority()
return;
}
-Http2SendDataFrameResult
+Http2SendADataFrameResult
Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_length)
{
const ssize_t window_size = std::min(this->client_rwnd, stream->client_rwnd);
@@ -1259,7 +1259,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
if (read_available_size > 0) {
// We only need to check for window size when there is a payload
if (window_size <= 0) {
- return Http2SendDataFrameResult::NO_WINDOW;
+ return HTTP2_SEND_A_DATA_FRAME_NO_WINDOW;
}
// Copy into the payload buffer. Seems like we should be able to skip this copy step
payload_length = current_reader->read(payload_buffer, write_available_size);
@@ -1271,7 +1271,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
// If we return here, we never send the END_STREAM in the case of a early terminating OS.
// OK if there is no body yet. Otherwise continue on to send a DATA frame and delete the stream
if (!stream->is_body_done() && payload_length == 0) {
- return Http2SendDataFrameResult::NO_PAYLOAD;
+ return HTTP2_SEND_A_DATA_FRAME_NO_PAYLOAD;
}
if (stream->is_body_done() && read_available_size <= write_available_size) {
@@ -1303,13 +1303,13 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
// Setting to the same state shouldn't be erroneous
stream->change_state(data.header().type, data.header().flags);
- return Http2SendDataFrameResult::DONE;
+ return HTTP2_SEND_A_DATA_FRAME_DONE;
}
- return Http2SendDataFrameResult::NO_ERROR;
+ return HTTP2_SEND_A_DATA_FRAME_NO_ERROR;
}
-Http2SendDataFrameResult
+void
Http2ConnectionState::send_data_frames(Http2Stream *stream)
{
// To follow RFC 7540 must not send more frames other than priority on
@@ -1318,25 +1318,27 @@ Http2ConnectionState::send_data_frames(Http2Stream *stream)
stream->get_state() == Http2StreamState::HTTP2_STREAM_STATE_CLOSED) {
Http2StreamDebug(this->ua_session, stream->get_id(), "Shutdown half closed local stream");
this->delete_stream(stream);
- return Http2SendDataFrameResult::NO_ERROR;
+ return;
}
- size_t len = 0;
- Http2SendDataFrameResult result = Http2SendDataFrameResult::NO_ERROR;
- while (result == Http2SendDataFrameResult::NO_ERROR) {
- result = send_a_data_frame(stream, len);
+ size_t len = 0;
+ while (true) {
+ Http2SendADataFrameResult result = send_a_data_frame(stream, len);
- if (result == Http2SendDataFrameResult::DONE) {
+ if (result == HTTP2_SEND_A_DATA_FRAME_DONE) {
// Delete a stream immediately
// TODO its should not be deleted for a several time to handling
// RST_STREAM and WINDOW_UPDATE.
// See 'closed' state written at [RFC 7540] 5.1.
Http2StreamDebug(this->ua_session, stream->get_id(), "Shutdown stream");
this->delete_stream(stream);
+ break;
+ } else if (result == HTTP2_SEND_A_DATA_FRAME_NO_ERROR) {
+ continue;
+ } else {
+ break;
}
}
-
- return result;
}
void
diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h
index 911a8de..25596f6 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -31,11 +31,11 @@
class Http2ClientSession;
-enum class Http2SendDataFrameResult {
- NO_ERROR = 0,
- NO_WINDOW = 1,
- NO_PAYLOAD = 2,
- DONE = 3,
+enum Http2SendADataFrameResult {
+ HTTP2_SEND_A_DATA_FRAME_NO_ERROR = 0,
+ HTTP2_SEND_A_DATA_FRAME_NO_WINDOW = 1,
+ HTTP2_SEND_A_DATA_FRAME_NO_PAYLOAD = 2,
+ HTTP2_SEND_A_DATA_FRAME_DONE = 3,
};
class Http2ConnectionSettings
@@ -234,8 +234,8 @@ public:
// HTTP/2 frame sender
void schedule_stream(Http2Stream *stream);
void send_data_frames_depends_on_priority();
- Http2SendDataFrameResult send_data_frames(Http2Stream *stream);
- Http2SendDataFrameResult send_a_data_frame(Http2Stream *stream, size_t &payload_length);
+ void send_data_frames(Http2Stream *stream);
+ Http2SendADataFrameResult send_a_data_frame(Http2Stream *stream, size_t &payload_length);
void send_headers_frame(Http2Stream *stream);
void send_push_promise_frame(Http2Stream *stream, URL &url, const MIMEField *accept_encoding);
void send_rst_stream_frame(Http2StreamId id, Http2ErrorCode ec);
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index c9da909..8a616cf 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -315,32 +315,26 @@ void
Http2Stream::do_io_close(int /* flags */)
{
SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
-
+ super::release(nullptr);
if (!closed) {
- if (parent && this->is_client_state_writeable()) {
- // Make sure any trailing end of stream frames are sent
- // Wee will be removed at send_data_frames or closing connection phase
- Http2SendDataFrameResult result = static_cast<Http2ClientSession *>(parent)->connection_state.send_data_frames(this);
- if (result == Http2SendDataFrameResult::NO_WINDOW) {
- Http2StreamDebug("cancel closing");
- return;
- } else {
- Http2StreamDebug("continue closing");
- }
- }
+ Http2StreamDebug("do_io_close");
// When we get here, the SM has initiated the shutdown. Either it received a WRITE_COMPLETE, or it is shutting down. Any
// remaining IO operations back to client should be abandoned. The SM-side buffers backing these operations will be deleted
// by the time this is called from transaction_done.
closed = true;
+ if (parent && this->is_client_state_writeable()) {
+ // Make sure any trailing end of stream frames are sent
+ // Wee will be removed at send_data_frames or closing connection phase
+ static_cast<Http2ClientSession *>(parent)->connection_state.send_data_frames(this);
+ }
+
clear_timers();
clear_io_events();
// Wait until transaction_done is called from HttpSM to signal that the TXN_CLOSE hook has been executed
}
-
- super::release(nullptr);
}
/*
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].