You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by so...@apache.org on 2016/06/07 16:27:43 UTC
[trafficserver] branch 6.2.x updated (e8b77ef -> 81faf97)
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a change to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git.
from e8b77ef TS-4436: Move hosts file implementation to `do_dns`
new 4a26939 TS-4486: Add release to MutexLock.
new d765dc1 TS-4469: TS-3612 restructuring issues causing crashes in plugins. This closes #657.
new 3f9d442 TS-4478: AsyncHttpFetch hangs since ProxyClientSession changes.
new 35c8bf9 Revert "TS-3440: Connect retries shouldn't happen if the connection has succeeeded and data has been sent"
new f153966 TS-4328 Not retry if any bytes were sent
new 678a2dc TS-4328: Add a test for the slow response case
new 6f4ecf6 TS-4328: removing flawed test case
new c7f9ad5 TS-3959: requests are retryable only if bytes were sent and there was no connection error
new 81faf97 Add tests for TS-3959
The 9 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "adds" were already present in the repository and have only
been added to this reference.
Summary of changes:
ci/tsqa/tests/test_connect_attempts.py | 113 +++++++++++++++++---------
iocore/eventsystem/I_Lock.h | 13 ++-
proxy/ProxyClientTransaction.cc | 15 ++++
proxy/ProxyClientTransaction.h | 4 +
proxy/http/HttpSM.cc | 13 +--
proxy/http/HttpServerSession.cc | 4 +
proxy/http/HttpTransact.cc | 29 +++----
proxy/http/HttpTransact.h | 4 +-
proxy/http/HttpTunnel.cc | 7 +-
proxy/http2/Http2ClientSession.h | 8 +-
proxy/http2/Http2Stream.cc | 144 ++++++++++++++++++++++++---------
proxy/http2/Http2Stream.h | 9 ++-
12 files changed, 252 insertions(+), 111 deletions(-)
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].
[trafficserver] 05/09: TS-4328 Not retry if any bytes were sent
Posted by so...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit f15396674429853dc7f5bcac15cb1b4d671ebb39
Author: Thomas Jackson <ja...@apache.org>
AuthorDate: Wed Apr 6 18:58:36 2016 -0700
TS-4328 Not retry if any bytes were sent
Before we would still retry if the origin didn't error-- but just took a really long time. So, instead of only checking if the body was sent-- we can check the SM to see if we wrote any header bytes to the wire. If any bytes were sent the request cannot be retried.
This closes #554
This also deprecates the `request_body_start` field, and as such removes it
(cherry picked from commit f129996e1adaab399359cd248f4750a686d6bfc5)
Conflicts:
proxy/http/HttpTransact.h
---
proxy/http/HttpSM.cc | 1 -
proxy/http/HttpTransact.cc | 10 +++++++---
proxy/http/HttpTransact.h | 4 +---
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index eabe0b5..2996250 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -5563,7 +5563,6 @@ HttpSM::do_setup_post_tunnel(HttpVC_t to_vc_type)
IOBufferReader *buf_start = post_buffer->alloc_reader();
int64_t post_bytes = chunked ? INT64_MAX : t_state.hdr_info.request_content_length;
- t_state.hdr_info.request_body_start = true;
// Note: Many browsers, Netscape and IE included send two extra
// bytes (CRLF) at the end of the post. We just ignore those
// bytes since the sending them is not spec
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index b75850a..65dcc8a 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -6463,13 +6463,17 @@ HttpTransact::is_request_valid(State *s, HTTPHdr *incoming_request)
// bool HttpTransact::is_request_retryable
//
-// If we started a POST/PUT tunnel then we can
-// not retry failed requests
+// 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
+// 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.
//
bool
HttpTransact::is_request_retryable(State *s)
{
- if (s->hdr_info.request_body_start == true) {
+ // If the connection was established-- we cannot retry
+ if (s->state_machine->server_request_hdr_bytes > 0) {
return false;
}
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 31f9c58..4fddf58 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -798,7 +798,6 @@ public:
bool trust_response_cl;
ResponseError_t response_error;
bool extension_method;
- bool request_body_start;
_HeaderInfo()
: client_request(),
@@ -814,8 +813,7 @@ public:
client_req_is_server_style(false),
trust_response_cl(false),
response_error(NO_RESPONSE_HEADER_ERROR),
- extension_method(false),
- request_body_start(false)
+ extension_method(false)
{
}
} HeaderInfo;
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 07/09: TS-4328: removing flawed test case
Posted by so...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit 6f4ecf663cf29874e0266b372777eede2895154e
Author: Thomas Jackson <ja...@apache.org>
AuthorDate: Wed Apr 6 19:51:11 2016 -0700
TS-4328: removing flawed test case
This test case was intended to test the case where the TCP session was established but no bytes were sent. The issue is that user space has no control over the SYN-ACK, accept() simply is user-space saying "i want this connection". So because of that this test is useless, and can be removed
(cherry picked from commit 770ca6fb2f10ea91d66a91ebca7e7689be174cb8)
---
ci/tsqa/tests/test_connect_attempts.py | 45 ----------------------------------
1 file changed, 45 deletions(-)
diff --git a/ci/tsqa/tests/test_connect_attempts.py b/ci/tsqa/tests/test_connect_attempts.py
index fc037ae..231f94d 100644
--- a/ci/tsqa/tests/test_connect_attempts.py
+++ b/ci/tsqa/tests/test_connect_attempts.py
@@ -38,36 +38,6 @@ def thread_die_on_connect(sock):
sock.close()
-def thread_delayed_accept_after_connect(sock):
- '''
- Thread to sleep a decreasing amount of time before requests
-
- sleep times: 2 -> 1 -> 0
- '''
- sock.listen(0)
- sleep_time = 2
- num_requests = 0
- # poll
- while True:
- select.select([sock], [], [])
- time.sleep(sleep_time)
- try:
- connection, addr = sock.accept()
- connection.send((
- 'HTTP/1.1 200 OK\r\n'
- 'Content-Length: {body_len}\r\n'
- 'Content-Type: text/html; charset=UTF-8\r\n'
- 'Connection: close\r\n\r\n{body}'.format(body_len=len(str(num_requests)), body=num_requests)
- ))
- connection.close()
- num_requests += 1
- except Exception as e:
- print 'connection died!', e
- pass
- if sleep_time > 0:
- sleep_time -= 1
-
-
def thread_reset_after_accept(sock):
sock.listen(0)
first = True
@@ -174,11 +144,6 @@ class TestOriginServerConnectAttempts(helpers.EnvironmentCase):
t.daemon = True
t.start()
- sock = _add_sock('delayed_accept_after_connect')
- t = threading.Thread(target=thread_delayed_accept_after_connect, args=(sock,))
- t.daemon = True
- t.start()
-
sock = _add_sock('slow_response')
t = threading.Thread(target=thread_slow_response, args=(sock,))
t.daemon = True
@@ -235,16 +200,6 @@ class TestOriginServerConnectAttempts(helpers.EnvironmentCase):
ret = requests.get(url)
self.assertEqual(ret.status_code, 502)
- def test_delayed_accept_after_connect_origin(self):
- '''Verify that we get 200s from origins that delayed_accept_after_connect'''
- url = 'http://127.0.0.1:{0}/delayed_accept_after_connect/s'.format(self.configs['records.config']['CONFIG']['proxy.config.http.server_ports'])
- ret = requests.get(url)
- # make sure it worked
- self.assertEqual(ret.status_code, 200)
- # make sure its not the first one (otherwise the test messed up somehow)
- print ret.text
- self.assertGreater(int(ret.text), 0)
-
def test_slow_response(self):
'''Verify that we get 5xx from origins that take longer than acceptable, since we will not retry them'''
url = 'http://127.0.0.1:{0}/slow_response/s'.format(self.configs['records.config']['CONFIG']['proxy.config.http.server_ports'])
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 02/09: TS-4469: TS-3612 restructuring issues
causing crashes in plugins. This closes #657.
Posted by so...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit d765dc17351f356949a3da7caafb3159c9928b9a
Author: Susan Hinrichs <sh...@ieee.org>
AuthorDate: Mon May 23 14:33:07 2016 +0000
TS-4469: TS-3612 restructuring issues causing crashes in plugins. This closes #657.
(cherry picked from commit 7013e90bf20a72a78a3227cb2653b69e30d4de73)
---
proxy/http/HttpServerSession.cc | 4 ++
proxy/http/HttpTunnel.cc | 7 +-
proxy/http2/Http2ClientSession.h | 8 ++-
proxy/http2/Http2Stream.cc | 144 ++++++++++++++++++++++++++++-----------
proxy/http2/Http2Stream.h | 9 ++-
5 files changed, 128 insertions(+), 44 deletions(-)
diff --git a/proxy/http/HttpServerSession.cc b/proxy/http/HttpServerSession.cc
index 8d0bd09..9336de3 100644
--- a/proxy/http/HttpServerSession.cc
+++ b/proxy/http/HttpServerSession.cc
@@ -180,6 +180,10 @@ HttpServerSession::release()
return;
}
+ // Make sure the vios for the current SM are cleared
+ server_vc->do_io_read(NULL, 0, NULL);
+ server_vc->do_io_write(NULL, 0, NULL);
+
HSMresult_t r = httpSessionManager.release_session(this);
if (r == HSM_RETRY) {
diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
index b96cc4b..f46ae67 100644
--- a/proxy/http/HttpTunnel.cc
+++ b/proxy/http/HttpTunnel.cc
@@ -1311,7 +1311,9 @@ HttpTunnel::consumer_reenable(HttpTunnelConsumer *c)
if (is_debug_tag_set("http_tunnel"))
Debug("http_tunnel", "Unthrottle %p %" PRId64 " / %" PRId64, p, backlog, p->backlog());
srcp->unthrottle();
- srcp->read_vio->reenable();
+ if (srcp->read_vio) {
+ srcp->read_vio->reenable();
+ }
// Kick source producer to get flow ... well, flowing.
this->producer_handler(VC_EVENT_READ_READY, srcp);
} else {
@@ -1325,7 +1327,8 @@ HttpTunnel::consumer_reenable(HttpTunnelConsumer *c)
}
}
}
- p->read_vio->reenable();
+ if (p->read_vio)
+ p->read_vio->reenable();
}
}
}
diff --git a/proxy/http2/Http2ClientSession.h b/proxy/http2/Http2ClientSession.h
index b0ddc69..48e6c84 100644
--- a/proxy/http2/Http2ClientSession.h
+++ b/proxy/http2/Http2ClientSession.h
@@ -180,7 +180,13 @@ public:
virtual void
release_netvc()
{
- client_vc = NULL;
+ // Make sure the vio's are also released to avoid
+ // later surprises in inactivity timeout
+ if (client_vc) {
+ client_vc->do_io_read(NULL, 0, NULL);
+ client_vc->do_io_write(NULL, 0, NULL);
+ client_vc = NULL;
+ }
}
sockaddr const *
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index f062b97..b2959df 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -36,9 +36,7 @@ Http2Stream::main_event_handler(int event, void *edata)
SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
if (e == cross_thread_event) {
cross_thread_event = NULL;
- }
-
- if (e == active_event) {
+ } else if (e == active_event) {
event = VC_EVENT_ACTIVE_TIMEOUT;
active_event = NULL;
} else if (e == inactive_event) {
@@ -46,6 +44,10 @@ Http2Stream::main_event_handler(int event, void *edata)
event = VC_EVENT_INACTIVITY_TIMEOUT;
clear_inactive_timer();
}
+ } else if (e == read_event) {
+ read_event = NULL;
+ } else if (e == write_event) {
+ write_event = NULL;
}
switch (event) {
case VC_EVENT_ACTIVE_TIMEOUT:
@@ -212,7 +214,7 @@ Http2Stream::do_io_read(Continuation *c, int64_t nbytes, MIOBuffer *buf)
// Is there already data in the request_buffer? If so, copy it over and then
// schedule a READ_READY or READ_COMPLETE event after we return.
- update_read_request(nbytes, true);
+ update_read_request(nbytes, false);
return &read_vio;
}
@@ -239,19 +241,27 @@ Http2Stream::do_io_write(Continuation *c, int64_t nbytes, IOBufferReader *abuffe
void
Http2Stream::do_io_close(int /* flags */)
{
+ SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
current_reader = NULL; // SM on the way out
if (!sent_delete) {
+ sent_delete = true;
Debug("http2_stream", "do_io_close stream %d", this->get_id());
// Only close if we are done sending data back to the client
if (parent && (!this->is_body_done() || this->response_is_data_available())) {
Debug("http2_stream", "%d: Undo close to pass data", this->get_id());
- closed = false; // "unclose" so this gets picked up later when the netvc side is done
- this->reenable(&write_vio); // Kick the mechanism to get any remaining data pushed out
- return;
+ closed = false; // "unclose" so this gets picked up later when the netvc side is done
+ // If chunking is playing games with us, make sure we noticed when the end of message has happened
+ if (!this->is_body_done() && this->write_vio.ndone == this->write_vio.nbytes) {
+ this->mark_body_done();
+ } else {
+ lock.release();
+ this->reenable(&write_vio); // Kick the mechanism to get any remaining data pushed out
+ Warning("Re-enabled to get data pushed out is_done=%d", this->is_body_done());
+ return;
+ }
}
closed = true;
- sent_delete = true;
if (parent) {
// Make sure any trailing end of stream frames are sent
@@ -260,9 +270,8 @@ Http2Stream::do_io_close(int /* flags */)
}
parent = NULL;
- SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
-
clear_timers();
+ clear_io_events();
if (cross_thread_event != NULL)
cross_thread_event->cancel();
@@ -289,6 +298,7 @@ Http2Stream::initiating_close()
parent = NULL;
clear_timers();
+ clear_io_events();
// This should result in do_io_close or release being called. That will schedule the final
// kill yourself signal
@@ -331,8 +341,25 @@ Http2Stream::initiating_close()
}
}
+/* Replace existing event only if the new event is different than the inprogress event */
+Event *
+Http2Stream::send_tracked_event(Event *in_event, int send_event, VIO *vio)
+{
+ Event *event = in_event;
+ if (event != NULL) {
+ if (event->callback_event != send_event) {
+ event->cancel();
+ event = NULL;
+ }
+ }
+ if (event == NULL) {
+ event = this_ethread()->schedule_imm(this, send_event, vio);
+ }
+ return event;
+}
+
void
-Http2Stream::update_read_request(int64_t read_len, bool send_update)
+Http2Stream::update_read_request(int64_t read_len, bool call_update)
{
if (closed || this->current_reader == NULL)
return;
@@ -345,33 +372,43 @@ Http2Stream::update_read_request(int64_t read_len, bool send_update)
return;
}
ink_release_assert(this->get_thread() == this_ethread());
- if (send_update) {
- SCOPED_MUTEX_LOCK(lock, read_vio.mutex, this_ethread());
- if (read_vio.nbytes > 0 && read_vio.ndone <= read_vio.nbytes) {
- // If this vio has a different buffer, we must copy
- ink_release_assert(this_ethread() == this->_thread);
- if (read_vio.buffer.writer() != (&request_buffer)) {
- int64_t num_to_read = read_vio.nbytes - read_vio.ndone;
- if (num_to_read > read_len)
- num_to_read = read_len;
- if (num_to_read > 0) {
- int bytes_added = read_vio.buffer.writer()->write(request_reader, num_to_read);
- if (bytes_added > 0) {
- request_reader->consume(bytes_added);
- read_vio.ndone += bytes_added;
- int send_event = (read_vio.nbytes == read_vio.ndone) ? VC_EVENT_READ_COMPLETE : VC_EVENT_READ_READY;
- this_ethread()->schedule_imm(this, send_event, &read_vio);
- // this->handleEvent(send_event, &read_vio);
+ SCOPED_MUTEX_LOCK(lock, read_vio.mutex, this_ethread());
+ if (read_vio.nbytes > 0 && read_vio.ndone <= read_vio.nbytes) {
+ // If this vio has a different buffer, we must copy
+ ink_release_assert(this_ethread() == this->_thread);
+ if (read_vio.buffer.writer() != (&request_buffer)) {
+ int64_t num_to_read = read_vio.nbytes - read_vio.ndone;
+ if (num_to_read > read_len)
+ num_to_read = read_len;
+ if (num_to_read > 0) {
+ int bytes_added = read_vio.buffer.writer()->write(request_reader, num_to_read);
+ if (bytes_added > 0) {
+ request_reader->consume(bytes_added);
+ read_vio.ndone += bytes_added;
+ int send_event = (read_vio.nbytes == read_vio.ndone) ? VC_EVENT_READ_COMPLETE : VC_EVENT_READ_READY;
+ // If call_update is true, should be safe to call the read_io continuation handler directly
+ // However, I was seeing performance regressions, so backed out this change to track that down
+ // Probably not the cause of performance regression, but need to test some more
+ /*if (call_update) { // Safe to call vio handler directly
+ inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
+ if (read_vio._cont && this->current_reader) read_vio._cont->handleEvent(send_event, &read_vio);
+ } else */ { // Called from do_io_read. Still setting things up. Send
+ // event to handle this after the dust settles
+ read_event = send_tracked_event(read_event, send_event, &read_vio);
}
- ink_release_assert(!this->closed);
}
- } else {
- // Try to be smart and only signal if there was additional data
- if (request_reader->read_avail() > 0) {
- int send_event = (read_vio.nbytes == read_vio.ndone) ? VC_EVENT_READ_COMPLETE : VC_EVENT_READ_READY;
- this_ethread()->schedule_imm(this, send_event, &read_vio);
- // this->handleEvent(send_event, &read_vio);
- ink_release_assert(!this->closed);
+ }
+ } else {
+ // Try to be smart and only signal if there was additional data
+ int send_event = (read_vio.nbytes == read_vio.ndone) ? VC_EVENT_READ_COMPLETE : VC_EVENT_READ_READY;
+ if (request_reader->read_avail() > 0 || send_event == VC_EVENT_READ_COMPLETE) {
+ // Same comment of call_update as above
+ /*if (call_update) { // Safe to call vio handler directly
+ inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
+ if (read_vio._cont && this->current_reader) read_vio._cont->handleEvent(send_event, &read_vio);
+ } else */ { // Called from do_io_read. Still setting things up. Send event
+ // to handle this after the dust settles
+ read_event = send_tracked_event(read_event, send_event, &read_vio);
}
}
}
@@ -379,7 +416,7 @@ Http2Stream::update_read_request(int64_t read_len, bool send_update)
}
bool
-Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool send_update)
+Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool call_update)
{
bool retval = true;
if (closed || parent == NULL)
@@ -436,7 +473,16 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
// 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) {
- this_ethread()->schedule_imm(this, VC_EVENT_WRITE_READY, &write_vio);
+ // 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
+ inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
+ 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 {
this->mark_body_done();
retval = false;
@@ -460,9 +506,16 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
send_response_body();
retval = false;
} else {
- this_ethread()->schedule_imm(this, VC_EVENT_WRITE_READY, &write_vio);
send_response_body();
- // write_vio._cont->handleEvent(send_event, &write_vio);
+ // Same comment about call_update as above
+ /*if (call_update) { // Coming from reenable. Safe to call the handler directly
+ inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
+ 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);
+ }
}
}
@@ -655,3 +708,14 @@ Http2Stream::clear_timers()
clear_inactive_timer();
clear_active_timer();
}
+
+void
+Http2Stream::clear_io_events()
+{
+ if (read_event)
+ read_event->cancel();
+ read_event = NULL;
+ if (write_event)
+ write_event->cancel();
+ write_event = NULL;
+}
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index 51606f0..1008072 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -61,7 +61,9 @@ public:
chunked(false),
cross_thread_event(NULL),
active_event(NULL),
- inactive_event(NULL)
+ inactive_event(NULL),
+ read_event(NULL),
+ write_event(NULL)
{
SET_HANDLER(&Http2Stream::main_event_handler);
}
@@ -226,8 +228,10 @@ public:
void clear_inactive_timer();
void clear_active_timer();
void clear_timers();
+ void clear_io_events();
private:
+ Event *send_tracked_event(Event *event, int send_event, VIO *vio);
HTTPParser http_parser;
ink_hrtime _start_time;
EThread *_thread;
@@ -255,6 +259,9 @@ private:
ink_hrtime inactive_timeout;
ink_hrtime inactive_timeout_at;
Event *inactive_event;
+
+ Event *read_event;
+ Event *write_event;
};
extern ClassAllocator<Http2Stream> http2StreamAllocator;
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 08/09: TS-3959: requests are retryable only if
bytes were sent and there was no connection error
Posted by so...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit c7f9ad5bc63c6581aa3bca156f67adba40be77d3
Author: Thomas Jackson <ja...@gmail.com>
AuthorDate: Mon May 23 09:20:36 2016 -0700
TS-3959: requests are retryable only if bytes were sent and there was no connection error
The root issue that TS-4328 and TS-3440 were trying to fix was that ATS was too aggressive in retries to origin (namely after the origin had already recieved the request). The previous patches attempted to have ATS not retry assuming bytes were sent on the wire-- sadly TS-4328 actually only checks that we set bytes to be written to the wire. To fix this we simply check if there was a connection failure to origin, if so we'll assume that it is retryable (since connection failures to or [...]
(cherry picked from commit 736009b973ad44ef4c7185f5f3bfb778903e4316)
---
proxy/http/HttpTransact.cc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 65dcc8a..c4d10f5 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -6472,8 +6472,8 @@ HttpTransact::is_request_valid(State *s, HTTPHdr *incoming_request)
bool
HttpTransact::is_request_retryable(State *s)
{
- // If the connection was established-- we cannot retry
- if (s->state_machine->server_request_hdr_bytes > 0) {
+ // 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) {
return false;
}
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 04/09: Revert "TS-3440: Connect retries shouldn't
happen if the connection has succeeeded and data has been sent"
Posted by so...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit 35c8bf91f54b9ff80fa4361c98ecfec5a90c75c1
Author: Thomas Jackson <ja...@apache.org>
AuthorDate: Wed Apr 6 18:58:12 2016 -0700
Revert "TS-3440: Connect retries shouldn't happen if the connection has succeeeded and data has been sent"
This reverts commit 440a14513e59baf21e55b07f0dd4aa39bac232de.
Conflicts:
proxy/http/HttpTransact.cc
(cherry picked from commit 59c3ed195acc2cac719eade3981411ab97cb4b6e)
---
proxy/http/HttpTransact.cc | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 8c7e119..b75850a 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -3717,19 +3717,6 @@ HttpTransact::handle_response_from_server(State *s)
s->current.server->set_connect_fail(EUSERS); // too many users
handle_server_connection_not_open(s);
break;
- case CONNECTION_CLOSED:
- /* fall through */
- case PARSE_ERROR:
- /* fall through */
- case BAD_INCOMING_RESPONSE: {
- // this case should not be allowed to retry because we'll end up making another request
- DebugTxn("http_trans",
- "[handle_response_from_server] Transaction received a bad response or a partial response, not retrying...");
- SET_VIA_STRING(VIA_DETAIL_SERVER_CONNECT, VIA_DETAIL_SERVER_FAILURE);
- handle_server_died(s);
- s->next_action = SM_ACTION_SEND_ERROR_CACHE_NOOP;
- break;
- }
case OPEN_RAW_ERROR:
/* fall through */
case CONNECTION_ERROR:
@@ -3737,6 +3724,12 @@ HttpTransact::handle_response_from_server(State *s)
case STATE_UNDEFINED:
/* fall through */
case INACTIVE_TIMEOUT:
+ /* fall through */
+ case PARSE_ERROR:
+ /* fall through */
+ case CONNECTION_CLOSED:
+ /* fall through */
+ case BAD_INCOMING_RESPONSE:
// Set to generic I/O error if not already set specifically.
if (!s->current.server->had_connect_fail())
s->current.server->set_connect_fail(EIO);
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 01/09: TS-4486: Add release to MutexLock.
Posted by so...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit 4a26939d7d61766875838518d9ac454c49319c34
Author: Alan M. Carroll <so...@yahoo-inc.com>
AuthorDate: Wed May 25 10:39:17 2016 -0500
TS-4486: Add release to MutexLock.
(cherry picked from commit bcdcd63d81f46d80c40883b6e27e4a5532d188c4)
Conflicts:
iocore/eventsystem/I_Lock.h
---
iocore/eventsystem/I_Lock.h | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/iocore/eventsystem/I_Lock.h b/iocore/eventsystem/I_Lock.h
index e90d1bf..e2a9999 100644
--- a/iocore/eventsystem/I_Lock.h
+++ b/iocore/eventsystem/I_Lock.h
@@ -432,6 +432,7 @@ class MutexLock
{
private:
Ptr<ProxyMutex> m;
+ bool locked_p;
public:
MutexLock(
@@ -439,7 +440,7 @@ public:
const SrcLoc &location, const char *ahandler,
#endif // DEBUG
ProxyMutex *am, EThread *t)
- : m(am)
+ : m(am), locked_p(true)
{
Mutex_lock(
#ifdef DEBUG
@@ -448,7 +449,15 @@ public:
m, t);
}
- ~MutexLock() { Mutex_unlock(m, m->thread_holding); }
+ void
+ release()
+ {
+ if (locked_p)
+ Mutex_unlock(m, m->thread_holding);
+ locked_p = false;
+ }
+
+ ~MutexLock() { this->release(); }
};
/** Scoped try lock class for ProxyMutex
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 09/09: Add tests for TS-3959
Posted by so...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit 81faf97f4ce6eac9e538c58a6b0e5723251a5876
Author: Thomas Jackson <ja...@gmail.com>
AuthorDate: Tue May 24 08:17:19 2016 -0700
Add tests for TS-3959
Specifically, we want to test the race condition between KA resuse ATS side and the origin closing the connection
(cherry picked from commit b67906d1a08cc543f5c768676cef606d152afe16)
---
ci/tsqa/tests/test_connect_attempts.py | 42 ++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/ci/tsqa/tests/test_connect_attempts.py b/ci/tsqa/tests/test_connect_attempts.py
index 231f94d..d52f41e 100644
--- a/ci/tsqa/tests/test_connect_attempts.py
+++ b/ci/tsqa/tests/test_connect_attempts.py
@@ -111,6 +111,36 @@ def thread_slow_response(sock):
sleep_time -= 1
+def thread_slow_close(sock):
+ '''
+ Thread to sleep a decreasing amount of time after the request, before closing
+
+ sleep times: 2 -> 1 -> 0
+ '''
+ sock.listen(0)
+ sleep_time = 2
+ num_requests = 0
+ # poll
+ while True:
+ select.select([sock], [], [])
+ try:
+ connection, addr = sock.accept()
+ connection.send((
+ 'HTTP/1.1 200 OK\r\n'
+ 'Content-Length: {body_len}\r\n'
+ 'Content-Type: text/html; charset=UTF-8\r\n'
+ 'Connection: close\r\n\r\n{body}'.format(body_len=len(str(num_requests)), body=num_requests)
+ ))
+ time.sleep(sleep_time)
+ connection.close()
+ num_requests += 1
+ except Exception as e:
+ print 'connection died!', e
+ pass
+ if sleep_time > 0:
+ sleep_time -= 1
+
+
class TestOriginServerConnectAttempts(helpers.EnvironmentCase):
@classmethod
def setUpEnv(cls, env):
@@ -154,6 +184,11 @@ class TestOriginServerConnectAttempts(helpers.EnvironmentCase):
t.daemon = True
t.start()
+ sock = _add_sock('slow_close')
+ t = threading.Thread(target=thread_slow_close, args=(sock,))
+ t.daemon = True
+ t.start()
+
# only add server headers when there weren't any
cls.configs['records.config']['CONFIG']['proxy.config.http.response_server_enabled'] = 2
@@ -206,3 +241,10 @@ class TestOriginServerConnectAttempts(helpers.EnvironmentCase):
ret = requests.get(url)
# make sure it worked
self.assertEqual(ret.status_code, 504)
+
+ def test_slow_close(self):
+ '''Verify that we retry connecting to an origin when there is a connection failure'''
+ url = 'http://127.0.0.1:{0}/slow_close/s'.format(self.configs['records.config']['CONFIG']['proxy.config.http.server_ports'])
+ ret = requests.get(url)
+ # make sure it worked
+ self.assertEqual(ret.status_code, 200)
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 06/09: TS-4328: Add a test for the slow response
case
Posted by so...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit 678a2dc973b5b1f7b2f62930e9b4948834ab180b
Author: Thomas Jackson <ja...@apache.org>
AuthorDate: Wed Apr 6 19:50:19 2016 -0700
TS-4328: Add a test for the slow response case
If the request hits a timeout-- but we sent the request to the origin we cannot rety the request
(cherry picked from commit a6e89ed4ea1a67613c27a4136e3a7a58ef7aead7)
---
ci/tsqa/tests/test_connect_attempts.py | 42 ++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/ci/tsqa/tests/test_connect_attempts.py b/ci/tsqa/tests/test_connect_attempts.py
index 58d70e1..fc037ae 100644
--- a/ci/tsqa/tests/test_connect_attempts.py
+++ b/ci/tsqa/tests/test_connect_attempts.py
@@ -111,6 +111,36 @@ def thread_partial_response(sock):
connection.close()
+def thread_slow_response(sock):
+ '''
+ Thread to sleep a decreasing amount of time before sending the response
+
+ sleep times: 2 -> 1 -> 0
+ '''
+ sock.listen(0)
+ sleep_time = 2
+ num_requests = 0
+ # poll
+ while True:
+ select.select([sock], [], [])
+ try:
+ connection, addr = sock.accept()
+ time.sleep(sleep_time)
+ connection.send((
+ 'HTTP/1.1 200 OK\r\n'
+ 'Content-Length: {body_len}\r\n'
+ 'Content-Type: text/html; charset=UTF-8\r\n'
+ 'Connection: close\r\n\r\n{body}'.format(body_len=len(str(num_requests)), body=num_requests)
+ ))
+ connection.close()
+ num_requests += 1
+ except Exception as e:
+ print 'connection died!', e
+ pass
+ if sleep_time > 0:
+ sleep_time -= 1
+
+
class TestOriginServerConnectAttempts(helpers.EnvironmentCase):
@classmethod
def setUpEnv(cls, env):
@@ -149,6 +179,11 @@ class TestOriginServerConnectAttempts(helpers.EnvironmentCase):
t.daemon = True
t.start()
+ sock = _add_sock('slow_response')
+ t = threading.Thread(target=thread_slow_response, args=(sock,))
+ t.daemon = True
+ t.start()
+
sock = _add_sock('partial_response')
t = threading.Thread(target=thread_partial_response, args=(sock,))
t.daemon = True
@@ -209,3 +244,10 @@ class TestOriginServerConnectAttempts(helpers.EnvironmentCase):
# make sure its not the first one (otherwise the test messed up somehow)
print ret.text
self.assertGreater(int(ret.text), 0)
+
+ def test_slow_response(self):
+ '''Verify that we get 5xx from origins that take longer than acceptable, since we will not retry them'''
+ url = 'http://127.0.0.1:{0}/slow_response/s'.format(self.configs['records.config']['CONFIG']['proxy.config.http.server_ports'])
+ ret = requests.get(url)
+ # make sure it worked
+ self.assertEqual(ret.status_code, 504)
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 03/09: TS-4478: AsyncHttpFetch hangs since
ProxyClientSession changes.
Posted by so...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit 3f9d4425c7c717c409bad3dba95b05212c402f7d
Author: Susan Hinrichs <sh...@ieee.org>
AuthorDate: Wed Jun 1 20:42:07 2016 +0000
TS-4478: AsyncHttpFetch hangs since ProxyClientSession changes.
(cherry picked from commit 85e0f0d34bfbbd1e6a55e7bfa0f55fe26b00eac2)
---
proxy/ProxyClientTransaction.cc | 15 +++++++++++++++
proxy/ProxyClientTransaction.h | 4 ++++
proxy/http/HttpSM.cc | 12 ++++--------
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/proxy/ProxyClientTransaction.cc b/proxy/ProxyClientTransaction.cc
index 835dc5a..067bb93 100644
--- a/proxy/ProxyClientTransaction.cc
+++ b/proxy/ProxyClientTransaction.cc
@@ -84,3 +84,18 @@ ProxyClientTransaction::attach_server_session(HttpServerSession *ssession, bool
{
parent->attach_server_session(ssession, transaction_done);
}
+
+Action *
+ProxyClientTransaction::adjust_thread(int event, void *data)
+{
+ NetVConnection *vc = this->get_netvc();
+ EThread *this_thread = this_ethread();
+ if (vc && vc->thread != this_thread) {
+ if (vc->thread->is_event_type(ET_NET) || vc->thread->is_event_type(SSLNetProcessor::ET_SSL)) {
+ return vc->thread->schedule_imm(this, event, data);
+ } else { // Not a net thread, take over this thread
+ vc->thread = this_thread;
+ }
+ }
+ return NULL;
+}
diff --git a/proxy/ProxyClientTransaction.h b/proxy/ProxyClientTransaction.h
index 7bba8fc..e724aaf 100644
--- a/proxy/ProxyClientTransaction.h
+++ b/proxy/ProxyClientTransaction.h
@@ -49,6 +49,10 @@ public:
virtual void attach_server_session(HttpServerSession *ssession, bool transaction_done = true);
+ // See if we need to schedule on the primary thread for the transaction or change the thread that is associated with the VC.
+ // If we reschedule, the scheduled action is returned. Otherwise, NULL is returned
+ Action *adjust_thread(int event, void *data);
+
int
get_transact_count() const
{
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 8652597..eabe0b5 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -2456,10 +2456,8 @@ HttpSM::state_cache_open_write(int event, void *data)
// Make sure we are on the "right" thread
if (ua_session) {
- NetVConnection *vc = ua_session->get_netvc();
- if (vc && vc->thread != this_ethread()) {
- pending_action = vc->thread->schedule_imm(this, event, data); // Stay on the same thread!
- return 0;
+ if ((pending_action = ua_session->adjust_thread(event, data))) {
+ return 0; // Go away if we reschedule
}
}
@@ -4666,10 +4664,8 @@ HttpSM::do_http_server_open(bool raw)
// Make sure we are on the "right" thread
if (ua_session) {
- NetVConnection *vc = ua_session->get_netvc();
- if (vc && vc->thread != this_ethread()) {
- pending_action = vc->thread->schedule_imm(this, EVENT_INTERVAL);
- return;
+ if ((pending_action = ua_session->adjust_thread(EVENT_INTERVAL, NULL))) {
+ return; // Go away if we reschedule
}
}
pending_action = NULL;
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.