You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2020/01/06 18:15:07 UTC

[trafficserver] branch 9.0.x updated (ddb5956 -> 0e65c9e)

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

zwoop pushed a change to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.


    from ddb5956  Cleaned up indentation etc.
     new fc09e20  Add invalid config warning when cache open write fail and read retry are inconsistent
     new 0e65c9e  Set wrap after checking all the parents

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


Summary of changes:
 proxy/ParentConsistentHash.cc | 15 ++++++++-------
 proxy/ParentSelection.h       |  7 +++++--
 proxy/http/HttpCacheSM.cc     | 10 ++++++++--
 proxy/http/HttpConfig.cc      |  8 ++++++++
 proxy/http/HttpConfig.h       | 10 ++++++++++
 proxy/http/HttpSM.cc          |  8 ++++----
 proxy/http/HttpTransact.cc    |  2 +-
 proxy/http/HttpTransact.h     | 10 ----------
 8 files changed, 44 insertions(+), 26 deletions(-)


[trafficserver] 02/02: Set wrap after checking all the parents

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

zwoop pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 0e65c9e41a9c3d83cde496e2affe1f86a6d84a6d
Author: Vijay Mamidi <vi...@yahoo.com>
AuthorDate: Sat Dec 21 18:46:24 2019 -0800

    Set wrap after checking all the parents
    
    (cherry picked from commit 0716ab758c6647c6f7c3e5d88474e615772750ee)
---
 proxy/ParentConsistentHash.cc | 15 ++++++++-------
 proxy/ParentSelection.h       |  7 +++++--
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/proxy/ParentConsistentHash.cc b/proxy/ParentConsistentHash.cc
index 1d374e5..f7977d7 100644
--- a/proxy/ParentConsistentHash.cc
+++ b/proxy/ParentConsistentHash.cc
@@ -108,7 +108,7 @@ ParentConsistentHash::getPathHash(HttpRequestData *hrdata, ATSHash64 *h)
 // Helper function to abstract calling ATSConsistentHash lookup_by_hashval() vs lookup().
 static pRecord *
 chash_lookup(ATSConsistentHash *fhash, uint64_t path_hash, ATSConsistentHashIter *chashIter, bool *wrap_around,
-             ATSHash64Sip24 *hash, bool *chash_init)
+             ATSHash64Sip24 *hash, bool *chash_init, bool *mapWrapped)
 {
   pRecord *prtmp;
 
@@ -118,6 +118,11 @@ chash_lookup(ATSConsistentHash *fhash, uint64_t path_hash, ATSConsistentHashIter
   } else {
     prtmp = (pRecord *)fhash->lookup(nullptr, chashIter, wrap_around, hash);
   }
+  // Do not set wrap_around to true until we try all the parents atleast once.
+  bool wrapped = *wrap_around;
+  *wrap_around = (*mapWrapped && *wrap_around) ? true : false;
+  if (!*mapWrapped && wrapped)
+    *mapWrapped = true;
   return prtmp;
 }
 
@@ -188,7 +193,7 @@ ParentConsistentHash::selectParent(bool first_call, ParentResult *result, Reques
   fhash     = chash[last_lookup];
   do { // search until we've selected a different parent if !firstCall
     prtmp = chash_lookup(fhash, path_hash, &result->chashIter[last_lookup], &wrap_around[last_lookup], &hash,
-                         &result->chash_init[last_lookup]);
+                         &result->chash_init[last_lookup], &result->mapWrapped[last_lookup]);
     lookups++;
     if (prtmp) {
       pRec = (parents[last_lookup] + prtmp->idx);
@@ -219,10 +224,6 @@ ParentConsistentHash::selectParent(bool first_call, ParentResult *result, Reques
     }
   }
   if (!pRec || (pRec && !pRec->available) || host_stat == HOST_STATUS_DOWN) {
-    if (firstCall) {
-      result->chash_init[PRIMARY]   = false;
-      result->chash_init[SECONDARY] = false;
-    }
     do {
       // check if the host is retryable.  It's retryable if the retry window has elapsed
       // and the global host status is HOST_STATUS_UP
@@ -276,7 +277,7 @@ ParentConsistentHash::selectParent(bool first_call, ParentResult *result, Reques
         }
         fhash = chash[last_lookup];
         prtmp = chash_lookup(fhash, path_hash, &result->chashIter[last_lookup], &wrap_around[last_lookup], &hash,
-                             &result->chash_init[last_lookup]);
+                             &result->chash_init[last_lookup], &result->mapWrapped[last_lookup]);
         lookups++;
         if (prtmp) {
           pRec = (parents[last_lookup] + prtmp->idx);
diff --git a/proxy/ParentSelection.h b/proxy/ParentSelection.h
index 759a23e..4ac9ae5 100644
--- a/proxy/ParentSelection.h
+++ b/proxy/ParentSelection.h
@@ -169,8 +169,10 @@ struct ParentResult {
   reset()
   {
     ink_zero(*this);
-    line_number = -1;
-    result      = PARENT_UNDEFINED;
+    line_number   = -1;
+    result        = PARENT_UNDEFINED;
+    mapWrapped[0] = false;
+    mapWrapped[1] = false;
   }
 
   bool
@@ -256,6 +258,7 @@ private:
   uint32_t last_parent;
   uint32_t start_parent;
   bool wrap_around;
+  bool mapWrapped[2];
   // state for consistent hash.
   int last_lookup;
   ATSConsistentHashIter chashIter[2];


[trafficserver] 01/02: Add invalid config warning when cache open write fail and read retry are inconsistent

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

zwoop pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit fc09e2079979370e38b08a7ecf7e5e19ef386b71
Author: Sudheer Vinukonda <su...@apache.org>
AuthorDate: Mon Dec 23 19:05:40 2019 -0800

    Add invalid config warning when cache open write fail and read retry
    are inconsistent
    
    (cherry picked from commit 757ac78146dfa4aca18a4508ced2b9544d070f64)
---
 proxy/http/HttpCacheSM.cc  | 10 ++++++++--
 proxy/http/HttpConfig.cc   |  8 ++++++++
 proxy/http/HttpConfig.h    | 10 ++++++++++
 proxy/http/HttpSM.cc       |  8 ++++----
 proxy/http/HttpTransact.cc |  2 +-
 proxy/http/HttpTransact.h  | 10 ----------
 6 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/proxy/http/HttpCacheSM.cc b/proxy/http/HttpCacheSM.cc
index 0832dbc..25b9e21 100644
--- a/proxy/http/HttpCacheSM.cc
+++ b/proxy/http/HttpCacheSM.cc
@@ -172,7 +172,7 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
     break;
 
   case CACHE_EVENT_OPEN_WRITE_FAILED:
-    if (master_sm->t_state.txn_conf->cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_READ_RETRY) {
+    if (master_sm->t_state.txn_conf->cache_open_write_fail_action == CACHE_WL_FAIL_ACTION_READ_RETRY) {
       // fall back to open_read_tries
       // Note that when CACHE_WL_FAIL_ACTION_READ_RETRY is configured, max_cache_open_write_retries
       // is automatically ignored. Make sure to not disable max_cache_open_read_retries
@@ -181,6 +181,12 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
       if (open_write_tries <= master_sm->t_state.txn_conf->max_cache_open_write_retries) {
         Debug("http_cache", "[%" PRId64 "] [state_cache_open_write] cache open write failure %d. read retry triggered",
               master_sm->sm_id, open_write_tries);
+        if (master_sm->t_state.txn_conf->max_cache_open_read_retries <= 0) {
+          Debug("http_cache",
+                "[%" PRId64 "] [state_cache_open_write] invalid config, cache write fail set to"
+                " read retry, but, max_cache_open_read_retries is not enabled",
+                master_sm->sm_id);
+        }
         open_read_tries          = 0;
         read_retry_on_write_fail = true;
         // make sure it doesn't loop indefinitely
@@ -206,7 +212,7 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
     break;
 
   case EVENT_INTERVAL:
-    if (master_sm->t_state.txn_conf->cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_READ_RETRY) {
+    if (master_sm->t_state.txn_conf->cache_open_write_fail_action == CACHE_WL_FAIL_ACTION_READ_RETRY) {
       Debug("http_cache",
             "[%" PRId64 "] [state_cache_open_write] cache open write failure %d. "
             "falling back to read retry...",
diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc
index 1aa62b3..88c0f7c 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -1432,6 +1432,14 @@ HttpConfig::reconfigure()
   params->keepalive_internal_vc      = INT_TO_BOOL(m_master.keepalive_internal_vc);
 
   params->oride.cache_open_write_fail_action = m_master.oride.cache_open_write_fail_action;
+  if (params->oride.cache_open_write_fail_action == CACHE_WL_FAIL_ACTION_READ_RETRY) {
+    if (params->oride.max_cache_open_read_retries <= 0 || params->oride.max_cache_open_write_retries <= 0) {
+      Warning("Invalid config, cache_open_write_fail_action (%d), max_cache_open_read_retries (%" PRIu64 "), "
+              "max_cache_open_write_retries (%" PRIu64 ")",
+              params->oride.cache_open_write_fail_action, params->oride.max_cache_open_read_retries,
+              params->oride.max_cache_open_write_retries);
+    }
+  }
 
   params->oride.cache_when_to_revalidate = m_master.oride.cache_when_to_revalidate;
   params->max_post_size                  = m_master.max_post_size;
diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h
index c030fa5..c3cb7b0 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -330,6 +330,16 @@ enum {
   http_stat_count
 };
 
+enum CacheOpenWriteFailAction_t {
+  CACHE_WL_FAIL_ACTION_DEFAULT                           = 0x00,
+  CACHE_WL_FAIL_ACTION_ERROR_ON_MISS                     = 0x01,
+  CACHE_WL_FAIL_ACTION_STALE_ON_REVALIDATE               = 0x02,
+  CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_STALE_ON_REVALIDATE = 0x03,
+  CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_OR_REVALIDATE       = 0x04,
+  CACHE_WL_FAIL_ACTION_READ_RETRY                        = 0x05,
+  TOTAL_CACHE_WL_FAIL_ACTION_TYPES
+};
+
 extern RecRawStatBlock *http_rsb;
 
 /* Stats should only be accessed using these macros */
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 7e3f25b..7f36d63 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -2393,17 +2393,17 @@ HttpSM::state_cache_open_write(int event, void *data)
     //  for reading
     if (t_state.redirect_info.redirect_in_process) {
       SMDebug("http_redirect", "[%" PRId64 "] CACHE_EVENT_OPEN_WRITE_FAILED during redirect follow", sm_id);
-      t_state.cache_open_write_fail_action = HttpTransact::CACHE_WL_FAIL_ACTION_DEFAULT;
+      t_state.cache_open_write_fail_action = CACHE_WL_FAIL_ACTION_DEFAULT;
       t_state.cache_info.write_lock_state  = HttpTransact::CACHE_WL_FAIL;
       break;
     }
-    if (t_state.txn_conf->cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_DEFAULT) {
+    if (t_state.txn_conf->cache_open_write_fail_action == CACHE_WL_FAIL_ACTION_DEFAULT) {
       t_state.cache_info.write_lock_state = HttpTransact::CACHE_WL_FAIL;
       break;
     } else {
       t_state.cache_open_write_fail_action = t_state.txn_conf->cache_open_write_fail_action;
       if (!t_state.cache_info.object_read ||
-          (t_state.cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_OR_REVALIDATE)) {
+          (t_state.cache_open_write_fail_action == CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_OR_REVALIDATE)) {
         // cache miss, set wl_state to fail
         SMDebug("http", "[%" PRId64 "] cache object read %p, cache_wl_fail_action %d", sm_id, t_state.cache_info.object_read,
                 t_state.cache_open_write_fail_action);
@@ -2418,7 +2418,7 @@ HttpSM::state_cache_open_write(int event, void *data)
       t_state.cache_open_write_fail_action = t_state.txn_conf->cache_open_write_fail_action;
       // Note that CACHE_LOOKUP_COMPLETE may be invoked more than once
       // if CACHE_WL_FAIL_ACTION_READ_RETRY is configured
-      ink_assert(t_state.cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_READ_RETRY);
+      ink_assert(t_state.cache_open_write_fail_action == CACHE_WL_FAIL_ACTION_READ_RETRY);
       t_state.cache_lookup_result         = HttpTransact::CACHE_LOOKUP_NONE;
       t_state.cache_info.write_lock_state = HttpTransact::CACHE_WL_READ_RETRY;
       break;
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 76b7aa8..c83c105 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -2906,7 +2906,7 @@ HttpTransact::handle_cache_write_lock(State *s)
       //  Write failed and read retry triggered
       //  Clean up server_request and re-initiate
       //  Cache Lookup
-      ink_assert(s->cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_READ_RETRY);
+      ink_assert(s->cache_open_write_fail_action == CACHE_WL_FAIL_ACTION_READ_RETRY);
       s->cache_info.write_status = CACHE_WRITE_LOCK_MISS;
       StateMachineAction_t next;
       next           = SM_ACTION_CACHE_LOOKUP;
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 7ee4d35..09ddc29 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -245,16 +245,6 @@ public:
     TOTAL_CACHE_ACTION_TYPES
   };
 
-  enum CacheOpenWriteFailAction_t {
-    CACHE_WL_FAIL_ACTION_DEFAULT                           = 0x00,
-    CACHE_WL_FAIL_ACTION_ERROR_ON_MISS                     = 0x01,
-    CACHE_WL_FAIL_ACTION_STALE_ON_REVALIDATE               = 0x02,
-    CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_STALE_ON_REVALIDATE = 0x03,
-    CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_OR_REVALIDATE       = 0x04,
-    CACHE_WL_FAIL_ACTION_READ_RETRY                        = 0x05,
-    TOTAL_CACHE_WL_FAIL_ACTION_TYPES
-  };
-
   enum CacheWriteLock_t {
     CACHE_WL_INIT,
     CACHE_WL_SUCCESS,