You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jr...@apache.org on 2016/04/14 18:23:58 UTC

[trafficserver] branch master updated: TS-4287: Add a simple and unavailable server retry feature to Parent Selection.

This is an automated email from the ASF dual-hosted git repository.

jrushford pushed a commit to branch master
in repository https://git-dual.apache.org/repos/asf/trafficserver.git

The following commit(s) were added to refs/heads/master by this push:
       new  a387890   TS-4287: Add a simple and unavailable server retry feature to Parent Selection.
a387890 is described below

commit a387890c51fe47843b92ec07327334d3247aea53
Author: John J. Rushford <Jo...@cable.comcast.com>
AuthorDate: Thu Apr 14 16:11:17 2016 +0000

    TS-4287: Add a simple and unavailable server retry feature to Parent Selection.
---
 doc/admin-guide/files/parent.config.en.rst | 40 +++++++++++++
 proxy/ParentSelection.cc                   | 91 +++++++++++++++++++++++++++++-
 proxy/ParentSelection.h                    | 32 ++++++++++-
 proxy/http/HttpDebugNames.cc               |  2 +
 proxy/http/HttpTransact.cc                 | 77 ++++++++++++++++++++++++-
 proxy/http/HttpTransact.h                  | 15 ++++-
 6 files changed, 252 insertions(+), 5 deletions(-)

diff --git a/doc/admin-guide/files/parent.config.en.rst b/doc/admin-guide/files/parent.config.en.rst
index a8554f8..c3df065 100644
--- a/doc/admin-guide/files/parent.config.en.rst
+++ b/doc/admin-guide/files/parent.config.en.rst
@@ -150,6 +150,46 @@ The following list shows the possible actions and their allowed values.
         the specified ``round_robin`` algorithm.  The FQDN is removed from
         the http request line.
 
+.. _parent-config-format-parent-retry:
+
+``parent_retry``
+  If ``parent_is_proxy`` is false, then you may configure ``parent_retry`` for one
+  of the following values:
+
+    - ``simple_retry`` - If the parent origin server returns a 404 response 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 reponse matches
+      a list of http 5xx responses defined in ``unavailable_server_retry_responses``, the currently selected
+      parent is marked down and a new parent is selected to retry the request.  The number of
+      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.
+
+.. _parent-config-format-unavailable-server-retry-responses:
+
+``unavailable_server_retry_responses``
+  If ``parent_is_proxy`` is false and ``parent_retry`` is set to either ``unavailable_server_retry`` or
+  ``both``, this parameter is a comma separated list of http 5xx response codes that will invoke the
+  ``unavailable_server_retry`` described in the ``parent_retry`` section.  By default, ``unavailable_server_retry_responses``
+  is set to 503.
+
+.. _parent-config-format-max-simple-retries:
+
+``max_simple_retries``
+  By default the value for ``max_simple_retries`` is 1.  It may be set to any value in the range 1 to 5.
+  If ``parent_is_proxy`` is false and ``parent_retry`` is set to ``simple_retry`` or ``both`` a 404 reponse
+  from a parent origin server will cause the request to be retried using a new parent at most 1 to 5
+  times as configured by ``max_simple_retries``.
+
+.. _parent-config-format-max-unavailable-server-retries:
+
+``max_unavailable_server_retries``
+  By default the value for ``max_unavailable_server_retries`` is 1.  It may be set to any value in the range 1 to 5.
+  If ``parent_is_proxy`` is false and ``parent_retry`` is set to ``unavailable_server_retries`` or ``both`` a 503 reponse
+  by default or any http 5xx response listed in the list ``unavailable_server_retry_responses`` from a parent origin server will
+  cause the request to be retried using a new parent after first marking the current parent down.  The request
+  will be retried at most 1 to 5 times as configured by ``max_unavailable_server_retries``.
+
 .. _parent-config-format-round-robin:
 
 ``round_robin``
diff --git a/proxy/ParentSelection.cc b/proxy/ParentSelection.cc
index 8fdfe58..370a46e 100644
--- a/proxy/ParentSelection.cc
+++ b/proxy/ParentSelection.cc
@@ -35,6 +35,9 @@
 #define PARENT_ReadConfigInteger REC_ReadConfigInteger
 #define PARENT_ReadConfigStringAlloc REC_ReadConfigStringAlloc
 
+#define MAX_SIMPLE_RETRIES 5
+#define MAX_UNAVAILABLE_SERVER_RETRIES 5
+
 typedef ControlMatcher<ParentRecord, ParentResult> P_table;
 
 // Global Vars for Parent Selection
@@ -324,6 +327,32 @@ ParentConfig::print()
   ParentConfig::release(params);
 }
 
+UnavailableServerResponseCodes::UnavailableServerResponseCodes(char *val)
+{
+  Tokenizer pTok(", \t\r");
+  int numTok = 0, c;
+
+  if (val == NULL) {
+    Warning("UnavailableServerResponseCodes - unavailable_server_retry_responses is null loading default 503 code.");
+    codes.push_back(HTTP_STATUS_SERVICE_UNAVAILABLE);
+    return;
+  }
+  numTok = pTok.Initialize(val, SHARE_TOKS);
+  if (numTok == 0) {
+    c = atoi(val);
+    if (c > 500 && c < 600)
+      codes.push_back(HTTP_STATUS_SERVICE_UNAVAILABLE);
+  }
+  for (int i = 0; i < numTok; i++) {
+    c = atoi(pTok[i]);
+    if (c > 500 && c < 600) {
+      TSDebug("parent_select", "loading response code: %d", c);
+      codes.push_back(c);
+    }
+  }
+  std::sort(codes.begin(), codes.end());
+}
+
 // const char* ParentRecord::ProcessParents(char* val, bool isPrimary)
 //
 //   Reads in the value of a "round-robin" or "order"
@@ -565,11 +594,46 @@ ParentRecord::Init(matcher_line *line_info)
       if (strcasecmp(val, "false") == 0) {
         parent_is_proxy = false;
       } else if (strcasecmp(val, "true") != 0) {
-        errPtr = "invalid argument to parent_is_proxy directed";
+        errPtr = "invalid argument to parent_is_proxy directive";
       } else {
         parent_is_proxy = true;
       }
       used = true;
+    } else if (strcasecmp(label, "parent_retry") == 0) {
+      if (strcasecmp(val, "simple_retry") == 0) {
+        parent_retry = PARENT_RETRY_SIMPLE;
+      } else if (strcasecmp(val, "unavailable_server_retry") == 0) {
+        parent_retry = PARENT_RETRY_UNAVAILABLE_SERVER;
+      } else if (strcasecmp(val, "both") == 0) {
+        parent_retry = PARENT_RETRY_BOTH;
+      } else {
+        errPtr = "invalid argument to parent_retry directive.";
+      }
+      used = true;
+    } else if (strcasecmp(label, "unavailable_server_retry_responses") == 0 && unavailable_server_retry_responses == NULL) {
+      unavailable_server_retry_responses = new UnavailableServerResponseCodes(val);
+      used = true;
+    } else if (strcasecmp(label, "max_simple_retries") == 0) {
+      int v = atoi(val);
+      if (v >= 1 && v < MAX_SIMPLE_RETRIES) {
+        max_simple_retries = v;
+        used = true;
+      } else {
+        char buf[128];
+        sprintf(buf, "invalid argument to max_simple_retries.  Argument must be between 1 and %d.", MAX_SIMPLE_RETRIES);
+        errPtr = buf;
+      }
+    } else if (strcasecmp(label, "max_unavailable_server_retries") == 0) {
+      int v = atoi(val);
+      if (v >= 1 && v < MAX_UNAVAILABLE_SERVER_RETRIES) {
+        max_unavailable_server_retries = v;
+        used = true;
+      } else {
+        char buf[128];
+        sprintf(buf, "invalid argument to max_unavailable_server_retries.  Argument must be between 1 and %d.",
+                MAX_UNAVAILABLE_SERVER_RETRIES);
+        errPtr = buf;
+      }
     }
     // Report errors generated by ProcessParents();
     if (errPtr != NULL) {
@@ -583,6 +647,30 @@ ParentRecord::Init(matcher_line *line_info)
     }
   }
 
+  // parent_retry may only be enabled if the parents are origin servers, parent_is_proxy is false.
+  if (parent_is_proxy == true) {
+    if (parent_retry > 0) {
+      parent_retry = PARENT_RETRY_NONE;
+      if (unavailable_server_retry_responses != NULL) {
+        delete unavailable_server_retry_responses;
+        unavailable_server_retry_responses = NULL;
+      }
+    }
+    Warning("%s disabling parent_retry on line %d because parent_is_proxy is true", modulePrefix, line_num);
+  } else {
+    // delete unavailable_server_retry_responses if unavailable_server_retry is not enabled.
+    if (unavailable_server_retry_responses != NULL && (!parent_retry & PARENT_RETRY_UNAVAILABLE_SERVER)) {
+      Warning("%s ignoring unavailable_server_retry_responses directive on line %d, as unavailable_server_retry is not enabled.",
+              modulePrefix, line_num);
+      delete unavailable_server_retry_responses;
+      unavailable_server_retry_responses = NULL;
+    } else if (unavailable_server_retry_responses == NULL && (parent_retry & PARENT_RETRY_UNAVAILABLE_SERVER)) {
+      // initialize UnavailableServerResponseCodes to the default value if unavailable_server_retry is enabled.
+      Warning("%s initializing UnavailableServerResponseCodes on line %d to 503 default.", modulePrefix, line_num);
+      unavailable_server_retry_responses = new UnavailableServerResponseCodes(NULL);
+    }
+  }
+
   if (this->parents == NULL && go_direct == false) {
     return config_parse_error("%s No parent specified in parent.config at line %d", modulePrefix, line_num);
   }
@@ -647,6 +735,7 @@ ParentRecord::~ParentRecord()
 {
   ats_free(parents);
   delete selection_strategy;
+  delete unavailable_server_retry_responses;
 }
 
 void
diff --git a/proxy/ParentSelection.h b/proxy/ParentSelection.h
index af0ea8e..62b67ff 100644
--- a/proxy/ParentSelection.h
+++ b/proxy/ParentSelection.h
@@ -40,6 +40,9 @@
 #include "ts/Tokenizer.h"
 #include "ts/ink_apidefs.h"
 
+#include <algorithm>
+#include <vector>
+
 #define MAX_PARENTS 64
 
 struct RequestData;
@@ -63,6 +66,28 @@ enum ParentRR_t {
   P_CONSISTENT_HASH,
 };
 
+enum ParentRetry_t {
+  PARENT_RETRY_NONE = 0,
+  PARENT_RETRY_SIMPLE = 1,
+  PARENT_RETRY_UNAVAILABLE_SERVER = 2,
+  // both simple and unavailable server retry
+  PARENT_RETRY_BOTH = 3
+};
+
+struct UnavailableServerResponseCodes {
+  UnavailableServerResponseCodes(char *val);
+  ~UnavailableServerResponseCodes(){};
+
+  bool
+  contains(int code)
+  {
+    return binary_search(codes.begin(), codes.end(), code);
+  }
+
+private:
+  std::vector<int> codes;
+};
+
 // struct pRecord
 //
 //    A record for an invidual parent
@@ -90,7 +115,8 @@ class ParentRecord : public ControlBase
 public:
   ParentRecord()
     : parents(NULL), secondary_parents(NULL), num_parents(0), num_secondary_parents(0), ignore_query(false), rr_next(0),
-      go_direct(true), parent_is_proxy(true), selection_strategy(NULL)
+      go_direct(true), parent_is_proxy(true), selection_strategy(NULL), unavailable_server_retry_responses(NULL), parent_retry(0),
+      max_simple_retries(1), max_unavailable_server_retries(1)
   {
   }
 
@@ -119,6 +145,10 @@ public:
   bool go_direct;
   bool parent_is_proxy;
   ParentSelectionStrategy *selection_strategy;
+  UnavailableServerResponseCodes *unavailable_server_retry_responses;
+  int parent_retry;
+  int max_simple_retries;
+  int max_unavailable_server_retries;
 };
 
 // If the parent was set by the external customer api,
diff --git a/proxy/http/HttpDebugNames.cc b/proxy/http/HttpDebugNames.cc
index 7db357d..da4e178 100644
--- a/proxy/http/HttpDebugNames.cc
+++ b/proxy/http/HttpDebugNames.cc
@@ -59,6 +59,8 @@ HttpDebugNames::get_server_state_name(HttpTransact::ServerState_t state)
     return "CONGEST_CONTROL_CONGESTED_ON_F";
   case HttpTransact::CONGEST_CONTROL_CONGESTED_ON_M:
     return "CONGEST_CONTROL_CONGESTED_ON_M";
+  case HttpTransact::PARENT_ORIGIN_RETRY:
+    return "PARENT_ORIGIN_RETRY";
   }
 
   return ("unknown state name");
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 085ebb5..4a068a7 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -65,6 +65,48 @@ extern HttpBodyFactory *body_factory;
 
 static const char local_host_ip_str[] = "127.0.0.1";
 
+inline static void
+simple_or_unavailable_server_retry(HttpTransact::State *s)
+{
+  int server_response = 0;
+
+  HTTP_RELEASE_ASSERT(!s->parent_result.rec->parent_is_proxy);
+
+  // reponse is from a parent origin server.
+  server_response = http_hdr_status_get(s->hdr_info.server_response.m_http);
+
+  DebugTxn("http_trans", "[simple_or_unavailabe_server_retry] server_response = %d, simple_retry_attempts: %d, numParents:%d \n",
+           server_response, s->current.simple_retry_attempts, s->parent_params->numParents(&s->parent_result));
+
+  // simple retry is enabled, 0x1
+  if ((s->parent_result.rec->parent_retry & HttpTransact::PARENT_ORIGIN_SIMPLE_RETRY) &&
+      (s->current.simple_retry_attempts < s->parent_result.rec->max_simple_retries && server_response == HTTP_STATUS_NOT_FOUND)) {
+    DebugTxn("parent_select", "RECEIVED A SIMPLE RETRY RESPONSE");
+    if (s->current.simple_retry_attempts < (int)s->parent_params->numParents(&s->parent_result)) {
+      s->current.state = HttpTransact::PARENT_ORIGIN_RETRY;
+      s->current.retry_type = HttpTransact::PARENT_ORIGIN_SIMPLE_RETRY;
+      return;
+    } else {
+      DebugTxn("http_trans", "PARENT_ORIGIN_SIMPLE_RETRY: retried all parents, send response to client.\n");
+      return;
+    }
+  }
+  // unavailable server retry is enabled 0x2
+  else if ((s->parent_result.rec->parent_retry & HttpTransact::PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY) &&
+           (s->current.unavailable_server_retry_attempts < s->parent_result.rec->max_unavailable_server_retries &&
+            s->parent_result.rec->unavailable_server_retry_responses->contains(server_response))) {
+    DebugTxn("parent_select", "RECEIVED A PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY RESPONSE");
+    if (s->current.unavailable_server_retry_attempts < (int)s->parent_params->numParents(&s->parent_result)) {
+      s->current.state = HttpTransact::PARENT_ORIGIN_RETRY;
+      s->current.retry_type = HttpTransact::PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY;
+      return;
+    } else {
+      DebugTxn("http_trans", "PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY: retried all parents, send error to client.\n");
+      return;
+    }
+  }
+}
+
 inline static bool
 is_request_conditional(HTTPHdr *header)
 {
@@ -3520,6 +3562,15 @@ HttpTransact::handle_response_from_parent(State *s)
   DebugTxn("http_trans", "[handle_response_from_parent] (hrfp)");
   HTTP_RELEASE_ASSERT(s->current.server == &s->parent_info);
 
+  // response is from a parent origin server.
+  if (is_response_valid(s, &s->hdr_info.server_response) && s->current.request_to == HttpTransact::PARENT_PROXY &&
+      !s->parent_result.rec->parent_is_proxy) {
+    // check for a retryable response if simple or unavailable server retry are enabled.
+    if (s->parent_result.rec->parent_retry & (PARENT_ORIGIN_SIMPLE_RETRY | PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY)) {
+      simple_or_unavailable_server_retry(s);
+    }
+  }
+
   s->parent_info.state = s->current.state;
   switch (s->current.state) {
   case CONNECTION_ALIVE:
@@ -3552,7 +3603,31 @@ HttpTransact::handle_response_from_parent(State *s)
       return;
     }
 
-    if (s->current.attempts < s->http_config_param->parent_connect_attempts) {
+    // try a simple retry if we received a simple retryable response from the parent.
+    if (s->current.retry_type == PARENT_ORIGIN_SIMPLE_RETRY || s->current.retry_type == PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY) {
+      if (s->current.retry_type == PARENT_ORIGIN_SIMPLE_RETRY) {
+        if (s->current.simple_retry_attempts >= s->parent_result.rec->max_simple_retries) {
+          DebugTxn("http_trans", "PARENT_ORIGIN_SIMPLE_RETRY: retried all parents, send error to client.\n");
+          s->current.retry_type = PARENT_ORIGIN_UNDEFINED_RETRY;
+        } else {
+          s->current.simple_retry_attempts++;
+          DebugTxn("http_trans", "PARENT_ORIGIN_SIMPLE_RETRY: try another parent.\n");
+          s->current.retry_type = PARENT_ORIGIN_UNDEFINED_RETRY;
+          next_lookup = find_server_and_update_current_info(s);
+        }
+      } else { // try unavailable server retry if we have a unavailable server retry response from the parent.
+        if (s->current.unavailable_server_retry_attempts >= s->parent_result.rec->max_unavailable_server_retries) {
+          DebugTxn("http_trans", "PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY: retried all parents, send error to client.\n");
+          s->current.retry_type = PARENT_ORIGIN_UNDEFINED_RETRY;
+        } else {
+          s->current.unavailable_server_retry_attempts++;
+          DebugTxn("http_trans", "PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY: marking parent down and trying another.\n");
+          s->current.retry_type = PARENT_ORIGIN_UNDEFINED_RETRY;
+          s->parent_params->markParentDown(&s->parent_result);
+          next_lookup = find_server_and_update_current_info(s);
+        }
+      }
+    } else if (s->current.attempts < s->http_config_param->parent_connect_attempts) {
       s->current.attempts++;
 
       // Are we done with this particular parent?
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 7a1891f..df5b928 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -350,6 +350,12 @@ public:
     HTTP_TRANSACT_MAGIC_SEPARATOR = 0x12345678
   };
 
+  enum ParentOriginRetry_t {
+    PARENT_ORIGIN_UNDEFINED_RETRY = 0x0,
+    PARENT_ORIGIN_SIMPLE_RETRY = 0x1,
+    PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY = 0x2
+  };
+
   enum LookingUp_t {
     ORIGIN_SERVER,
     UNDEFINED_LOOKUP,
@@ -407,7 +413,8 @@ public:
     PARSE_ERROR,
     TRANSACTION_COMPLETE,
     CONGEST_CONTROL_CONGESTED_ON_F,
-    CONGEST_CONTROL_CONGESTED_ON_M
+    CONGEST_CONTROL_CONGESTED_ON_M,
+    PARENT_ORIGIN_RETRY
   };
 
   enum CacheWriteStatus_t {
@@ -682,9 +689,13 @@ public:
     ink_time_t now;
     ServerState_t state;
     int attempts;
+    int simple_retry_attempts;
+    int unavailable_server_retry_attempts;
+    ParentOriginRetry_t retry_type;
 
     _CurrentInfo()
-      : mode(UNDEFINED_MODE), request_to(UNDEFINED_LOOKUP), server(NULL), now(0), state(STATE_UNDEFINED), attempts(1){};
+      : mode(UNDEFINED_MODE), request_to(UNDEFINED_LOOKUP), server(NULL), now(0), state(STATE_UNDEFINED), attempts(1),
+        simple_retry_attempts(0), unavailable_server_retry_attempts(0), retry_type(PARENT_ORIGIN_UNDEFINED_RETRY){};
   } CurrentInfo;
 
   typedef struct _DNSLookupInfo {

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].