git commit: TS-2559: Disconnect clients on unrecoverable origin errors

[jp...@apache.org]

Updated Branches:
  refs/heads/master 93beae596 -> e6b9cb533


TS-2559: Disconnect clients on unrecoverable origin errors

If we receive an unrecoverable error from the origin, then the HTTP
tunnel shuts down the cache write, but leaves clients connected.
This means that when a transaction is aborted due to an error, or
by exceeding the configured active transaction time, the client is
left hanging for a response that will never come.

The solution is to treat errors the same as end-of-stream. In all
cases, if we have the response body we are ok, but otherwise we
have to shut down all the tunnel consumers. They can't recover from
the response body being truncated so there's no real choice.

This can be tested by setting
proxy.config.http.transaction_active_timeout_out to a short value
and taking too long to send the response body. Alternatively, close
the origin socket before sending all the body bytes, pausing to
make sure we procress to the tunnel handler state before closing.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/e6b9cb53
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/e6b9cb53
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/e6b9cb53

Branch: refs/heads/master
Commit: e6b9cb533bba28920ad9098b261a6e11db2d4553
Parents: 93beae5
Author: James Peach <jp...@apache.org>
Authored: Thu Feb 6 13:39:36 2014 -0800
Committer: James Peach <jp...@apache.org>
Committed: Fri Feb 7 11:03:56 2014 -0800

----------------------------------------------------------------------
 CHANGES              |  2 ++
 proxy/http/HttpSM.cc | 24 ++++++++++++------------
 2 files changed, 14 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e6b9cb53/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 770f95a..8069a7f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 5.0.0
 
+  *) [TS-2559] Disconnect clients on unrecoverable origin errors.
+
   *) [TS-2556] Fix ink_hrtime_diff_msec.
 
   *) [TS-2553] Fix a segfault in the Metalink plugin reported by Faysal Banna

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e6b9cb53/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 8618728..d888bf6 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -2815,6 +2815,9 @@ HttpSM::tunnel_handler_server(int event, HttpTunnelProducer * p)
   case VC_EVENT_ERROR:
     t_state.squid_codes.log_code = SQUID_LOG_ERR_READ_TIMEOUT;
     t_state.squid_codes.hier_code = SQUID_HIER_TIMEOUT_DIRECT;
+    /* fallthru */
+
+  case VC_EVENT_EOS:
 
     switch (event) {
     case VC_EVENT_INACTIVITY_TIMEOUT:
@@ -2826,26 +2829,23 @@ HttpSM::tunnel_handler_server(int event, HttpTunnelProducer * p)
     case VC_EVENT_ERROR:
       t_state.current.server->state = HttpTransact::CONNECTION_ERROR;
       break;
+    case VC_EVENT_EOS:
+      t_state.current.server->state = HttpTransact::TRANSACTION_COMPLETE;
+      t_state.squid_codes.log_code = SQUID_LOG_ERR_READ_ERROR;
+      break;
     }
 
-    t_state.current.server->abort = HttpTransact::ABORTED;
-    tunnel.abort_cache_write_finish_others(p);
-    close_connection = true;
-    break;
-  case VC_EVENT_EOS:
-    // Transfer terminated - check to see if this
-    //  is an abort
-    close_connection = true;
-    t_state.current.server->state = HttpTransact::TRANSACTION_COMPLETE;
-
+    close_connection = false;
     ink_assert(p->vc_type == HT_HTTP_SERVER);
+
     if (is_http_server_eos_truncation(p)) {
-      DebugSM("http", "[%" PRId64 "] [HttpSM::tunnel_handler_server] aborting cache writes due to server truncation", sm_id);
-      tunnel.abort_cache_write_finish_others(p);
+      DebugSM("http", "[%" PRId64 "] [HttpSM::tunnel_handler_server] aborting HTTP tunnel due to server truncation", sm_id);
+      tunnel.chain_abort_all(p);
       t_state.current.server->abort = HttpTransact::ABORTED;
       t_state.client_info.keep_alive = HTTP_NO_KEEPALIVE;
       t_state.current.server->keep_alive = HTTP_NO_KEEPALIVE;
     } else {
+      DebugSM("http", "[%" PRId64 "] [HttpSM::tunnel_handler_server] finishing HTTP tunnel", sm_id);
       p->read_success = true;
       t_state.current.server->abort = HttpTransact::DIDNOT_ABORT;
       // Appending reason to a response without Content-Length will result in