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 2018/02/08 01:27:48 UTC
[trafficserver] branch master updated: Send
VC_EVENT_WRITE_READY/COMPLETE when write_vio is consumed
This is an automated email from the ASF dual-hosted git repository.
masaori 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 a98021c Send VC_EVENT_WRITE_READY/COMPLETE when write_vio is consumed
a98021c is described below
commit a98021cb21ea2e180ba2664a80d9c154893b97af
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
---
proxy/http2/Http2ConnectionState.cc | 3 +
proxy/http2/Http2Stream.cc | 74 +++++++++++----------
proxy/http2/Http2Stream.h | 3 +-
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, 138 insertions(+), 39 deletions(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index e14920e..7fb120f 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1252,6 +1252,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 26d468e..7264afc 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -518,10 +518,7 @@ Http2Stream::update_read_request(int64_t read_len, bool call_update, bool check_
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
@@ -572,8 +569,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
@@ -605,28 +600,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;
}
@@ -637,30 +619,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());
@@ -668,14 +663,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 fabb4bd..0fa8399 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -157,6 +157,7 @@ public:
void do_io_shutdown(ShutdownHowTo_t) override {}
void update_read_request(int64_t read_len, bool send_update, bool check_eos = false);
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) override;
virtual void transaction_done() override;
virtual bool
@@ -168,7 +169,7 @@ public:
}
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
masaori@apache.org.