You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2016/05/02 18:55:48 UTC

[trafficserver] branch master updated (8c3d81c -> 7a590ed)

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

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

      from  8c3d81c   Build fix for test_mime on Ubuntu 14.04.
       new  ab4b69e   TS-4388: Remove useless record API defines.
       new  0689f63   TS-4388: Remove unused HttpConfig::parent_proxy_routing_enable.
       new  060b8e8   TS-4388: Fix global hook handling in API tests.
       new  41e013d   TS-4388: Rename ParentResult:r.
       new  2bf1a31   TS-4388: Refactor ParentResult initialization.
       new  7a590ed   TS-4388: TSHttpTxnParentProxySet crashes in parent selection.

The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "adds" were already present in the repository and have only
been added to this reference.


Summary of changes:
 iocore/net/Socks.cc           |   6 +--
 proxy/InkAPITest.cc           | 116 ++++++++++++++++++++++--------------------
 proxy/InkAPITestTool.cc       |  38 ++++++++++----
 proxy/ParentConsistentHash.cc |  24 ++++-----
 proxy/ParentRoundRobin.cc     |  26 +++++-----
 proxy/ParentSelection.cc      |  68 +++++++++++--------------
 proxy/ParentSelection.h       | 104 +++++++++++++++++++++++++++++++++----
 proxy/http/HttpConfig.cc      |   3 --
 proxy/http/HttpConfig.h       |   7 ++-
 proxy/http/HttpSM.cc          |   6 +--
 proxy/http/HttpTransact.cc    |  77 ++++++++++++++--------------
 proxy/http/HttpTransact.h     |   8 +--
 12 files changed, 288 insertions(+), 195 deletions(-)

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

[trafficserver] 02/06: TS-4388: Remove unused HttpConfig::parent_proxy_routing_enable.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 0689f632cb40922b0c1aa5589f83059838d37047
Author: James Peach <jp...@apache.org>
AuthorDate: Wed Apr 27 11:20:25 2016 -0700

    TS-4388: Remove unused HttpConfig::parent_proxy_routing_enable.
---
 proxy/http/HttpConfig.cc | 3 ---
 proxy/http/HttpConfig.h  | 7 +++----
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc
index 885ff3c..d40f9e9 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -888,8 +888,6 @@ HttpConfig::startup()
   HttpEstablishStaticConfigLongLong(c.origin_min_keep_alive_connections, "proxy.config.http.origin_min_keep_alive_connections");
   HttpEstablishStaticConfigLongLong(c.oride.attach_server_session_to_client, "proxy.config.http.attach_server_session_to_client");
 
-  HttpEstablishStaticConfigByte(c.parent_proxy_routing_enable, "proxy.config.http.parent_proxy_routing_enable");
-
   // Wank me.
   HttpEstablishStaticConfigByte(c.disable_ssl_parenting, "proxy.local.http.parent_proxy.disable_connect_tunneling");
   HttpEstablishStaticConfigByte(c.no_dns_forward_to_parent, "proxy.config.http.no_dns_just_forward_to_parent");
@@ -1173,7 +1171,6 @@ HttpConfig::reconfigure()
     params->origin_min_keep_alive_connections = params->oride.origin_max_connections;
   }
 
-  params->parent_proxy_routing_enable = INT_TO_BOOL(m_master.parent_proxy_routing_enable);
   params->enable_url_expandomatic = INT_TO_BOOL(m_master.enable_url_expandomatic);
 
   params->oride.insert_request_via_string = m_master.oride.insert_request_via_string;
diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h
index 17d327a..ef3c608 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -638,7 +638,6 @@ public:
   MgmtInt origin_min_keep_alive_connections; // TODO: This one really ought to be overridable, but difficult right now.
   MgmtInt max_websocket_connections;
 
-  MgmtByte parent_proxy_routing_enable;
   MgmtByte disable_ssl_parenting;
 
   MgmtByte enable_url_expandomatic;
@@ -852,9 +851,9 @@ extern volatile int32_t icp_dynamic_enabled;
 /////////////////////////////////////////////////////////////
 inline HttpConfigParams::HttpConfigParams()
   : proxy_hostname(NULL), proxy_hostname_len(0), server_max_connections(0), origin_min_keep_alive_connections(0),
-    max_websocket_connections(-1), parent_proxy_routing_enable(0), disable_ssl_parenting(0), enable_url_expandomatic(0),
-    no_dns_forward_to_parent(0), uncacheable_requests_bypass_parent(1), no_origin_server_dns(0), use_client_target_addr(0),
-    use_client_source_port(0), proxy_request_via_string(NULL), proxy_request_via_string_len(0), proxy_response_via_string(NULL),
+    max_websocket_connections(-1), disable_ssl_parenting(0), enable_url_expandomatic(0), no_dns_forward_to_parent(0),
+    uncacheable_requests_bypass_parent(1), no_origin_server_dns(0), use_client_target_addr(0), use_client_source_port(0),
+    proxy_request_via_string(NULL), proxy_request_via_string_len(0), proxy_response_via_string(NULL),
     proxy_response_via_string_len(0), url_expansions_string(NULL), url_expansions(NULL), num_url_expansions(0),
     session_auth_cache_keep_alive_enabled(1), transaction_active_timeout_in(900), accept_no_activity_timeout(120),
     parent_connect_attempts(4), per_parent_connect_attempts(2), parent_connect_timeout(30), anonymize_other_header_list(NULL),

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

[trafficserver] 06/06: TS-4388: TSHttpTxnParentProxySet crashes in parent selection.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 7a590edc76843233524f6ba093e57e523d560ba3
Author: James Peach <jp...@apache.org>
AuthorDate: Wed Apr 27 15:16:32 2016 -0700

    TS-4388: TSHttpTxnParentProxySet crashes in parent selection.
    
    Fix ParentResult to handle the case where the parent is specified
    by the TSHttpTxnParentProxySet API. Encapsulate the internals of
    the result better so that it is easier for the HTTP state machine
    to do the right thing.
---
 proxy/InkAPITest.cc           |  6 ---
 proxy/ParentConsistentHash.cc |  4 +-
 proxy/ParentRoundRobin.cc     |  4 +-
 proxy/ParentSelection.cc      |  2 +-
 proxy/ParentSelection.h       | 87 +++++++++++++++++++++++++++++++++++++++++--
 proxy/http/HttpTransact.cc    | 49 ++++++++++++------------
 proxy/http/HttpTransact.h     |  8 ++--
 7 files changed, 117 insertions(+), 43 deletions(-)

diff --git a/proxy/InkAPITest.cc b/proxy/InkAPITest.cc
index 342e824..e635219 100644
--- a/proxy/InkAPITest.cc
+++ b/proxy/InkAPITest.cc
@@ -5982,12 +5982,6 @@ parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
 
 EXCLUSIVE_REGRESSION_TEST(SDK_API_HttpParentProxySet)(RegressionTest *test, int level, int *pstatus)
 {
-  // Don't enable this test by default until it passes.
-  if (level < REGRESSION_TEST_FATAL) {
-    *pstatus = REGRESSION_TEST_NOT_RUN;
-    return;
-  }
-
   *pstatus = REGRESSION_TEST_INPROGRESS;
 
   TSCont cont = TSContCreate(parent_proxy_handler, TSMutexCreate());
diff --git a/proxy/ParentConsistentHash.cc b/proxy/ParentConsistentHash.cc
index 1b5e396..e145cbc 100644
--- a/proxy/ParentConsistentHash.cc
+++ b/proxy/ParentConsistentHash.cc
@@ -236,7 +236,7 @@ ParentConsistentHash::markParentDown(const ParentSelectionPolicy *policy, Parent
   }
   // If we were set through the API we currently have not failover
   //   so just return fail
-  if (result->rec == extApiRecord) {
+  if (result->is_api_result()) {
     return;
   }
 
@@ -312,7 +312,7 @@ ParentConsistentHash::markParentUp(ParentResult *result)
   }
   // If we were set through the API we currently have not failover
   //   so just return fail
-  if (result->rec == extApiRecord) {
+  if (result->is_api_result()) {
     ink_assert(0);
     return;
   }
diff --git a/proxy/ParentRoundRobin.cc b/proxy/ParentRoundRobin.cc
index 0716d78..c3ecf6d 100644
--- a/proxy/ParentRoundRobin.cc
+++ b/proxy/ParentRoundRobin.cc
@@ -198,7 +198,7 @@ ParentRoundRobin::markParentDown(const ParentSelectionPolicy *policy, ParentResu
   }
   // If we were set through the API we currently have not failover
   //   so just return fail
-  if (result->rec == extApiRecord) {
+  if (result->is_api_result()) {
     return;
   }
 
@@ -257,7 +257,7 @@ ParentRoundRobin::markParentUp(ParentResult *result)
   }
   // If we were set through the API we currently have not failover
   //   so just return fail
-  if (result->rec == extApiRecord) {
+  if (result->is_api_result()) {
     ink_assert(0);
     return;
   }
diff --git a/proxy/ParentSelection.cc b/proxy/ParentSelection.cc
index bbedbaf..705fa4c 100644
--- a/proxy/ParentSelection.cc
+++ b/proxy/ParentSelection.cc
@@ -195,7 +195,7 @@ ParentConfigParams::nextParent(HttpRequestData *rdata, ParentResult *result)
   }
   // If we were set through the API we currently have not failover
   //   so just return fail
-  if (result->rec == extApiRecord) {
+  if (result->is_api_result()) {
     Debug("parent_select", "Retry result for %s was %s", rdata->get_host(), ParentResultStr[result->result]);
     result->result = PARENT_FAIL;
     return;
diff --git a/proxy/ParentSelection.h b/proxy/ParentSelection.h
index 975b07a..ccee7d9 100644
--- a/proxy/ParentSelection.h
+++ b/proxy/ParentSelection.h
@@ -115,8 +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), unavailable_server_retry_responses(NULL), parent_retry(0),
-      max_simple_retries(1), max_unavailable_server_retries(1)
+      go_direct(true), parent_is_proxy(true), selection_strategy(NULL), unavailable_server_retry_responses(NULL),
+      parent_retry(PARENT_RETRY_NONE), max_simple_retries(1), max_unavailable_server_retries(1)
   {
   }
 
@@ -146,7 +146,7 @@ public:
   bool parent_is_proxy;
   ParentSelectionStrategy *selection_strategy;
   UnavailableServerResponseCodes *unavailable_server_retry_responses;
-  int parent_retry;
+  ParentRetry_t parent_retry;
   int max_simple_retries;
   int max_unavailable_server_retries;
 };
@@ -173,6 +173,82 @@ struct ParentResult {
     result = PARENT_UNDEFINED;
   }
 
+  bool
+  is_api_result() const
+  {
+    return rec == extApiRecord;
+  }
+
+  // Do we have some result?
+  bool
+  is_some() const
+  {
+    if (rec == NULL) {
+      // If we don't have a result, we either haven't done a parent
+      // lookup yet (PARENT_UNDEFINED), or the lookup didn't match
+      // anything (PARENT_DIRECT).
+      ink_assert(result == PARENT_UNDEFINED || result == PARENT_DIRECT);
+      return false;
+    }
+
+    return true;
+  }
+
+  bool
+  parent_is_proxy() const
+  {
+    // Parents set by the TSHttpTxnParentProxySet API are always considered proxies rather than origins.
+    return is_api_result() ? true : rec->parent_is_proxy;
+  }
+
+  unsigned
+  retry_type() const
+  {
+    return is_api_result() ? PARENT_RETRY_NONE : rec->parent_retry;
+  }
+
+  unsigned
+  max_retries(ParentRetry_t method) const
+  {
+    // There's no API for specifying the retries, so you get 0.
+    if (is_api_result()) {
+      return 0;
+    }
+
+    switch (method) {
+    case PARENT_RETRY_NONE:
+      return 0;
+    case PARENT_RETRY_SIMPLE:
+      return rec->max_simple_retries;
+    case PARENT_RETRY_UNAVAILABLE_SERVER:
+      return rec->max_unavailable_server_retries;
+    case PARENT_RETRY_BOTH:
+      return std::max(rec->max_unavailable_server_retries, rec->max_simple_retries);
+    }
+
+    return 0;
+  }
+
+  bool
+  response_is_retryable(HTTPStatus response_code) const
+  {
+    return (retry_type() & PARENT_RETRY_UNAVAILABLE_SERVER) && rec->unavailable_server_retry_responses->contains(response_code);
+  }
+
+  bool
+  bypass_ok() const
+  {
+    if (is_api_result()) {
+      return false;
+    } else {
+      // Caller should check for a valid result beforehand.
+      ink_assert(result != PARENT_UNDEFINED);
+      ink_assert(is_some());
+      return rec->bypass_ok();
+    }
+  }
+
+private:
   // Internal use only
   //   Not to be modified by HTTP
   int line_number;
@@ -181,6 +257,11 @@ struct ParentResult {
   uint32_t start_parent;
   bool wrap_around;
   int last_lookup; // state for for consistent hash.
+
+  friend class ParentConsistentHash;
+  friend class ParentRoundRobin;
+  friend class ParentConfigParams;
+  friend class ParentRecord;
 };
 
 struct ParentSelectionPolicy {
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 5a333c3..1985bd3 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -68,21 +68,20 @@ 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);
+  HTTP_RELEASE_ASSERT(!s->parent_result.parent_is_proxy());
 
   // reponse is from a parent origin server.
-  server_response = http_hdr_status_get(s->hdr_info.server_response.m_http);
+  HTTPStatus 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)) {
+  if ((s->parent_result.retry_type() & HttpTransact::PARENT_ORIGIN_SIMPLE_RETRY) &&
+      s->current.simple_retry_attempts < s->parent_result.max_retries(PARENT_RETRY_SIMPLE) &&
+      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)) {
+    if (s->current.simple_retry_attempts < s->parent_params->numParents(&s->parent_result)) {
       s->current.state = HttpTransact::PARENT_ORIGIN_RETRY;
       s->current.retry_type = HttpTransact::PARENT_ORIGIN_SIMPLE_RETRY;
       return;
@@ -92,11 +91,11 @@ simple_or_unavailable_server_retry(HttpTransact::State *s)
     }
   }
   // 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))) {
+  else if ((s->parent_result.retry_type() & HttpTransact::PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY) &&
+           s->current.unavailable_server_retry_attempts < s->parent_result.max_retries(PARENT_RETRY_UNAVAILABLE_SERVER) &&
+           s->parent_result.response_is_retryable(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)) {
+    if (s->current.unavailable_server_retry_attempts < 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;
@@ -254,7 +253,7 @@ find_server_and_update_current_info(HttpTransact::State *s)
     s->parent_result.result = PARENT_DIRECT;
   } else if (s->method == HTTP_WKSIDX_CONNECT && s->http_config_param->disable_ssl_parenting) {
     s->parent_params->findParent(&s->request_data, &s->parent_result);
-    if (s->parent_result.rec == NULL || s->parent_result.rec->parent_is_proxy) {
+    if (!s->parent_result.is_some() || s->parent_result.is_api_result() || s->parent_result.parent_is_proxy()) {
       DebugTxn("http_trans", "request not cacheable, so bypass parent");
       s->parent_result.result = PARENT_DIRECT;
     }
@@ -267,7 +266,7 @@ find_server_and_update_current_info(HttpTransact::State *s)
     // with respect to whether a request is cacheable or not.
     // For example, the cache_urls_that_look_dynamic variable.
     s->parent_params->findParent(&s->request_data, &s->parent_result);
-    if (s->parent_result.rec == NULL || s->parent_result.rec->parent_is_proxy) {
+    if (!s->parent_result.is_some() || s->parent_result.is_api_result() || s->parent_result.parent_is_proxy()) {
       DebugTxn("http_trans", "request not cacheable, so bypass parent");
       s->parent_result.result = PARENT_DIRECT;
     }
@@ -291,11 +290,11 @@ find_server_and_update_current_info(HttpTransact::State *s)
     case PARENT_FAIL:
       // Check to see if should bypass the parent and go direct
       //   We can only do this if
-      //   1) the parent was not set from API
+      //   1) the config permitted us to dns the origin server
       //   2) the config permits us
-      //   3) the config permitted us to dns the origin server
-      if (!s->parent_params->apiParentExists(&s->request_data) && s->parent_result.rec->bypass_ok() &&
-          s->http_config_param->no_dns_forward_to_parent == 0 && s->parent_result.rec->parent_is_proxy) {
+      //   3) the parent was not set from API
+      if (s->http_config_param->no_dns_forward_to_parent == 0 && s->parent_result.bypass_ok() &&
+          s->parent_result.parent_is_proxy() && !s->parent_params->apiParentExists(&s->request_data)) {
         s->parent_result.result = PARENT_DIRECT;
       }
       break;
@@ -3549,9 +3548,9 @@ HttpTransact::handle_response_from_parent(State *s)
 
   // 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) {
+      !s->parent_result.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)) {
+    if (s->parent_result.retry_type() & (PARENT_ORIGIN_SIMPLE_RETRY | PARENT_ORIGIN_UNAVAILABLE_SERVER_RETRY)) {
       simple_or_unavailable_server_retry(s);
     }
   }
@@ -3591,7 +3590,7 @@ HttpTransact::handle_response_from_parent(State *s)
     // 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) {
+        if (s->current.simple_retry_attempts >= s->parent_result.max_retries(PARENT_RETRY_SIMPLE)) {
           DebugTxn("http_trans", "PARENT_ORIGIN_SIMPLE_RETRY: retried all parents, send error to client.\n");
           s->current.retry_type = PARENT_ORIGIN_UNDEFINED_RETRY;
         } else {
@@ -3601,7 +3600,7 @@ HttpTransact::handle_response_from_parent(State *s)
           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) {
+        if (s->current.unavailable_server_retry_attempts >= s->parent_result.max_retries(PARENT_RETRY_UNAVAILABLE_SERVER)) {
           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 {
@@ -3694,7 +3693,7 @@ HttpTransact::handle_response_from_server(State *s)
 {
   DebugTxn("http_trans", "[handle_response_from_server] (hrfs)");
   HTTP_RELEASE_ASSERT(s->current.server == &s->server_info);
-  int max_connect_retries = 0;
+  unsigned max_connect_retries = 0;
 
   // plugin call
   s->server_info.state = s->current.state;
@@ -3831,7 +3830,7 @@ HttpTransact::delete_server_rr_entry(State *s, int max_retries)
 //
 ///////////////////////////////////////////////////////////////////////////////
 void
-HttpTransact::retry_server_connection_not_open(State *s, ServerState_t conn_state, int max_retries)
+HttpTransact::retry_server_connection_not_open(State *s, ServerState_t conn_state, unsigned max_retries)
 {
   ink_assert(s->current.state != CONNECTION_ALIVE);
   ink_assert(s->current.state != ACTIVE_TIMEOUT);
@@ -7840,13 +7839,13 @@ HttpTransact::build_request(State *s, HTTPHdr *base_request, HTTPHdr *outgoing_r
   // If we're going to a parent proxy, make sure we pass host and port
   // in the URL even if we didn't get them (e.g. transparent proxy)
   if (s->current.request_to == PARENT_PROXY) {
-    if (!outgoing_request->is_target_in_url() && s->parent_result.rec->parent_is_proxy) {
+    if (!outgoing_request->is_target_in_url() && s->parent_result.parent_is_proxy()) {
       DebugTxn("http_trans", "[build_request] adding target to URL for parent proxy");
 
       // No worry about HTTP/0.9 because we reject forward proxy requests that
       // don't have a host anywhere.
       outgoing_request->set_url_target_from_host_field();
-    } else if (s->current.request_to == PARENT_PROXY && !s->parent_result.rec->parent_is_proxy &&
+    } else if (s->current.request_to == PARENT_PROXY && !s->parent_result.parent_is_proxy() &&
                outgoing_request->is_target_in_url()) {
       // If the parent is an origin server remove the hostname from the url.
       DebugTxn("http_trans", "[build_request] removing target from URL for a parent origin.");
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 8f77363..831046b 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -686,9 +686,9 @@ public:
     ConnectionAttributes *server;
     ink_time_t now;
     ServerState_t state;
-    int attempts;
-    int simple_retry_attempts;
-    int unavailable_server_retry_attempts;
+    unsigned attempts;
+    unsigned simple_retry_attempts;
+    unsigned unavailable_server_retry_attempts;
     ParentOriginRetry_t retry_type;
 
     _CurrentInfo()
@@ -1114,7 +1114,7 @@ public:
   static void handle_response_from_parent(State *s);
   static void handle_response_from_server(State *s);
   static void delete_server_rr_entry(State *s, int max_retries);
-  static void retry_server_connection_not_open(State *s, ServerState_t conn_state, int max_retries);
+  static void retry_server_connection_not_open(State *s, ServerState_t conn_state, unsigned max_retries);
   static void handle_server_connection_not_open(State *s);
   static void handle_forward_server_connection_open(State *s);
   static void handle_cache_operation_on_forward_server_response(State *s);

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

[trafficserver] 05/06: TS-4388: Refactor ParentResult initialization.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 2bf1a31cc95a2ffb1404d0a53e37190243c0aa1d
Author: James Peach <jp...@apache.org>
AuthorDate: Fri Apr 29 20:34:56 2016 -0700

    TS-4388: Refactor ParentResult initialization.
    
    There are a couple of placed where ParentResult objects are
    reinitialized. Refactor this into ParentResult::reset() so avoid
    duplicated code.
---
 proxy/ParentSelection.cc |  6 +-----
 proxy/ParentSelection.h  | 15 +++++++++------
 proxy/http/HttpSM.cc     |  2 +-
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/proxy/ParentSelection.cc b/proxy/ParentSelection.cc
index 280ac9c..bbedbaf 100644
--- a/proxy/ParentSelection.cc
+++ b/proxy/ParentSelection.cc
@@ -119,11 +119,7 @@ ParentConfigParams::findParent(HttpRequestData *rdata, ParentResult *result)
     return;
   }
   // Initialize the result structure
-  result->rec = NULL;
-  result->line_number = 0xffffffff;
-  result->wrap_around = false;
-  result->start_parent = 0;
-  result->last_parent = 0;
+  result->reset();
 
   // Check to see if the parent was set through the
   //   api
diff --git a/proxy/ParentSelection.h b/proxy/ParentSelection.h
index 6d8db3b..975b07a 100644
--- a/proxy/ParentSelection.h
+++ b/proxy/ParentSelection.h
@@ -158,18 +158,21 @@ public:
 ParentRecord *const extApiRecord = (ParentRecord *)0xeeeeffff;
 
 struct ParentResult {
-  ParentResult()
-    : result(PARENT_UNDEFINED), hostname(NULL), port(0), retry(false), line_number(0), rec(NULL), last_parent(0), start_parent(0),
-      wrap_around(false), last_lookup(0)
-  {
-  }
-
+  ParentResult() { reset(); }
   // For outside consumption
   ParentResultType result;
   const char *hostname;
   int port;
   bool retry;
 
+  void
+  reset()
+  {
+    ink_zero(*this);
+    line_number = -1;
+    result = PARENT_UNDEFINED;
+  }
+
   // Internal use only
   //   Not to be modified by HTTP
   int line_number;
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index be76c29..9221aa8 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -7647,7 +7647,7 @@ HttpSM::redirect_request(const char *redirect_url, const int redirect_len)
   // we want to close the server session
   // will do that in handle_api_return under the
   // HttpTransact::SM_ACTION_REDIRECT_READ state
-  t_state.parent_result.result = PARENT_UNDEFINED;
+  t_state.parent_result.reset();
   t_state.request_sent_time = 0;
   t_state.response_received_time = 0;
   t_state.cache_info.write_lock_state = HttpTransact::CACHE_WL_INIT;

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

[trafficserver] 04/06: TS-4388: Rename ParentResult:r.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 41e013db839c0ece543ddc854805fa59a731b0db
Author: James Peach <jp...@apache.org>
AuthorDate: Fri Apr 29 20:04:20 2016 -0700

    TS-4388: Rename ParentResult:r.
    
    A single-letter state variable name is hard to read and hard to
    search for. Rename this to result since it is the result of a parent
    proxy lookup.
---
 iocore/net/Socks.cc           |  6 +++---
 proxy/ParentConsistentHash.cc | 20 ++++++++++----------
 proxy/ParentRoundRobin.cc     | 22 +++++++++++-----------
 proxy/ParentSelection.cc      | 40 ++++++++++++++++++++--------------------
 proxy/ParentSelection.h       |  4 ++--
 proxy/http/HttpSM.cc          |  6 +++---
 proxy/http/HttpTransact.cc    | 28 ++++++++++++++--------------
 7 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/iocore/net/Socks.cc b/iocore/net/Socks.cc
index a4b0330..792c949 100644
--- a/iocore/net/Socks.cc
+++ b/iocore/net/Socks.cc
@@ -87,7 +87,7 @@ SocksEntry::findServer()
 
 #ifdef SOCKS_WITH_TS
   if (nattempts == 1) {
-    ink_assert(server_result.r == PARENT_UNDEFINED);
+    ink_assert(server_result.result == PARENT_UNDEFINED);
     server_params->findParent(&req_data, &server_result);
   } else {
     socks_conf_struct *conf = netProcessor.socks_conf_stuff;
@@ -97,12 +97,12 @@ SocksEntry::findServer()
     server_params->markParentDown(&server_result);
 
     if (nattempts > conf->connection_attempts)
-      server_result.r = PARENT_FAIL;
+      server_result.result = PARENT_FAIL;
     else
       server_params->nextParent(&req_data, &server_result);
   }
 
-  switch (server_result.r) {
+  switch (server_result.result) {
   case PARENT_SPECIFIED:
     // Original was inet_addr, but should hostnames work?
     // ats_ip_pton only supports numeric (because other clients
diff --git a/proxy/ParentConsistentHash.cc b/proxy/ParentConsistentHash.cc
index 6c19e9a..1b5e396 100644
--- a/proxy/ParentConsistentHash.cc
+++ b/proxy/ParentConsistentHash.cc
@@ -105,9 +105,9 @@ ParentConsistentHash::selectParent(const ParentSelectionPolicy *policy, bool fir
   // Should only get into this state if we are supposed to go direct.
   if (parents[PRIMARY] == NULL && parents[SECONDARY] == NULL) {
     if (result->rec->go_direct == true && result->rec->parent_is_proxy == true) {
-      result->r = PARENT_DIRECT;
+      result->result = PARENT_DIRECT;
     } else {
-      result->r = PARENT_FAIL;
+      result->result = PARENT_FAIL;
     }
     result->hostname = NULL;
     result->port = 0;
@@ -156,7 +156,7 @@ ParentConsistentHash::selectParent(const ParentSelectionPolicy *policy, bool fir
           result->last_parent = pRec->idx;
           result->last_lookup = last_lookup;
           result->retry = parentRetry;
-          result->r = PARENT_SPECIFIED;
+          result->result = PARENT_SPECIFIED;
           Debug("parent_select", "Down parent %s is now retryable, marked it available.", pRec->hostname);
           break;
         }
@@ -196,7 +196,7 @@ ParentConsistentHash::selectParent(const ParentSelectionPolicy *policy, bool fir
 
   // use the available or marked for retry parent.
   if (pRec && (pRec->available || result->retry)) {
-    result->r = PARENT_SPECIFIED;
+    result->result = PARENT_SPECIFIED;
     result->hostname = pRec->hostname;
     result->port = pRec->port;
     result->last_parent = pRec->idx;
@@ -207,9 +207,9 @@ ParentConsistentHash::selectParent(const ParentSelectionPolicy *policy, bool fir
     Debug("parent_select", "Chosen parent: %s.%d", result->hostname, result->port);
   } else {
     if (result->rec->go_direct == true && result->rec->parent_is_proxy == true) {
-      result->r = PARENT_DIRECT;
+      result->result = PARENT_DIRECT;
     } else {
-      result->r = PARENT_FAIL;
+      result->result = PARENT_FAIL;
     }
     result->hostname = NULL;
     result->port = 0;
@@ -230,8 +230,8 @@ ParentConsistentHash::markParentDown(const ParentSelectionPolicy *policy, Parent
 
   //  Make sure that we are being called back with with a
   //   result structure with a parent
-  ink_assert(result->r == PARENT_SPECIFIED);
-  if (result->r != PARENT_SPECIFIED) {
+  ink_assert(result->result == PARENT_SPECIFIED);
+  if (result->result != PARENT_SPECIFIED) {
     return;
   }
   // If we were set through the API we currently have not failover
@@ -306,8 +306,8 @@ ParentConsistentHash::markParentUp(ParentResult *result)
   //  Make sure that we are being called back with with a
   //   result structure with a parent that is being retried
   ink_release_assert(result->retry == true);
-  ink_assert(result->r == PARENT_SPECIFIED);
-  if (result->r != PARENT_SPECIFIED) {
+  ink_assert(result->result == PARENT_SPECIFIED);
+  if (result->result != PARENT_SPECIFIED) {
     return;
   }
   // If we were set through the API we currently have not failover
diff --git a/proxy/ParentRoundRobin.cc b/proxy/ParentRoundRobin.cc
index ac453d3..0716d78 100644
--- a/proxy/ParentRoundRobin.cc
+++ b/proxy/ParentRoundRobin.cc
@@ -69,9 +69,9 @@ ParentRoundRobin::selectParent(const ParentSelectionPolicy *policy, bool first_c
       ink_assert(result->rec->go_direct == true);
       // Could not find a parent
       if (result->rec->go_direct == true && result->rec->parent_is_proxy == true) {
-        result->r = PARENT_DIRECT;
+        result->result = PARENT_DIRECT;
       } else {
-        result->r = PARENT_FAIL;
+        result->result = PARENT_FAIL;
       }
 
       result->hostname = NULL;
@@ -112,9 +112,9 @@ ParentRoundRobin::selectParent(const ParentSelectionPolicy *policy, bool first_c
       if (bypass_ok == true) {
         // Could not find a parent
         if (result->rec->go_direct == true && result->rec->parent_is_proxy == true) {
-          result->r = PARENT_DIRECT;
+          result->result = PARENT_DIRECT;
         } else {
-          result->r = PARENT_FAIL;
+          result->result = PARENT_FAIL;
         }
         result->hostname = NULL;
         result->port = 0;
@@ -153,7 +153,7 @@ ParentRoundRobin::selectParent(const ParentSelectionPolicy *policy, bool first_c
     }
 
     if (parentUp == true) {
-      result->r = PARENT_SPECIFIED;
+      result->result = PARENT_SPECIFIED;
       result->hostname = result->rec->parents[cur_index].hostname;
       result->port = result->rec->parents[cur_index].port;
       result->last_parent = cur_index;
@@ -167,9 +167,9 @@ ParentRoundRobin::selectParent(const ParentSelectionPolicy *policy, bool first_c
   } while ((unsigned int)cur_index != result->start_parent);
 
   if (result->rec->go_direct == true && result->rec->parent_is_proxy == true) {
-    result->r = PARENT_DIRECT;
+    result->result = PARENT_DIRECT;
   } else {
-    result->r = PARENT_FAIL;
+    result->result = PARENT_FAIL;
   }
 
   result->hostname = NULL;
@@ -192,8 +192,8 @@ ParentRoundRobin::markParentDown(const ParentSelectionPolicy *policy, ParentResu
   Debug("parent_select", "Starting ParentRoundRobin::markParentDown()");
   //  Make sure that we are being called back with with a
   //   result structure with a parent
-  ink_assert(result->r == PARENT_SPECIFIED);
-  if (result->r != PARENT_SPECIFIED) {
+  ink_assert(result->result == PARENT_SPECIFIED);
+  if (result->result != PARENT_SPECIFIED) {
     return;
   }
   // If we were set through the API we currently have not failover
@@ -251,8 +251,8 @@ ParentRoundRobin::markParentUp(ParentResult *result)
   //  Make sure that we are being called back with with a
   //   result structure with a parent that is being retried
   ink_release_assert(result->retry == true);
-  ink_assert(result->r == PARENT_SPECIFIED);
-  if (result->r != PARENT_SPECIFIED) {
+  ink_assert(result->result == PARENT_SPECIFIED);
+  if (result->result != PARENT_SPECIFIED) {
     return;
   }
   // If we were set through the API we currently have not failover
diff --git a/proxy/ParentSelection.cc b/proxy/ParentSelection.cc
index abb6cb5..280ac9c 100644
--- a/proxy/ParentSelection.cc
+++ b/proxy/ParentSelection.cc
@@ -110,12 +110,12 @@ ParentConfigParams::findParent(HttpRequestData *rdata, ParentResult *result)
   ParentRecord *rec;
 
   Debug("parent_select", "In ParentConfigParams::findParent(): parent_table: %p.", parent_table);
-  ink_assert(result->r == PARENT_UNDEFINED);
+  ink_assert(result->result == PARENT_UNDEFINED);
 
   // Check to see if we are enabled
   Debug("parent_select", "policy.ParentEnable: %d", policy.ParentEnable);
   if (policy.ParentEnable == 0) {
-    result->r = PARENT_DIRECT;
+    result->result = PARENT_DIRECT;
     return;
   }
   // Initialize the result structure
@@ -128,7 +128,7 @@ ParentConfigParams::findParent(HttpRequestData *rdata, ParentResult *result)
   // Check to see if the parent was set through the
   //   api
   if (apiParentExists(rdata)) {
-    result->r = PARENT_SPECIFIED;
+    result->result = PARENT_SPECIFIED;
     result->hostname = rdata->api_info->parent_proxy_name;
     result->port = rdata->api_info->parent_proxy_port;
     result->rec = extApiRecord;
@@ -148,7 +148,7 @@ ParentConfigParams::findParent(HttpRequestData *rdata, ParentResult *result)
     if (defaultPtr != NULL) {
       rec = result->rec = defaultPtr;
     } else {
-      result->r = PARENT_DIRECT;
+      result->result = PARENT_DIRECT;
       Debug("parent_select", "Returning PARENT_DIRECT (no parents were found)");
       return;
     }
@@ -160,17 +160,17 @@ ParentConfigParams::findParent(HttpRequestData *rdata, ParentResult *result)
 
   const char *host = rdata->get_host();
 
-  switch (result->r) {
+  switch (result->result) {
   case PARENT_UNDEFINED:
     Debug("parent_select", "PARENT_UNDEFINED");
-    Debug("parent_select", "Result for %s was %s", host, ParentResultStr[result->r]);
+    Debug("parent_select", "Result for %s was %s", host, ParentResultStr[result->result]);
     break;
   case PARENT_FAIL:
     Debug("parent_select", "PARENT_FAIL");
     break;
   case PARENT_DIRECT:
     Debug("parent_select", "PARENT_DIRECT");
-    Debug("parent_select", "Result for %s was %s", host, ParentResultStr[result->r]);
+    Debug("parent_select", "Result for %s was %s", host, ParentResultStr[result->result]);
     break;
   case PARENT_SPECIFIED:
     Debug("parent_select", "PARENT_SPECIFIED");
@@ -192,19 +192,19 @@ ParentConfigParams::nextParent(HttpRequestData *rdata, ParentResult *result)
 
   //  Make sure that we are being called back with a
   //   result structure with a parent
-  ink_assert(result->r == PARENT_SPECIFIED);
-  if (result->r != PARENT_SPECIFIED) {
-    result->r = PARENT_FAIL;
+  ink_assert(result->result == PARENT_SPECIFIED);
+  if (result->result != PARENT_SPECIFIED) {
+    result->result = PARENT_FAIL;
     return;
   }
   // If we were set through the API we currently have not failover
   //   so just return fail
   if (result->rec == extApiRecord) {
-    Debug("parent_select", "Retry result for %s was %s", rdata->get_host(), ParentResultStr[result->r]);
-    result->r = PARENT_FAIL;
+    Debug("parent_select", "Retry result for %s was %s", rdata->get_host(), ParentResultStr[result->result]);
+    result->result = PARENT_FAIL;
     return;
   }
-  Debug("parent_select", "ParentConfigParams::nextParent(): result->r: %d, tablePtr: %p", result->r, tablePtr);
+  Debug("parent_select", "ParentConfigParams::nextParent(): result->r: %d, tablePtr: %p", result->result, tablePtr);
 
   // Find the next parent in the array
   Debug("parent_select", "Calling selectParent() from nextParent");
@@ -212,18 +212,18 @@ ParentConfigParams::nextParent(HttpRequestData *rdata, ParentResult *result)
 
   const char *host = rdata->get_host();
 
-  switch (result->r) {
+  switch (result->result) {
   case PARENT_UNDEFINED:
     Debug("parent_select", "PARENT_UNDEFINED");
-    Debug("parent_select", "Retry result for %s was %s", host, ParentResultStr[result->r]);
+    Debug("parent_select", "Retry result for %s was %s", host, ParentResultStr[result->result]);
     break;
   case PARENT_FAIL:
     Debug("parent_select", "PARENT_FAIL");
-    Debug("parent_select", "Retry result for %s was %s", host, ParentResultStr[result->r]);
+    Debug("parent_select", "Retry result for %s was %s", host, ParentResultStr[result->result]);
     break;
   case PARENT_DIRECT:
     Debug("parent_select", "PARENT_DIRECT");
-    Debug("parent_select", "Retry result for %s was %s", host, ParentResultStr[result->r]);
+    Debug("parent_select", "Retry result for %s was %s", host, ParentResultStr[result->result]);
     break;
   case PARENT_SPECIFIED:
     Debug("parent_select", "Retry result for %s was parent %s:%d", host, result->hostname, result->port);
@@ -242,7 +242,7 @@ ParentConfigParams::parentExists(HttpRequestData *rdata)
 
   findParent(rdata, &result);
 
-  if (result.r == PARENT_SPECIFIED) {
+  if (result.result == PARENT_SPECIFIED) {
     return true;
   } else {
     return false;
@@ -1308,7 +1308,7 @@ verify(ParentResult *r, ParentResultType e, const char *h, int p)
 {
   if (is_debug_tag_set("parent_select"))
     show_result(r);
-  return (r->r != e) ? 0 : ((e != PARENT_SPECIFIED) ? 1 : (strcmp(r->hostname, h) ? 0 : ((r->port == p) ? 1 : 0)));
+  return (r->result != e) ? 0 : ((e != PARENT_SPECIFIED) ? 1 : (strcmp(r->hostname, h) ? 0 : ((r->port == p) ? 1 : 0)));
 }
 
 // br creates an HttpRequestData object
@@ -1330,7 +1330,7 @@ br(HttpRequestData *h, const char *os_hostname, sockaddr const *dest_ip)
 void
 show_result(ParentResult *p)
 {
-  switch (p->r) {
+  switch (p->result) {
   case PARENT_UNDEFINED:
     printf("result is PARENT_UNDEFINED\n");
     break;
diff --git a/proxy/ParentSelection.h b/proxy/ParentSelection.h
index 79c8b5e..6d8db3b 100644
--- a/proxy/ParentSelection.h
+++ b/proxy/ParentSelection.h
@@ -159,13 +159,13 @@ ParentRecord *const extApiRecord = (ParentRecord *)0xeeeeffff;
 
 struct ParentResult {
   ParentResult()
-    : r(PARENT_UNDEFINED), hostname(NULL), port(0), retry(false), line_number(0), rec(NULL), last_parent(0), start_parent(0),
+    : result(PARENT_UNDEFINED), hostname(NULL), port(0), retry(false), line_number(0), rec(NULL), last_parent(0), start_parent(0),
       wrap_around(false), last_lookup(0)
   {
   }
 
   // For outside consumption
-  ParentResultType r;
+  ParentResultType result;
   const char *hostname;
   int port;
   bool retry;
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 9582c0f..be76c29 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -7107,7 +7107,7 @@ HttpSM::set_next_state()
       call_transact_and_set_next_state(HttpTransact::HandleFiltering);
       break;
     } else if (t_state.http_config_param->use_client_target_addr == 2 && !t_state.url_remap_success &&
-               t_state.parent_result.r != PARENT_SPECIFIED && t_state.client_info.is_transparent &&
+               t_state.parent_result.result != PARENT_SPECIFIED && t_state.client_info.is_transparent &&
                t_state.dns_info.os_addr_style == HttpTransact::DNSLookupInfo::OS_ADDR_TRY_DEFAULT &&
                ats_is_ip(addr = t_state.state_machine->ua_session->get_netvc()->get_local_addr())) {
       /* If the connection is client side transparent and the URL
@@ -7135,7 +7135,7 @@ HttpSM::set_next_state()
       t_state.dns_info.os_addr_style = HttpTransact::DNSLookupInfo::OS_ADDR_TRY_CLIENT;
       call_transact_and_set_next_state(NULL);
       break;
-    } else if (t_state.parent_result.r == PARENT_UNDEFINED && t_state.dns_info.lookup_success) {
+    } else if (t_state.parent_result.result == PARENT_UNDEFINED && t_state.dns_info.lookup_success) {
       // Already set, and we don't have a parent proxy to lookup
       ink_assert(ats_is_ip(t_state.host_db_info.ip()));
       DebugSM("dns", "[HttpTransact::HandleRequest] Skipping DNS lookup, provided by plugin");
@@ -7647,7 +7647,7 @@ HttpSM::redirect_request(const char *redirect_url, const int redirect_len)
   // we want to close the server session
   // will do that in handle_api_return under the
   // HttpTransact::SM_ACTION_REDIRECT_READ state
-  t_state.parent_result.r = PARENT_UNDEFINED;
+  t_state.parent_result.result = PARENT_UNDEFINED;
   t_state.request_sent_time = 0;
   t_state.response_received_time = 0;
   t_state.cache_info.write_lock_state = HttpTransact::CACHE_WL_INIT;
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 093f13f..5a333c3 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -251,12 +251,12 @@ find_server_and_update_current_info(HttpTransact::State *s)
     // Do not forward requests to local_host onto a parent.
     // I just wanted to do this for cop heartbeats, someone else
     // wanted it for all requests to local_host.
-    s->parent_result.r = PARENT_DIRECT;
+    s->parent_result.result = PARENT_DIRECT;
   } else if (s->method == HTTP_WKSIDX_CONNECT && s->http_config_param->disable_ssl_parenting) {
     s->parent_params->findParent(&s->request_data, &s->parent_result);
     if (s->parent_result.rec == NULL || s->parent_result.rec->parent_is_proxy) {
       DebugTxn("http_trans", "request not cacheable, so bypass parent");
-      s->parent_result.r = PARENT_DIRECT;
+      s->parent_result.result = PARENT_DIRECT;
     }
   } else if (s->http_config_param->uncacheable_requests_bypass_parent && s->http_config_param->no_dns_forward_to_parent == 0 &&
              !HttpTransact::is_request_cache_lookupable(s)) {
@@ -269,10 +269,10 @@ find_server_and_update_current_info(HttpTransact::State *s)
     s->parent_params->findParent(&s->request_data, &s->parent_result);
     if (s->parent_result.rec == NULL || s->parent_result.rec->parent_is_proxy) {
       DebugTxn("http_trans", "request not cacheable, so bypass parent");
-      s->parent_result.r = PARENT_DIRECT;
+      s->parent_result.result = PARENT_DIRECT;
     }
   } else {
-    switch (s->parent_result.r) {
+    switch (s->parent_result.result) {
     case PARENT_UNDEFINED:
       s->parent_params->findParent(&s->request_data, &s->parent_result);
       break;
@@ -283,9 +283,9 @@ find_server_and_update_current_info(HttpTransact::State *s)
       // We already have a parent that failed, if we are now told
       //  to go the origin server, we can only obey this if we
       //  dns'ed the origin server
-      if (s->parent_result.r == PARENT_DIRECT && s->http_config_param->no_dns_forward_to_parent != 0) {
+      if (s->parent_result.result == PARENT_DIRECT && s->http_config_param->no_dns_forward_to_parent != 0) {
         ink_assert(!s->server_info.dst_addr.isValid());
-        s->parent_result.r = PARENT_FAIL;
+        s->parent_result.result = PARENT_FAIL;
       }
       break;
     case PARENT_FAIL:
@@ -296,7 +296,7 @@ find_server_and_update_current_info(HttpTransact::State *s)
       //   3) the config permitted us to dns the origin server
       if (!s->parent_params->apiParentExists(&s->request_data) && s->parent_result.rec->bypass_ok() &&
           s->http_config_param->no_dns_forward_to_parent == 0 && s->parent_result.rec->parent_is_proxy) {
-        s->parent_result.r = PARENT_DIRECT;
+        s->parent_result.result = PARENT_DIRECT;
       }
       break;
     default:
@@ -310,7 +310,7 @@ find_server_and_update_current_info(HttpTransact::State *s)
     }
   }
 
-  switch (s->parent_result.r) {
+  switch (s->parent_result.result) {
   case PARENT_SPECIFIED:
     s->parent_info.name = s->arena.str_store(s->parent_result.hostname, strlen(s->parent_result.hostname));
     update_current_info(&s->current, &s->parent_info, HttpTransact::PARENT_PROXY, (s->current.attempts)++);
@@ -457,7 +457,7 @@ how_to_open_connection(HttpTransact::State *s)
     break;
   }
 
-  if (s->method == HTTP_WKSIDX_CONNECT && s->parent_result.r != PARENT_SPECIFIED) {
+  if (s->method == HTTP_WKSIDX_CONNECT && s->parent_result.result != PARENT_SPECIFIED) {
     s->cdn_saved_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_RAW_OPEN;
   } else {
     s->cdn_saved_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN;
@@ -2706,7 +2706,7 @@ HttpTransact::HandleCacheOpenReadHit(State *s)
       }
       // a parent lookup could come back as PARENT_FAIL if in parent.config, go_direct == false and
       // there are no available parents (all down).
-      else if (s->current.request_to == HOST_NONE && s->parent_result.r == PARENT_FAIL) {
+      else if (s->current.request_to == HOST_NONE && s->parent_result.result == PARENT_FAIL) {
         if (is_server_negative_cached(s) && response_returnable == true && is_stale_cache_response_returnable(s) == true) {
           server_up = false;
           update_current_info(&s->current, NULL, UNDEFINED_LOOKUP, 0);
@@ -3153,7 +3153,7 @@ HttpTransact::HandleCacheOpenReadMiss(State *s)
     find_server_and_update_current_info(s);
     // a parent lookup could come back as PARENT_FAIL if in parent.config go_direct == false and
     // there are no available parents (all down).
-    if (s->parent_result.r == PARENT_FAIL) {
+    if (s->parent_result.result == PARENT_FAIL) {
       handle_parent_died(s);
       return;
     }
@@ -3583,7 +3583,7 @@ HttpTransact::handle_response_from_parent(State *s)
     // If the request is not retryable, just give up!
     if (!is_request_retryable(s)) {
       s->parent_params->markParentDown(&s->parent_result);
-      s->parent_result.r = PARENT_FAIL;
+      s->parent_result.result = PARENT_FAIL;
       handle_parent_died(s);
       return;
     }
@@ -3639,7 +3639,7 @@ HttpTransact::handle_response_from_parent(State *s)
       //   appropriate
       DebugTxn("http_trans", "[handle_response_from_parent] Error. No more retries.");
       s->parent_params->markParentDown(&s->parent_result);
-      s->parent_result.r = PARENT_FAIL;
+      s->parent_result.result = PARENT_FAIL;
       next_lookup = find_server_and_update_current_info(s);
     }
 
@@ -7601,7 +7601,7 @@ HttpTransact::AuthenticationNeeded(const OverridableHttpConfigParams *p, HTTPHdr
 void
 HttpTransact::handle_parent_died(State *s)
 {
-  ink_assert(s->parent_result.r == PARENT_FAIL);
+  ink_assert(s->parent_result.result == PARENT_FAIL);
 
   build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Next Hop Connection Failed", "connect#failed_connect", NULL);
   TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, NULL);

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

[trafficserver] 01/06: TS-4388: Remove useless record API defines.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit ab4b69e08ff20ac4df6da758c1a9380af54971e7
Author: James Peach <jp...@apache.org>
AuthorDate: Wed Apr 27 11:12:11 2016 -0700

    TS-4388: Remove useless record API defines.
---
 proxy/ParentSelection.cc | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/proxy/ParentSelection.cc b/proxy/ParentSelection.cc
index a439a27..abb6cb5 100644
--- a/proxy/ParentSelection.cc
+++ b/proxy/ParentSelection.cc
@@ -31,10 +31,6 @@
 #include "HTTP.h"
 #include "HttpTransact.h"
 
-#define PARENT_RegisterConfigUpdateFunc REC_RegisterConfigUpdateFunc
-#define PARENT_ReadConfigInteger REC_ReadConfigInteger
-#define PARENT_ReadConfigStringAlloc REC_ReadConfigStringAlloc
-
 #define MAX_SIMPLE_RETRIES 5
 #define MAX_UNAVAILABLE_SERVER_RETRIES 5
 
@@ -74,19 +70,19 @@ ParentSelectionPolicy::ParentSelectionPolicy()
   int32_t dns_parent_only = 0;
 
   // Handle parent timeout
-  PARENT_ReadConfigInteger(retry_time, retry_var);
+  REC_ReadConfigInteger(retry_time, retry_var);
   ParentRetryTime = retry_time;
 
   // Handle parent enable
-  PARENT_ReadConfigInteger(enable, enable_var);
+  REC_ReadConfigInteger(enable, enable_var);
   ParentEnable = enable;
 
   // Handle the fail threshold
-  PARENT_ReadConfigInteger(fail_threshold, threshold_var);
+  REC_ReadConfigInteger(fail_threshold, threshold_var);
   FailThreshold = fail_threshold;
 
   // Handle dns parent only
-  PARENT_ReadConfigInteger(dns_parent_only, dns_parent_only_var);
+  REC_ReadConfigInteger(dns_parent_only, dns_parent_only_var);
   DNS_ParentOnly = dns_parent_only;
 }
 
@@ -95,7 +91,7 @@ ParentConfigParams::ParentConfigParams(P_table *_parent_table) : parent_table(_p
   char *default_val = NULL;
 
   // Handle default parent
-  PARENT_ReadConfigStringAlloc(default_val, default_var);
+  REC_ReadConfigStringAlloc(default_val, default_var);
   DefaultParent = createDefaultParent(default_val);
   ats_free(default_val);
 }
@@ -829,7 +825,7 @@ SocksServerConfig::reconfigure()
   ink_assert(params != NULL);
 
   // Handle default parent
-  PARENT_ReadConfigStringAlloc(default_val, "proxy.config.socks.default_servers");
+  REC_ReadConfigStringAlloc(default_val, "proxy.config.socks.default_servers");
   params->DefaultParent = createDefaultParent(default_val);
   ats_free(default_val);
 
@@ -839,7 +835,7 @@ SocksServerConfig::reconfigure()
     setup_socks_servers(params->parent_table->ipMatch->data_array, params->parent_table->ipMatch->array_len);
 
   // Handle parent timeout
-  PARENT_ReadConfigInteger(retry_time, "proxy.config.socks.server_retry_time");
+  REC_ReadConfigInteger(retry_time, "proxy.config.socks.server_retry_time");
   params->policy.ParentRetryTime = retry_time;
 
   // Handle parent enable
@@ -847,7 +843,7 @@ SocksServerConfig::reconfigure()
   params->policy.ParentEnable = 1;
 
   // Handle the fail threshold
-  PARENT_ReadConfigInteger(fail_threshold, "proxy.config.socks.server_fail_threshold");
+  REC_ReadConfigInteger(fail_threshold, "proxy.config.socks.server_fail_threshold");
   params->policy.FailThreshold = fail_threshold;
 
   // Handle dns parent only

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

[trafficserver] 03/06: TS-4388: Fix global hook handling in API tests.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 060b8e8bd0d794ee038595e42e3e104d6c89371a
Author: James Peach <jp...@apache.org>
AuthorDate: Thu Apr 28 20:02:38 2016 +0000

    TS-4388: Fix global hook handling in API tests.
    
    Many of the API regression tests work by trampolining off a global
    hook (which is really what you have to do). However, there's no way
    to unregister a global hook, so once the test is done, it needs to
    be careful to co-operate with the remaining tests. We clear the
    continuation data, and if is is clear, we either ignore the event
    or re-enable the HTTP transaction.
---
 proxy/InkAPITest.cc     | 110 ++++++++++++++++++++++++++----------------------
 proxy/InkAPITestTool.cc |  38 ++++++++++++-----
 2 files changed, 88 insertions(+), 60 deletions(-)

diff --git a/proxy/InkAPITest.cc b/proxy/InkAPITest.cc
index cb883ec..342e824 100644
--- a/proxy/InkAPITest.cc
+++ b/proxy/InkAPITest.cc
@@ -55,6 +55,39 @@
 
 #define UTDBG_TAG "sdk_ut"
 
+// Since there's no way to unregister global hooks, tests that register a hook
+// have to co-operate once they are complete by re-enabling and transactions
+// and getting out of the way.
+#define CHECK_SPURIOUS_EVENT(cont, event, edata)                     \
+  if (TSContDataGet(cont) == NULL) {                                 \
+    switch (event) {                                                 \
+    case TS_EVENT_IMMEDIATE:                                         \
+    case TS_EVENT_TIMEOUT:                                           \
+      return TS_EVENT_NONE;                                          \
+    case TS_EVENT_HTTP_SELECT_ALT:                                   \
+      return TS_EVENT_NONE;                                          \
+    case TS_EVENT_HTTP_READ_REQUEST_HDR:                             \
+    case TS_EVENT_HTTP_OS_DNS:                                       \
+    case TS_EVENT_HTTP_SEND_REQUEST_HDR:                             \
+    case TS_EVENT_HTTP_READ_CACHE_HDR:                               \
+    case TS_EVENT_HTTP_READ_RESPONSE_HDR:                            \
+    case TS_EVENT_HTTP_SEND_RESPONSE_HDR:                            \
+    case TS_EVENT_HTTP_REQUEST_TRANSFORM:                            \
+    case TS_EVENT_HTTP_RESPONSE_TRANSFORM:                           \
+    case TS_EVENT_HTTP_TXN_START:                                    \
+    case TS_EVENT_HTTP_TXN_CLOSE:                                    \
+    case TS_EVENT_HTTP_SSN_START:                                    \
+    case TS_EVENT_HTTP_SSN_CLOSE:                                    \
+    case TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE:                        \
+    case TS_EVENT_HTTP_PRE_REMAP:                                    \
+    case TS_EVENT_HTTP_POST_REMAP:                                   \
+      TSHttpTxnReenable((TSHttpTxn)(edata), TS_EVENT_HTTP_CONTINUE); \
+      return TS_EVENT_NONE;                                          \
+    default:                                                         \
+      break;                                                         \
+    }                                                                \
+  }
+
 /******************************************************************************/
 
 /* Use SDK_RPRINT to report failure or success for each test case */
@@ -2303,6 +2336,7 @@ mytest_handler(TSCont contp, TSEvent event, void *data)
       // transaction is over. clean up.
       synclient_txn_delete(test->browser);
       synserver_delete(test->os);
+      test->os = NULL;
 
       test->magic = MAGIC_DEAD;
       TSfree(test);
@@ -5804,6 +5838,7 @@ ssn_handler(TSCont contp, TSEvent event, void *edata)
       /* Don't need it as didn't initialize the server
          synserver_delete(data->os);
        */
+      data->os = NULL;
       data->magic = MAGIC_DEAD;
       TSfree(data);
       TSContDataSet(contp, NULL);
@@ -5871,9 +5906,12 @@ struct ParentTest {
 static int
 parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
 {
-  ParentTest *ptest = (ParentTest *)TSContDataGet(contp);
+  ParentTest *ptest = NULL;
   TSHttpTxn txnp = (TSHttpTxn)edata;
 
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
+  ptest = (ParentTest *)TSContDataGet(contp);
+
   switch (event) {
   case TS_EVENT_HTTP_READ_REQUEST_HDR:
     rprintf(ptest->regtest, "setting synserver parent proxy to %s:%d\n", "127.0.0.1", SYNSERVER_LISTEN_PORT);
@@ -5914,11 +5952,13 @@ parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
       // Otherwise the test completed so clean up.
       RecSetRecordInt("proxy.config.http.parent_proxy_routing_enable", ptest->parent_proxy_routing_enable, REC_SOURCE_EXPLICIT);
 
-      ptest->magic = MAGIC_DEAD;
+      TSContDataSet(contp, NULL);
+
       synclient_txn_delete(ptest->browser);
       synserver_delete(ptest->os);
+      ptest->os = NULL;
+      ptest->magic = MAGIC_DEAD;
       TSfree(ptest);
-      TSContDataSet(contp, NULL);
     }
     break;
 
@@ -6017,20 +6057,10 @@ static int
 cache_hook_handler(TSCont contp, TSEvent event, void *edata)
 {
   TSHttpTxn txnp = NULL;
-  CacheTestData *data = (CacheTestData *)TSContDataGet(contp);
+  CacheTestData *data = NULL;
 
-  if (data == NULL) {
-    switch (event) {
-    case TS_EVENT_IMMEDIATE:
-    case TS_EVENT_TIMEOUT:
-      break;
-    case TS_EVENT_HTTP_READ_CACHE_HDR:
-    default:
-      TSHttpTxnReenable((TSHttpTxn)edata, TS_EVENT_HTTP_CONTINUE);
-      break;
-    }
-    return 0;
-  }
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
+  data = (CacheTestData *)TSContDataGet(contp);
 
   switch (event) {
   case TS_EVENT_HTTP_READ_REQUEST_HDR:
@@ -6139,6 +6169,7 @@ cache_hook_handler(TSCont contp, TSEvent event, void *edata)
         data->first_time = false;
         /* Kill the origin server */
         synserver_delete(data->os);
+        data->os = NULL;
 
         /* Send another similar client request */
         synclient_txn_send_request(data->browser2, data->request);
@@ -6523,19 +6554,9 @@ transform_hook_handler(TSCont contp, TSEvent event, void *edata)
 {
   TSHttpTxn txnp = NULL;
   TransformTestData *data = NULL;
+
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
   data = (TransformTestData *)TSContDataGet(contp);
-  if (data == NULL) {
-    switch (event) {
-    case TS_EVENT_IMMEDIATE:
-    case TS_EVENT_TIMEOUT:
-      break;
-    case TS_EVENT_HTTP_READ_RESPONSE_HDR:
-    default:
-      TSHttpTxnReenable((TSHttpTxn)edata, TS_EVENT_HTTP_CONTINUE);
-      break;
-    }
-    return 0;
-  }
 
   switch (event) {
   case TS_EVENT_HTTP_READ_REQUEST_HDR:
@@ -6651,6 +6672,7 @@ transform_hook_handler(TSCont contp, TSEvent event, void *edata)
         return 0;
       }
       synserver_delete(data->os);
+      data->os = NULL;
       data->req_no++;
       TSfree(data->request1);
       TSfree(data->request2);
@@ -6823,20 +6845,8 @@ altinfo_hook_handler(TSCont contp, TSEvent event, void *edata)
   AltInfoTestData *data = NULL;
   TSHttpTxn txnp = NULL;
 
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
   data = (AltInfoTestData *)TSContDataGet(contp);
-  if (data == NULL) {
-    switch (event) {
-    case TS_EVENT_IMMEDIATE:
-    case TS_EVENT_TIMEOUT:
-      break;
-    case TS_EVENT_HTTP_SELECT_ALT:
-      break;
-    default:
-      TSHttpTxnReenable((TSHttpTxn)edata, TS_EVENT_HTTP_CONTINUE);
-      break;
-    }
-    return 0;
-  }
 
   switch (event) {
   case TS_EVENT_HTTP_READ_REQUEST_HDR:
@@ -6928,6 +6938,7 @@ altinfo_hook_handler(TSCont contp, TSEvent event, void *edata)
         data->first_time = false;
         /* Kill the origin server */
         synserver_delete(data->os);
+        data->os = NULL;
         // ink_release_assert(0);
         /* Send another similar client request */
         synclient_txn_send_request(data->browser3, data->request3);
@@ -7044,8 +7055,6 @@ EXCLUSIVE_REGRESSION_TEST(SDK_API_HttpAltInfo)(RegressionTest *test, int /* atyp
 #define TEST_CASE_CONNECT_ID1 9  // TSHttpTxnIntercept
 #define TEST_CASE_CONNECT_ID2 10 // TSHttpTxnServerIntercept
 
-#define SYNSERVER_DUMMY_PORT -1
-
 typedef struct {
   RegressionTest *test;
   int *pstatus;
@@ -7061,9 +7070,12 @@ static int
 cont_test_handler(TSCont contp, TSEvent event, void *edata)
 {
   TSHttpTxn txnp = (TSHttpTxn)edata;
-  ConnectTestData *data = (ConnectTestData *)TSContDataGet(contp);
+  ConnectTestData *data = NULL;
   int request_id = -1;
 
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
+  data = (ConnectTestData *)TSContDataGet(contp);
+
   TSReleaseAssert(data->magic == MAGIC_ALIVE);
   TSReleaseAssert((data->test_case == TEST_CASE_CONNECT_ID1) || (data->test_case == TEST_CASE_CONNECT_ID2));
 
@@ -7139,12 +7151,10 @@ cont_test_handler(TSCont contp, TSEvent event, void *edata)
       // transaction is over. clean it up.
       synclient_txn_delete(data->browser);
       synserver_delete(data->os);
-
-      // As we registered to a global hook, we may be called back again.
-      // Do not destroy the continuation...
-      // data->magic = MAGIC_DEAD;
-      // TSfree(data);
-      // TSContDataSet(contp, NULL);
+      data->os = NULL;
+      data->magic = MAGIC_DEAD;
+      TSfree(data);
+      TSContDataSet(contp, NULL);
     }
     break;
 
diff --git a/proxy/InkAPITestTool.cc b/proxy/InkAPITestTool.cc
index eb399fd..1ebac13 100644
--- a/proxy/InkAPITestTool.cc
+++ b/proxy/InkAPITestTool.cc
@@ -42,6 +42,7 @@
 #define MAGIC_DEAD 0xdeadbeef
 
 #define SYNSERVER_LISTEN_PORT 3300
+#define SYNSERVER_DUMMY_PORT -1
 
 #define PROXY_CONFIG_NAME_HTTP_PORT "proxy.config.http.server_port"
 #define PROXY_HTTP_DEFAULT_PORT 8080
@@ -762,6 +763,11 @@ synclient_txn_main_handler(TSCont contp, TSEvent event, void *data)
 SocketServer *
 synserver_create(int port)
 {
+  if (port != SYNSERVER_DUMMY_PORT) {
+    TSAssert(port > 0);
+    TSAssert(port < INT16_MAX);
+  }
+
   SocketServer *s = (SocketServer *)TSmalloc(sizeof(SocketServer));
   s->magic = MAGIC_ALIVE;
   s->accept_port = port;
@@ -775,7 +781,15 @@ static int
 synserver_start(SocketServer *s)
 {
   TSAssert(s->magic == MAGIC_ALIVE);
-  s->accept_action = TSNetAccept(s->accept_cont, s->accept_port, -1, 0);
+  TSAssert(s->accept_action == NULL);
+
+  if (s->accept_port != SYNSERVER_DUMMY_PORT) {
+    TSAssert(s->accept_port > 0);
+    TSAssert(s->accept_port < INT16_MAX);
+
+    s->accept_action = TSNetAccept(s->accept_cont, s->accept_port, AF_INET, 0);
+  }
+
   return 1;
 }
 
@@ -795,17 +809,21 @@ synserver_stop(SocketServer *s)
 static int
 synserver_delete(SocketServer *s)
 {
-  TSAssert(s->magic == MAGIC_ALIVE);
-  synserver_stop(s);
+  if (s != NULL) {
+    TSAssert(s->magic == MAGIC_ALIVE);
+    synserver_stop(s);
+
+    if (s->accept_cont) {
+      TSContDestroy(s->accept_cont);
+      s->accept_cont = NULL;
+      TSDebug(SDBG_TAG, "destroyed accept cont");
+    }
 
-  if (s->accept_cont) {
-    TSContDestroy(s->accept_cont);
-    s->accept_cont = NULL;
-    TSDebug(SDBG_TAG, "destroyed accept cont");
+    s->magic = MAGIC_DEAD;
+    TSfree(s);
+    TSDebug(SDBG_TAG, "deleted server");
   }
-  s->magic = MAGIC_DEAD;
-  TSfree(s);
-  TSDebug(SDBG_TAG, "deleted server");
+
   return 1;
 }
 

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