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 2018/02/13 23:38:28 UTC
[trafficserver] branch 7.1.x updated (f95c739 -> da5f084)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a change to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.
from f95c739 Deprecated the PageSpeed plugin
new 9ddf21f Send VC_EVENT_WRITE_READY/COMPLETE when write_vio is consumed
new da5f084 Cleanup Http2Stream
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
proxy/http2/Http2ConnectionState.cc | 3 +
proxy/http2/Http2Stream.cc | 155 +++++++++++----------
proxy/http2/Http2Stream.h | 6 +-
tests/gold_tests/h2/gold/priority_0_stderr.gold | 11 ++
tests/gold_tests/h2/gold/priority_0_stdout.gold | 1 +
.../h2/{http2.test.py => http2_priority.test.py} | 69 +++++----
6 files changed, 132 insertions(+), 113 deletions(-)
create mode 100644 tests/gold_tests/h2/gold/priority_0_stderr.gold
create mode 100644 tests/gold_tests/h2/gold/priority_0_stdout.gold
copy tests/gold_tests/h2/{http2.test.py => http2_priority.test.py} (61%)
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.
[trafficserver] 02/02: Cleanup Http2Stream
Posted by zw...@apache.org.
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
commit da5f0840057b6dea8d0d5091334e2a5911b1a829
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Wed Feb 7 16:53:55 2018 +0900
Cleanup Http2Stream
- Remove return value of Http2Stream::update_write_request(). Because return value is not used in most cases.
And Http2Stream::do_io_write() can always return &write_vio where only the return value is used.
- Http2Stream::update_write_request() returns immediately if there're no data to proceed
- Make Http2Stream::send_response_body() private
(cherry picked from commit 17f0e83620bf3ab2604e73fc7cd13d9e048513eb)
Conflicts:
proxy/http2/Http2Stream.cc
proxy/http2/Http2Stream.h
---
proxy/http2/Http2Stream.cc | 97 ++++++++++++++++++++++++----------------------
proxy/http2/Http2Stream.h | 4 +-
2 files changed, 52 insertions(+), 49 deletions(-)
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index d1bbba3..91fbdb8 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -309,7 +309,10 @@ Http2Stream::do_io_write(Continuation *c, int64_t nbytes, IOBufferReader *abuffe
write_vio.vc_server = this;
write_vio.op = VIO::WRITE;
response_reader = abuffer;
- return update_write_request(abuffer, nbytes, false) ? &write_vio : nullptr;
+
+ update_write_request(abuffer, nbytes, false);
+
+ return &write_vio;
}
// Initiated from SM
@@ -518,12 +521,11 @@ Http2Stream::restart_sending()
this->send_response_body(true);
}
-bool
+void
Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool call_update)
{
- bool retval = true;
if (!this->is_client_state_writeable() || closed || parent == nullptr || write_vio.mutex == nullptr) {
- return retval;
+ return;
}
if (this->get_thread() != this_ethread()) {
SCOPED_MUTEX_LOCK(stream_lock, this->mutex, this_ethread());
@@ -531,7 +533,7 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
// Send to the right thread
cross_thread_event = this->get_thread()->schedule_imm(this, VC_EVENT_WRITE_READY, nullptr);
}
- return retval;
+ return;
}
ink_release_assert(this->get_thread() == this_ethread());
Http2ClientSession *parent = static_cast<Http2ClientSession *>(this->get_parent());
@@ -565,54 +567,55 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
", reader.read_avail=%" PRId64,
write_vio.nbytes, write_vio.ndone, write_vio.get_writer()->write_avail(), bytes_avail);
- if (bytes_avail > 0 || is_done) {
- // Process the new data
- if (!this->response_header_done) {
- // Still parsing the response_header
- int bytes_used = 0;
- int state = this->response_header.parse_resp(&http_parser, this->response_reader, &bytes_used, false);
- // HTTPHdr::parse_resp() consumed the response_reader in above
- write_vio.ndone += this->response_header.length_get();
-
- switch (state) {
- case PARSE_RESULT_DONE: {
- this->response_header_done = true;
-
- // Send the response header back
- parent->connection_state.send_headers_frame(this);
-
- // See if the response is chunked. Set up the dechunking logic if it is
- // Make sure to check if the chunk is complete and signal appropriately
- this->response_initialize_data_handling(is_done);
-
- // If there is additional data, send it along in a data frame. Or if this was header only
- // make sure to send the end of stream
- if (this->response_is_data_available() || is_done) {
- if ((write_vio.ntodo() + this->response_header.length_get()) == bytes_avail || is_done) {
- this->mark_body_done();
- }
+ if (bytes_avail <= 0 && !is_done) {
+ return;
+ }
+
+ // Process the new data
+ if (!this->response_header_done) {
+ // Still parsing the response_header
+ int bytes_used = 0;
+ int state = this->response_header.parse_resp(&http_parser, this->response_reader, &bytes_used, false);
+ // HTTPHdr::parse_resp() consumed the response_reader in above
+ write_vio.ndone += this->response_header.length_get();
+
+ switch (state) {
+ case PARSE_RESULT_DONE: {
+ this->response_header_done = true;
- this->send_response_body(call_update);
+ // Send the response header back
+ parent->connection_state.send_headers_frame(this);
+
+ // See if the response is chunked. Set up the dechunking logic if it is
+ // Make sure to check if the chunk is complete and signal appropriately
+ this->response_initialize_data_handling(is_done);
+
+ // If there is additional data, send it along in a data frame. Or if this was header only
+ // make sure to send the end of stream
+ if (this->response_is_data_available() || is_done) {
+ if ((write_vio.ntodo() + this->response_header.length_get()) == bytes_avail || is_done) {
+ this->mark_body_done();
}
- break;
- }
- case PARSE_RESULT_CONT:
- // Let it ride for next time
- break;
- default:
- break;
- }
- } else {
- if (write_vio.ntodo() == bytes_avail || is_done) {
- this->mark_body_done();
- retval = false;
- }
- this->send_response_body(call_update);
+ this->send_response_body(call_update);
+ }
+ break;
+ }
+ case PARSE_RESULT_CONT:
+ // Let it ride for next time
+ break;
+ default:
+ break;
}
+ } else {
+ if (write_vio.ntodo() == bytes_avail || is_done) {
+ this->mark_body_done();
+ }
+
+ this->send_response_body(call_update);
}
- return retval;
+ return;
}
void
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index 87f7c34..3bc1dab 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -177,13 +177,12 @@ public:
void terminate_if_possible();
void do_io_shutdown(ShutdownHowTo_t) {}
void update_read_request(int64_t read_len, bool send_update);
- bool update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool send_update);
+ void update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool send_update);
void signal_write_event(bool call_update);
void reenable(VIO *vio);
virtual void transaction_done();
void restart_sending();
- void send_response_body(bool call_update);
void push_promise(URL &url, const MIMEField *accept_encoding);
// Stream level window size
@@ -261,6 +260,7 @@ private:
void response_process_data(bool &is_done);
bool response_is_data_available() const;
Event *send_tracked_event(Event *event, int send_event, VIO *vio);
+ void send_response_body(bool call_update);
HTTPParser http_parser;
ink_hrtime _start_time;
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.
[trafficserver] 01/02: Send VC_EVENT_WRITE_READY/COMPLETE when
write_vio is consumed
Posted by zw...@apache.org.
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
commit 9ddf21fb12d05164f14f33adb49fb47a61774b5d
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Feb 6 12:53:08 2018 +0900
Send VC_EVENT_WRITE_READY/COMPLETE when write_vio is consumed
- Add Http2Stream::signal_write_event(bool) to send VC_EVENT_WRITE_READY/COMPLETE event
- Add tests for HTTP/2 Stream Priority Feature
(cherry picked from commit a98021cb21ea2e180ba2664a80d9c154893b97af)
Conflicts:
proxy/http2/Http2Stream.cc
proxy/http2/Http2Stream.h
---
proxy/http2/Http2ConnectionState.cc | 3 +
proxy/http2/Http2Stream.cc | 72 ++++++++++-----------
proxy/http2/Http2Stream.h | 4 +-
tests/gold_tests/h2/gold/priority_0_stderr.gold | 11 ++++
tests/gold_tests/h2/gold/priority_0_stdout.gold | 1 +
tests/gold_tests/h2/http2_priority.test.py | 85 +++++++++++++++++++++++++
6 files changed, 139 insertions(+), 37 deletions(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 7b62a31..897f53d 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1224,6 +1224,9 @@ Http2ConnectionState::send_data_frames_depends_on_priority()
dependency_tree->deactivate(node, len);
} else {
dependency_tree->update(node, len);
+
+ SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread());
+ stream->signal_write_event(true);
}
break;
}
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 02babcd..d1bbba3 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -515,10 +515,7 @@ Http2Stream::update_read_request(int64_t read_len, bool call_update)
void
Http2Stream::restart_sending()
{
- send_response_body();
- if (this->write_vio.ntodo() > 0 && this->write_vio.get_writer()->write_avail() > 0) {
- write_vio._cont->handleEvent(VC_EVENT_WRITE_READY, &write_vio);
- }
+ this->send_response_body(true);
}
bool
@@ -569,8 +566,6 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
write_vio.nbytes, write_vio.ndone, write_vio.get_writer()->write_avail(), bytes_avail);
if (bytes_avail > 0 || is_done) {
- int send_event = (write_vio.ntodo() == bytes_avail || is_done) ? VC_EVENT_WRITE_COMPLETE : VC_EVENT_WRITE_READY;
-
// Process the new data
if (!this->response_header_done) {
// Still parsing the response_header
@@ -589,27 +584,15 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
// See if the response is chunked. Set up the dechunking logic if it is
// Make sure to check if the chunk is complete and signal appropriately
this->response_initialize_data_handling(is_done);
- if (is_done) {
- send_event = VC_EVENT_WRITE_COMPLETE;
- }
// If there is additional data, send it along in a data frame. Or if this was header only
// make sure to send the end of stream
- if (this->response_is_data_available() || send_event == VC_EVENT_WRITE_COMPLETE) {
- if (send_event != VC_EVENT_WRITE_COMPLETE) {
- send_response_body();
- // As with update_read_request, should be safe to call handler directly here if
- // call_update is true. Commented out for now while tracking a performance regression
- if (call_update) { // Coming from reenable. Safe to call the handler directly
- if (write_vio._cont && this->current_reader)
- write_vio._cont->handleEvent(send_event, &write_vio);
- } else { // Called from do_io_write. Might still be setting up state. Send an event to let the dust settle
- write_event = send_tracked_event(write_event, send_event, &write_vio);
- }
- } else {
+ if (this->response_is_data_available() || is_done) {
+ if ((write_vio.ntodo() + this->response_header.length_get()) == bytes_avail || is_done) {
this->mark_body_done();
- send_response_body();
}
+
+ this->send_response_body(call_update);
}
break;
}
@@ -620,29 +603,43 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
break;
}
} else {
- if (send_event == VC_EVENT_WRITE_COMPLETE) {
- // Defer sending the write complete until the send_data_frame has sent it all
- // this_ethread()->schedule_imm(this, send_event, &write_vio);
+ if (write_vio.ntodo() == bytes_avail || is_done) {
this->mark_body_done();
- send_response_body();
retval = false;
- } else {
- send_response_body();
- if (call_update) { // Coming from reenable. Safe to call the handler directly
- if (write_vio._cont && this->current_reader)
- write_vio._cont->handleEvent(send_event, &write_vio);
- } else { // Called from do_io_write. Might still be setting up state. Send an event to let the dust settle
- write_event = send_tracked_event(write_event, send_event, &write_vio);
- }
}
+
+ this->send_response_body(call_update);
}
- Http2StreamDebug("write update (event=%d)", send_event);
}
return retval;
}
void
+Http2Stream::signal_write_event(bool call_update)
+{
+ if (this->write_vio._cont == nullptr || this->write_vio.op == VIO::NONE) {
+ return;
+ }
+
+ if (this->write_vio.get_writer()->write_avail() == 0) {
+ return;
+ }
+
+ int send_event = this->write_vio.ntodo() == 0 ? VC_EVENT_WRITE_COMPLETE : VC_EVENT_WRITE_READY;
+
+ if (call_update) {
+ // Coming from reenable. Safe to call the handler directly
+ if (write_vio._cont && this->current_reader) {
+ write_vio._cont->handleEvent(send_event, &write_vio);
+ }
+ } else {
+ // Called from do_io_write. Might still be setting up state. Send an event to let the dust settle
+ write_event = send_tracked_event(write_event, send_event, &write_vio);
+ }
+}
+
+void
Http2Stream::push_promise(URL &url, const MIMEField *accept_encoding)
{
Http2ClientSession *parent = static_cast<Http2ClientSession *>(this->get_parent());
@@ -650,14 +647,17 @@ Http2Stream::push_promise(URL &url, const MIMEField *accept_encoding)
}
void
-Http2Stream::send_response_body()
+Http2Stream::send_response_body(bool call_update)
{
Http2ClientSession *parent = static_cast<Http2ClientSession *>(this->get_parent());
if (Http2::stream_priority_enabled) {
parent->connection_state.schedule_stream(this);
+ // signal_write_event() will be called from `Http2ConnectionState::send_data_frames_depends_on_priority()`
+ // when write_vio is consumed
} else {
parent->connection_state.send_data_frames(this);
+ this->signal_write_event(call_update);
}
inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
}
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index d240b46..87f7c34 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -178,10 +178,12 @@ public:
void do_io_shutdown(ShutdownHowTo_t) {}
void update_read_request(int64_t read_len, bool send_update);
bool update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool send_update);
+ void signal_write_event(bool call_update);
void reenable(VIO *vio);
virtual void transaction_done();
+
void restart_sending();
- void send_response_body();
+ void send_response_body(bool call_update);
void push_promise(URL &url, const MIMEField *accept_encoding);
// Stream level window size
diff --git a/tests/gold_tests/h2/gold/priority_0_stderr.gold b/tests/gold_tests/h2/gold/priority_0_stderr.gold
new file mode 100644
index 0000000..73b1a77
--- /dev/null
+++ b/tests/gold_tests/h2/gold/priority_0_stderr.gold
@@ -0,0 +1,11 @@
+``
+> GET /bigfile HTTP/2
+> Host: ``
+> User-Agent: curl/``
+> Accept: */*
+``
+< HTTP/2 200 ``
+< server: ATS/``
+``
+< content-length: 1048576
+``
diff --git a/tests/gold_tests/h2/gold/priority_0_stdout.gold b/tests/gold_tests/h2/gold/priority_0_stdout.gold
new file mode 100644
index 0000000..6b6476a
--- /dev/null
+++ b/tests/gold_tests/h2/gold/priority_0_stdout.gold
@@ -0,0 +1 @@
+3d5e50d1d61fb452feb3d0eaada226e52c6dc11af1b3745e0e850146f384b777 -
diff --git a/tests/gold_tests/h2/http2_priority.test.py b/tests/gold_tests/h2/http2_priority.test.py
new file mode 100644
index 0000000..ac85328
--- /dev/null
+++ b/tests/gold_tests/h2/http2_priority.test.py
@@ -0,0 +1,85 @@
+'''
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+
+# ----
+# Setup Test
+# ----
+Test.Summary = '''
+Test a basic remap of a http connection with Stream Priority Feature
+'''
+# need Curl
+Test.SkipUnless(
+ Condition.HasProgram("curl", "Curl need to be installed on system for this test to work"),
+ Condition.HasCurlFeature('http2'),
+ Condition.HasProgram("shasum", "shasum need to be installed on system for this test to work"),
+)
+Test.ContinueOnFail = True
+
+# ----
+# Setup Origin Server
+# ----
+server = Test.MakeOriginServer("server")
+
+# Test Case 0:
+server.addResponse("sessionlog.json",
+ {"headers": "GET /bigfile HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""},
+ {"headers": "HTTP/1.1 200 OK\r\nServer: microserver\r\nConnection: close\r\nCache-Control: max-age=3600\r\nContent-Length: 1048576\r\n\r\n", "timestamp": "1469733493.993", "body": ""})
+
+# ----
+# Setup ATS
+# ----
+ts = Test.MakeATSProcess("ts", select_ports=False)
+
+ts.addSSLfile("ssl/server.pem")
+ts.addSSLfile("ssl/server.key")
+ts.Variables.ssl_port = 4443
+ts.Disk.remap_config.AddLine(
+ 'map / http://127.0.0.1:{0}'.format(server.Variables.Port)
+)
+ts.Disk.ssl_multicert_config.AddLine(
+ 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
+)
+ts.Disk.records_config.update({
+ 'proxy.config.http.server_ports': '{0} {1}:ssl'.format(ts.Variables.port, ts.Variables.ssl_port),
+ 'proxy.config.http.cache.http': 0,
+ 'proxy.config.http2.stream_priority_enabled': 1,
+ 'proxy.config.http2.no_activity_timeout_in': 3,
+ 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.ssl.client.verify.server': 0,
+ 'proxy.config.ssl.server.cipher_suite': 'ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA384:AES128-GCM-SHA256:AES256-GCM-SHA384:ECDHE-RSA-RC4-SHA:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-SHA:RC4-SHA:RC4-MD5:AES128-SHA:AES256-SHA:DES-CBC3-SHA!SRP:!DSS:!PSK:!aNULL:!eNULL:!SSLv2',
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http2',
+})
+
+# ----
+# Test Cases
+# ----
+
+# Test Case 0:
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = 'curl -vs -k --http2 https://127.0.0.1:{0}/bigfile | shasum -a 256'.format(ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.TimeOut = 5
+tr.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port))
+tr.Processes.Default.StartBefore(Test.Processes.ts, ready=When.PortOpen(ts.Variables.ssl_port))
+tr.Processes.Default.Streams.stdout = "gold/priority_0_stdout.gold"
+tr.Processes.Default.Streams.stderr = "gold/priority_0_stderr.gold"
+tr.StillRunningAfter = server
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.