Add 5xx's to be allowed to be used for simple retries (apache#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

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:

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());

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 {

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;
     }