diff --git a/doc/admin-guide/files/parent.config.en.rst b/doc/admin-guide/files/parent.config.en.rst index 39d33e36543..50fa96fe737 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 @@ -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: diff --git a/proxy/ParentSelection.cc b/proxy/ParentSelection.cc index 29f5bdf7892..48de7ab548a 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 954e19cdb85..8b7748f3ea7 100644 --- a/proxy/ParentSelection.h +++ b/proxy/ParentSelection.h @@ -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 { diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index b1f161d7fba..4f6a5233da5 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(response_code) < 400 || static_cast(response_code) > 499) { - return false; - } - - return true; -} - -inline static bool -is_response_unavailable_code(HTTPStatus response_code) -{ - if (static_cast(response_code) < 500 || static_cast(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; }