Skip to content

Add 5xx's to be allowed to be used for simple retries #8518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ezelkow1 merged 3 commits into from
Nov 16, 2021
+26 −36
Merged

Add 5xx's to be allowed to be used for simple retries #8518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc52e8f
Add 5xx's to be allowed to be used for simple retries
ezelkow1 Nov 2, 2021
cec155b
Fix clang format
ezelkow1 Nov 15, 2021
82d7663
Remove trailing whitespace in docs
ezelkow1 Nov 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions doc/admin-guide/files/parent.config.en.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -223,11 +223,17 @@ The following list shows the possible actions and their allowed values.
retries is controlled by ``max_unavailable_server_retries`` which is set to 1 by default.
- ``both`` - This enables both ``simple_retry`` and ``unavailable_server_retry`` as described above.

.. 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:
Expand Down
6 changes: 3 additions & 3 deletions proxy/ParentSelection.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
10 changes: 5 additions & 5 deletions proxy/ParentSelection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,17 +324,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 {
Expand Down
36 changes: 10 additions & 26 deletions proxy/http/HttpTransact.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}
Expand Down