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 <masa...@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
zw...@apache.org.

Reply via email to