This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new a220739a0 Add 5xx's to be allowed to be used for simple retries (#8518)
a220739a0 is described below
commit a220739a01e049251aaef018f8f0b5b8f7e940c6
Author: Evan Zelkowitz <[email protected]>
AuthorDate: Mon Nov 15 17:17:15 2021 -0700
Add 5xx's to be allowed to be used for simple retries (#8518)
* Add 5xx's to be allowed to be used for simple retries
Remove unnecessary functions in transact for finding ranges
Change PS response checking to not use internal state. Now pass in retry
type and code
(cherry picked from commit 30096b45c98e90334a8ead5ba2998c4e424f87e0)
---
doc/admin-guide/files/parent.config.en.rst | 10 +++++++--
proxy/ParentSelection.cc | 6 ++---
proxy/ParentSelection.h | 10 ++++-----
proxy/http/HttpTransact.cc | 36 +++++++++---------------------
4 files changed, 26 insertions(+), 36 deletions(-)
diff --git a/doc/admin-guide/files/parent.config.en.rst
b/doc/admin-guide/files/parent.config.en.rst
index 4c0e2e025..dcb12a0c7 100644
--- a/doc/admin-guide/files/parent.config.en.rst
+++ b/doc/admin-guide/files/parent.config.en.rst
@@ -214,7 +214,7 @@ The following list shows the possible actions and their
allowed values.
``parent_retry``
- ``simple_retry`` - If the parent returns a 404 response or if the
response matches
- a list of http 4xx responses defined in
``simple_server_retry_responses`` on a request
+ a list of http 4xx and/or 5xx responses defined in
``simple_server_retry_responses`` on a request
a new parent is selected and the request is retried. The number of
retries is controlled
by ``max_simple_retries`` which is set to 1 by default.
- ``unavailable_server_retry`` - If the parent returns a 503 response or
if the response matches
@@ -224,11 +224,17 @@ The following list shows the possible actions and their
allowed values.
- ``both`` - This enables both ``simple_retry`` and
``unavailable_server_retry`` as described above.
- If not set, by default all response codes will be considered a success,
and parents will not be retried based on any HTTP response code.
+ .. Note::
+
+ If a response code exists in both the simple and unavailable lists and
both
+ is the retry type then simple_retry will take precedence and
unavailable_server_retry
+ will not be used for that code.
+
.. _parent-config-format-simple-server-retry-responses:
``simple_server_retry_responses``
If ``parent_retry`` is set to either ``simple_retry`` or ``both``, this
parameter is a comma separated list of
- http 4xx response codes that will invoke the ``simple_retry`` described in
the ``parent_retry`` section. By
+ http 4xx and/or 5xx response codes that will invoke the ``simple_retry``
described in the ``parent_retry`` section. By
default, ``simple_server_retry_responses`` is set to 404.
.. _parent-config-format-unavailable-server-retry-responses:
diff --git a/proxy/ParentSelection.cc b/proxy/ParentSelection.cc
index 680063205..8c0b54e07 100644
--- a/proxy/ParentSelection.cc
+++ b/proxy/ParentSelection.cc
@@ -361,17 +361,17 @@ SimpleRetryResponseCodes::SimpleRetryResponseCodes(char
*val)
numTok = pTok.Initialize(val, SHARE_TOKS);
if (numTok == 0) {
c = atoi(val);
- if (c > 399 && c < 500) {
+ if (c > 399 && c < 600) {
codes.push_back(HTTP_STATUS_NOT_FOUND);
}
}
for (int i = 0; i < numTok; i++) {
c = atoi(pTok[i]);
- if (c > 399 && c < 500) {
+ if (c > 399 && c < 600) {
Debug("parent_select", "loading simple response code: %d", c);
codes.push_back(c);
} else {
- Warning("SimpleRetryResponseCodes received non-4xx code '%s',
ignoring!", pTok[i]);
+ Warning("SimpleRetryResponseCodes received non-4xx or 5xx code '%s',
ignoring!", pTok[i]);
}
}
std::sort(codes.begin(), codes.end());
diff --git a/proxy/ParentSelection.h b/proxy/ParentSelection.h
index add3845dc..4ea1eebd2 100644
--- a/proxy/ParentSelection.h
+++ b/proxy/ParentSelection.h
@@ -261,17 +261,17 @@ struct ParentResult {
}
bool
- response_is_retryable(HTTPStatus response_code) const
+ response_is_retryable(ParentRetry_t retry_type, HTTPStatus response_code)
const
{
- Debug("parent_select", "In response_is_retryable, code: %d",
response_code);
- if (retry_type() == PARENT_RETRY_BOTH) {
+ Debug("parent_select", "In response_is_retryable, code: %d, type: %d",
response_code, retry_type);
+ if (retry_type == PARENT_RETRY_BOTH) {
Debug("parent_select", "Saw retry both");
return (rec->unavailable_server_retry_responses->contains(response_code)
||
rec->simple_server_retry_responses->contains(response_code));
- } else if (retry_type() == PARENT_RETRY_UNAVAILABLE_SERVER) {
+ } else if (retry_type == PARENT_RETRY_UNAVAILABLE_SERVER) {
Debug("parent_select", "Saw retry unavailable server");
return rec->unavailable_server_retry_responses->contains(response_code);
- } else if (retry_type() == PARENT_RETRY_SIMPLE) {
+ } else if (retry_type == PARENT_RETRY_SIMPLE) {
Debug("parent_select", "Saw retry simple retry");
return rec->simple_server_retry_responses->contains(response_code);
} else {
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 4ce185eae..b5bbec45b 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -304,26 +304,6 @@ is_localhost(const char *name, int len)
return (len == (sizeof(local) - 1)) && (memcmp(name, local, len) == 0);
}
-inline static bool
-is_response_simple_code(HTTPStatus response_code)
-{
- if (static_cast<unsigned int>(response_code) < 400 || static_cast<unsigned
int>(response_code) > 499) {
- return false;
- }
-
- return true;
-}
-
-inline static bool
-is_response_unavailable_code(HTTPStatus response_code)
-{
- if (static_cast<unsigned int>(response_code) < 500 || static_cast<unsigned
int>(response_code) > 599) {
- return false;
- }
-
- return true;
-}
-
bool
HttpTransact::is_response_valid(State *s, HTTPHdr *incoming_response)
{
@@ -399,21 +379,25 @@ response_is_retryable(HttpTransact::State *s, HTTPStatus
response_code)
return mp->strategy->responseIsRetryable(s->state_machine->sm_id,
s->current, response_code);
}
- if (s->parent_params &&
!s->parent_result.response_is_retryable(response_code)) {
+ if (s->parent_params &&
!s->parent_result.response_is_retryable((ParentRetry_t)(s->parent_result.retry_type()),
response_code)) {
return PARENT_RETRY_NONE;
}
-
- const unsigned int s_retry_type = retry_type(s);
- const HTTPStatus server_response =
http_hdr_status_get(s->hdr_info.server_response.m_http);
- if ((s_retry_type & PARENT_RETRY_SIMPLE) &&
is_response_simple_code(server_response) &&
+ const unsigned int s_retry_type = retry_type(s);
+ // If simple or both, check if code is simple-retryable and for retry
attempts
+ if ((s_retry_type & PARENT_RETRY_SIMPLE) &&
s->parent_result.response_is_retryable(PARENT_RETRY_SIMPLE, response_code) &&
s->current.simple_retry_attempts < max_retries(s, PARENT_RETRY_SIMPLE)) {
+ TxnDebug("http_trans", "saw parent retry simple first in trans");
if (s->current.simple_retry_attempts < numParents(s)) {
return PARENT_RETRY_SIMPLE;
}
return PARENT_RETRY_NONE;
}
- if ((s_retry_type & PARENT_RETRY_UNAVAILABLE_SERVER) &&
is_response_unavailable_code(server_response) &&
+ // If unavailable or both, check if code is unavailable-retryable AND also
not simple-retryable, then unavailable retry attempts
+ if ((s_retry_type & PARENT_RETRY_UNAVAILABLE_SERVER) &&
+ s->parent_result.response_is_retryable(PARENT_RETRY_UNAVAILABLE_SERVER,
response_code) &&
+ !s->parent_result.response_is_retryable(PARENT_RETRY_SIMPLE,
response_code) &&
s->current.unavailable_server_retry_attempts < max_retries(s,
PARENT_RETRY_UNAVAILABLE_SERVER)) {
+ TxnDebug("http_trans", "saw parent retry unavailable first in trans");
if (s->current.unavailable_server_retry_attempts < numParents(s)) {
return PARENT_RETRY_UNAVAILABLE_SERVER;
}
