This is an automated email from the ASF dual-hosted git repository.
jacksontj pushed a commit to branch master
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 3c7d1b1 TS-4509 Add `outstanding_bytes` to VConnection
3c7d1b1 is described below
commit 3c7d1b1f341c3cd28275774eeb994c3d4883277e
Author: Thomas Jackson <[email protected]>
AuthorDate: Mon Oct 3 08:16:28 2016 -0700
TS-4509 Add `outstanding_bytes` to VConnection
With this we can better check request retryability. This (in addition to
not releasing the sessions immediately on error) means that if the request is
retryable we can simply check if the number of bytes queued is the same as the
number of bytes we've asked to write. If these match, then we can be sure we
didn't send any ACKd packets-- meaning we are completely safe to retry.
---
iocore/net/I_NetVConnection.h | 8 ++++++++
iocore/net/P_UnixNetVConnection.h | 1 +
iocore/net/UnixNetVConnection.cc | 13 +++++++++++++
proxy/http/HttpSM.cc | 6 ------
proxy/http/HttpTransact.cc | 17 +++++++++++++----
5 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/iocore/net/I_NetVConnection.h b/iocore/net/I_NetVConnection.h
index 3216944..c02ceb5 100644
--- a/iocore/net/I_NetVConnection.h
+++ b/iocore/net/I_NetVConnection.h
@@ -242,6 +242,14 @@ private:
class NetVConnection : public VConnection
{
public:
+ // How many bytes have been queued to the OS for sending by haven't been
sent yet
+ // Not all platforms support this, and if they don't we'll return -1 for them
+ virtual const int64_t
+ outstanding()
+ {
+ return -1;
+ };
+
/**
Initiates read. Thread safe, may be called when not handling
an event from the NetVConnection, or the NetVConnection creation
diff --git a/iocore/net/P_UnixNetVConnection.h
b/iocore/net/P_UnixNetVConnection.h
index 8725efd..b28202b 100644
--- a/iocore/net/P_UnixNetVConnection.h
+++ b/iocore/net/P_UnixNetVConnection.h
@@ -100,6 +100,7 @@ struct OOB_callback : public Continuation {
class UnixNetVConnection : public NetVConnection
{
public:
+ virtual const int64_t outstanding();
virtual VIO *do_io_read(Continuation *c, int64_t nbytes, MIOBuffer *buf);
virtual VIO *do_io_write(Continuation *c, int64_t nbytes, IOBufferReader
*buf, bool owner = false);
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index 7c18e27..a4376e9 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -632,6 +632,19 @@ UnixNetVConnection::get_data(int id, void *data)
}
}
+const int64_t
+UnixNetVConnection::outstanding()
+{
+ int n;
+ int ret = ioctl(this->get_socket(), TIOCOUTQ, &n);
+ // if there was an error (such as ioctl doesn't support this call on this
platform) then
+ // we return -1
+ if (ret == -1) {
+ return ret;
+ }
+ return n;
+}
+
VIO *
UnixNetVConnection::do_io_read(Continuation *c, int64_t nbytes, MIOBuffer *buf)
{
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 4cd28b1..11cfa33 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -5313,9 +5313,6 @@ HttpSM::handle_post_failure()
tunnel.deallocate_buffers();
tunnel.reset();
// Server died
- vc_table.cleanup_entry(server_entry);
- server_entry = NULL;
- server_session = NULL;
t_state.current.state = HttpTransact::CONNECTION_CLOSED;
call_transact_and_set_next_state(HttpTransact::HandleResponse);
}
@@ -5477,9 +5474,6 @@ HttpSM::handle_server_setup_error(int event, void *data)
// Closedown server connection and deallocate buffers
ink_assert(server_entry->in_tunnel == false);
- vc_table.cleanup_entry(server_entry);
- server_entry = NULL;
- server_session = NULL;
// if we are waiting on a plugin callout for
// HTTP_API_SEND_REQUEST_HDR defer calling transact until
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 87888d3..6382edb 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -6533,16 +6533,25 @@ HttpTransact::is_request_valid(State *s, HTTPHdr
*incoming_request)
// bool HttpTransact::is_request_retryable
//
// In the general case once bytes have been sent on the wire the request
cannot be retried.
-// The reason we cannot retry is that the rfc2616 does not make any gaurantees
about the
+// The reason we cannot retry is that the rfc2616 does not make any guarantees
about the
// retry-ability of a request. In fact in the reverse proxy case it is quite
common for GET
-// requests on the origin to fire tracking events etc. So, as a proxy once we
have sent bytes
-// on the wire to the server we cannot gaurantee that the request is safe to
redispatch to another server.
+// requests on the origin to fire tracking events etc. So, as a proxy, once
bytes have been ACKd
+// by the server we cannot guarantee that the request is safe to retry or
redispatch to another server.
+// This is distinction of "ACKd" vs "sent" is intended, and has reason. In the
case of a
+// new origin connection there is little difference, as the chance of a RST
between setup
+// and the first set of bytes is relatively small. This distinction is more
apparent in the
+// case where the origin connection is a KA session. In this case, the session
may not have
+// been used for a long time. In that case, we'll immediately queue up session
to send to the
+// origin, without any idea of the state of the connection. If the origin is
dead (or the connection
+// is broken for some other reason) we'll immediately get a RST back. In that
case-- since no
+// bytes where ACKd by the remote end, we can retry/redispatch the request.
//
bool
HttpTransact::is_request_retryable(State *s)
{
// If there was no error establishing the connection (and we sent bytes)--
we cannot retry
- if (s->current.state != CONNECTION_ERROR &&
s->state_machine->server_request_hdr_bytes > 0) {
+ if (s->current.state != CONNECTION_ERROR &&
s->state_machine->server_request_hdr_bytes > 0 &&
+ s->state_machine->get_server_session()->get_netvc()->outstanding() !=
s->state_machine->server_request_hdr_bytes) {
return false;
}
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
