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 2022/02/17 00:16:12 UTC

[trafficserver] 01/03: Ports #7897 from core strategies to parent_select plugin. (#8580)

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

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

commit b5f1ef4f53bcb90c8740cd5e3ace5420894cd127
Author: Robert O Butts <ro...@users.noreply.github.com>
AuthorDate: Fri Jan 28 09:57:34 2022 -0700

    Ports #7897 from core strategies to parent_select plugin. (#8580)
    
    Ports apache#7897 from core strategies to parent_select plugin.
    
    Adds a new peering_ring mode to strategies, which allows parenting to
    a consistent-hashed peer instead of the next hierarchical tier,
    which can be useful in many situations, especially when the requesting
    client itself is requesting different caches for the same key.
    
    (cherry picked from commit 9d0de53519ac3ab6d9a686bbf8dbeec79376adc6)
---
 include/ts/parentselectdefs.h                      |  1 +
 .../experimental/parent_select/consistenthash.cc   | 45 +++++++++++++++++++---
 .../experimental/parent_select/consistenthash.h    |  4 +-
 .../experimental/parent_select/parent_select.cc    | 12 ++++--
 plugins/experimental/parent_select/strategy.cc     | 15 ++++++--
 plugins/experimental/parent_select/strategy.h      | 18 +++++++--
 proxy/http/HttpTransact.cc                         | 13 +++++--
 7 files changed, 87 insertions(+), 21 deletions(-)

diff --git a/include/ts/parentselectdefs.h b/include/ts/parentselectdefs.h
index 021b918..e4ca8b4 100644
--- a/include/ts/parentselectdefs.h
+++ b/include/ts/parentselectdefs.h
@@ -57,4 +57,5 @@ typedef struct {
   bool responseIsRetryable;
   bool goDirect;
   bool parentIsProxy;
+  bool no_cache;
 } TSResponseAction;
diff --git a/plugins/experimental/parent_select/consistenthash.cc b/plugins/experimental/parent_select/consistenthash.cc
index 831caf9..00f20e0 100644
--- a/plugins/experimental/parent_select/consistenthash.cc
+++ b/plugins/experimental/parent_select/consistenthash.cc
@@ -161,6 +161,14 @@ PLNextHopConsistentHash::Init(const YAML::Node &n)
     hash.clear();
     rings.push_back(hash_ring);
   }
+
+  if (ring_mode == PL_NH_PEERING_RING) {
+    if (groups != 2) {
+      PL_NH_Error("ring mode is '%s', requires two host groups (peering group and an upstream group).", peering_rings.data());
+      return false;
+    }
+  }
+
   return true;
 }
 
@@ -248,11 +256,13 @@ PLNextHopConsistentHash::getHashKey(uint64_t sm_id, TSMBuffer reqp, TSMLoc url,
 }
 
 static void
-makeNextParentErr(const char **hostname, size_t *hostname_len, in_port_t *port)
+makeNextParentErr(const char **hostname, size_t *hostname_len, in_port_t *port, bool *retry, bool *no_cache)
 {
   *hostname     = nullptr;
   *hostname_len = 0;
   *port         = 0;
+  *retry        = false;
+  *no_cache     = false;
 }
 
 void *
@@ -280,7 +290,7 @@ PLNextHopConsistentHash::deleteTxn(void *txn)
 void
 PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exclude_hostname, size_t exclude_hostname_len,
                               in_port_t exclude_port, const char **out_hostname, size_t *out_hostname_len, in_port_t *out_port,
-                              bool *out_retry, time_t now)
+                              bool *out_retry, bool *out_no_cache, time_t now)
 {
   // TODO add logic in the strategy to track when someone is retrying, and not give it out to multiple threads at once, to prevent
   // thundering retries See github issue #7485
@@ -296,7 +306,7 @@ PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exc
   TSMLoc hdr;
   ScopedFreeMLoc hdr_cleanup(&reqp, TS_NULL_MLOC, &hdr);
   if (TSHttpTxnClientReqGet(txnp, &reqp, &hdr) == TS_ERROR) {
-    makeNextParentErr(out_hostname, out_hostname_len, out_port);
+    makeNextParentErr(out_hostname, out_hostname_len, out_port, out_retry, out_no_cache);
     return;
   }
 
@@ -304,7 +314,7 @@ PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exc
   ScopedFreeMLoc parent_selection_url_cleanup(&reqp, TS_NULL_MLOC, &parent_selection_url);
   if (TSUrlCreate(reqp, &parent_selection_url) != TS_SUCCESS) {
     PL_NH_Error("nexthop failed to create url for parent_selection_url");
-    makeNextParentErr(out_hostname, out_hostname_len, out_port);
+    makeNextParentErr(out_hostname, out_hostname_len, out_port, out_retry, out_no_cache);
     return;
   }
   if (TSHttpTxnParentSelectionUrlGet(txnp, reqp, parent_selection_url) != TS_SUCCESS) {
@@ -315,7 +325,7 @@ PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exc
   ScopedFreeMLoc url_cleanup(&reqp, hdr, &url);
   if (TSHttpHdrUrlGet(reqp, hdr, &url) != TS_SUCCESS) {
     PL_NH_Error("failed to get header url, cannot find next hop");
-    makeNextParentErr(out_hostname, out_hostname_len, out_port);
+    makeNextParentErr(out_hostname, out_hostname_len, out_port, out_retry, out_no_cache);
     return;
   }
 
@@ -326,7 +336,7 @@ PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exc
   if (TSHttpTxnConfigIntGet(txnp, TS_CONFIG_HTTP_PARENT_PROXY_RETRY_TIME, &retry_time) != TS_SUCCESS) {
     // TODO get and cache on init, to prevent potential runtime failure?
     PL_NH_Error("failed to get parent retry time, cannot find next hop");
-    makeNextParentErr(out_hostname, out_hostname_len, out_port);
+    makeNextParentErr(out_hostname, out_hostname_len, out_port, out_retry, out_no_cache);
     return;
   }
 
@@ -367,6 +377,12 @@ PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exc
         cur_ring = state->last_group;
       }
       break;
+    case PL_NH_PEERING_RING:
+      ink_assert(groups == 2);
+      // look for the next parent on the
+      // upstream ring.
+      state->last_group = cur_ring = 1;
+      break;
     case PL_NH_EXHAUST_RING:
     default:
       if (!wrapped) {
@@ -395,6 +411,13 @@ PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exc
         const bool hostExists =
           pRec ? (TSHostStatusGet(pRec->hostname.c_str(), pRec->hostname.size(), &hostStatus, nullptr) == TS_SUCCESS) : false;
         state->first_choice_status = hostExists ? hostStatus : TSHostStatus::TS_HOST_STATUS_UP;
+        // if peering and the selected host is myself, change rings and search for an upstream
+        // parent.
+        if (ring_mode == PL_NH_PEERING_RING && TSHostnameIsSelf(pRec->hostname.c_str(), pRec->hostname.size()) == TS_SUCCESS) {
+          // switch to the upstream ring.
+          cur_ring = 1;
+          continue;
+        }
         break;
       }
     } else {
@@ -529,6 +552,13 @@ PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exc
       break;
     }
     state->retry = nextHopRetry;
+
+    // if using a peering ring mode and the parent selected came from the 'peering' group,
+    // cur_ring == 0, then if the config allows it, set the flag to not cache the result.
+    state->no_cache = ring_mode == PL_NH_PEERING_RING && !cache_peer_result && cur_ring == 0;
+    PL_NH_Debug(PL_NH_DEBUG_TAG, "[%" PRIu64 "] setting do not cache response from a peer per config: %s", sm_id,
+                state->no_cache ? "true" : "false");
+
     ink_assert(state->hostname != nullptr);
     ink_assert(state->port != 0);
     PL_NH_Debug(PL_NH_DEBUG_TAG, "nextParent [%" PRIu64 "] state->result: %s Chosen parent: %.*s:%d", sm_id,
@@ -543,6 +573,7 @@ PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exc
     state->hostname_len = 0;
     state->port         = 0;
     state->retry        = false;
+    state->no_cache     = false;
     PL_NH_Debug(PL_NH_DEBUG_TAG, "nextParent [%" PRIu64 "] state->result: %s set hostname null port 0 retry false", sm_id,
                 PLNHParentResultStr[state->result]);
   }
@@ -550,6 +581,8 @@ PLNextHopConsistentHash::next(TSHttpTxn txnp, void *strategyTxn, const char *exc
   *out_hostname     = state->hostname;
   *out_hostname_len = state->hostname_len;
   *out_port         = state->port;
+  *out_retry        = state->retry;
+  *out_no_cache     = state->no_cache;
 }
 
 void
diff --git a/plugins/experimental/parent_select/consistenthash.h b/plugins/experimental/parent_select/consistenthash.h
index 7625b19..a8c46af 100644
--- a/plugins/experimental/parent_select/consistenthash.h
+++ b/plugins/experimental/parent_select/consistenthash.h
@@ -55,6 +55,7 @@ struct PLNextHopConsistentHashTxn {
   size_t hostname_len  = 0;
   in_port_t port       = 0;
   bool retry           = false;
+  bool no_cache        = false;
 };
 
 class PLNextHopConsistentHash : public PLNextHopSelectionStrategy
@@ -72,7 +73,8 @@ public:
   ~PLNextHopConsistentHash();
   bool Init(const YAML::Node &n);
   void next(TSHttpTxn txnp, void *strategyTxn, const char *exclude_hostname, size_t exclude_hostname_len, in_port_t exclude_port,
-            const char **out_hostname, size_t *out_hostname_len, in_port_t *out_port, bool *out_retry, time_t now = 0) override;
+            const char **out_hostname, size_t *out_hostname_len, in_port_t *out_port, bool *out_retry, bool *out_no_cache,
+            time_t now = 0) override;
   void mark(TSHttpTxn txnp, void *strategyTxn, const char *hostname, const size_t hostname_len, const in_port_t port,
             const PLNHCmd status, const time_t now) override;
   void *newTxn() override;
diff --git a/plugins/experimental/parent_select/parent_select.cc b/plugins/experimental/parent_select/parent_select.cc
index c6d4a1f..17ad8f1 100644
--- a/plugins/experimental/parent_select/parent_select.cc
+++ b/plugins/experimental/parent_select/parent_select.cc
@@ -51,6 +51,7 @@ struct StrategyTxn {
   size_t prev_host_len;
   in_port_t prev_port;
   bool prev_is_retry;
+  bool prev_no_cache;
 };
 
 int
@@ -81,9 +82,10 @@ handle_send_request(TSHttpTxn txnp, StrategyTxn *strategyTxn)
   strategyTxn->prev_host_len = ra.hostname_len;
   strategyTxn->prev_port     = ra.port;
   strategyTxn->prev_is_retry = ra.is_retry;
+  strategyTxn->prev_no_cache = ra.no_cache;
 
   strategy->next(txnp, strategyTxn->txn, ra.hostname, ra.hostname_len, ra.port, &ra.hostname, &ra.hostname_len, &ra.port,
-                 &ra.is_retry);
+                 &ra.is_retry, &ra.no_cache);
 
   ra.nextHopExists = strategy->nextHopExists(txnp);
   ra.fail = !ra.nextHopExists; // failed is whether to fail and return to the client. failed=false means to retry the parent we set
@@ -121,6 +123,7 @@ mark_response(TSHttpTxn txnp, StrategyTxn *strategyTxn, TSHttpStatus status)
     ra.hostname_len = strategyTxn->prev_host_len;
     ra.port         = strategyTxn->prev_port;
     ra.is_retry     = strategyTxn->prev_is_retry;
+    ra.no_cache     = strategyTxn->prev_no_cache;
     TSDebug(PLUGIN_NAME, "mark_response using prev %.*s:%d", int(ra.hostname_len), ra.hostname, ra.port);
   } else {
     TSHttpTxnResponseActionGet(txnp, &ra);
@@ -201,6 +204,7 @@ handle_read_response(TSHttpTxn txnp, StrategyTxn *strategyTxn)
   strategyTxn->prev_host_len = 0;
   strategyTxn->prev_port     = 0;
   strategyTxn->prev_is_retry = false;
+  strategyTxn->prev_no_cache = false;
 
   TSHandleMLocRelease(resp, TS_NULL_MLOC, resp_hdr);
   TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
@@ -235,6 +239,7 @@ handle_os_dns(TSHttpTxn txnp, StrategyTxn *strategyTxn)
     strategyTxn->prev_host     = nullptr;
     strategyTxn->prev_port     = 0;
     strategyTxn->prev_is_retry = false;
+    strategyTxn->prev_no_cache = false;
     TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
     return TS_SUCCESS;
   }
@@ -247,7 +252,7 @@ handle_os_dns(TSHttpTxn txnp, StrategyTxn *strategyTxn)
   const size_t exclude_host_len  = 0;
   const in_port_t exclude_port   = 0;
   strategy->next(txnp, strategyTxn->txn, exclude_host, exclude_host_len, exclude_port, &ra.hostname, &ra.hostname_len, &ra.port,
-                 &ra.is_retry);
+                 &ra.is_retry, &ra.no_cache);
 
   ra.fail = ra.hostname == nullptr; // failed is whether to immediately fail and return the client a 502. In this case: whether or
                                     // not we found another parent.
@@ -415,6 +420,7 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
   strategyTxn->prev_host     = nullptr;
   strategyTxn->prev_port     = 0;
   strategyTxn->prev_is_retry = false;
+  strategyTxn->prev_no_cache = false;
   TSContDataSet(cont, (void *)strategyTxn);
 
   // TSHttpTxnHookAdd(txnp, TS_HTTP_READ_REQUEST_HDR_HOOK, cont);
@@ -430,7 +436,7 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
   constexpr const size_t exclude_host_len  = 0;
   constexpr const in_port_t exclude_port   = 0;
   strategy->next(txnp, strategyTxn->txn, exclude_host, exclude_host_len, exclude_port, &ra.hostname, &ra.hostname_len, &ra.port,
-                 &ra.is_retry);
+                 &ra.is_retry, &ra.no_cache);
 
   if (ra.hostname == nullptr) {
     // TODO make configurable
diff --git a/plugins/experimental/parent_select/strategy.cc b/plugins/experimental/parent_select/strategy.cc
index e205f8f..8a822d4 100644
--- a/plugins/experimental/parent_select/strategy.cc
+++ b/plugins/experimental/parent_select/strategy.cc
@@ -47,12 +47,13 @@
 //
 
 // ring mode strings
-constexpr std::string_view alternate_rings = "alternate_ring";
-constexpr std::string_view exhaust_rings   = "exhaust_ring";
+const std::string_view alternate_rings = "alternate_ring";
+const std::string_view exhaust_rings   = "exhaust_ring";
+const std::string_view peering_rings   = "peering_ring";
 
 // health check strings
-constexpr std::string_view active_health_check  = "active";
-constexpr std::string_view passive_health_check = "passive";
+const std::string_view active_health_check  = "active";
+const std::string_view passive_health_check = "passive";
 
 PLNextHopSelectionStrategy::PLNextHopSelectionStrategy(const std::string_view &name)
 {
@@ -96,6 +97,10 @@ PLNextHopSelectionStrategy::Init(const YAML::Node &n)
       ignore_self_detect = n["ignore_self_detect"].as<bool>();
     }
 
+    if (n["cache_peer_result"]) {
+      cache_peer_result = n["cache_peer_result"].as<bool>();
+    }
+
     // failover node.
     YAML::Node failover_node;
     if (n["failover"]) {
@@ -106,6 +111,8 @@ PLNextHopSelectionStrategy::Init(const YAML::Node &n)
           ring_mode = PL_NH_ALTERNATE_RING;
         } else if (ring_mode_val == exhaust_rings) {
           ring_mode = PL_NH_EXHAUST_RING;
+        } else if (ring_mode_val == peering_rings) {
+          ring_mode = PL_NH_PEERING_RING;
         } else {
           ring_mode = PL_NH_ALTERNATE_RING;
           PL_NH_Note("Invalid 'ring_mode' value, '%s', for the strategy named '%s', using default '%s'.", ring_mode_val.c_str(),
diff --git a/plugins/experimental/parent_select/strategy.h b/plugins/experimental/parent_select/strategy.h
index df15514..deff4d1 100644
--- a/plugins/experimental/parent_select/strategy.h
+++ b/plugins/experimental/parent_select/strategy.h
@@ -38,6 +38,15 @@
 
 constexpr const char *PL_NH_DEBUG_TAG = "plugin_nexthop";
 
+// ring mode strings
+extern const std::string_view alternate_rings;
+extern const std::string_view exhaust_rings;
+extern const std::string_view peering_rings;
+
+// health check strings
+extern const std::string_view active_health_check;
+extern const std::string_view passive_health_check;
+
 #define PL_NH_Debug(tag, fmt, ...) TSDebug(tag, "[%s:%d]: " fmt, __FILE__, __LINE__, ##__VA_ARGS__)
 #define PL_NH_Error(fmt, ...) TSError("(%s) [%s:%d]: " fmt, PLUGIN_NAME, __FILE__, __LINE__, ##__VA_ARGS__)
 #define PL_NH_Note(fmt, ...) TSDebug(PL_NH_DEBUG_TAG, "[%s:%d]: " fmt, __FILE__, __LINE__, ##__VA_ARGS__)
@@ -59,7 +68,7 @@ enum PLNHPolicyType {
 
 enum PLNHSchemeType { PL_NH_SCHEME_NONE = 0, PL_NH_SCHEME_HTTP, PL_NH_SCHEME_HTTPS };
 
-enum PLNHRingMode { PL_NH_ALTERNATE_RING = 0, PL_NH_EXHAUST_RING };
+enum PLNHRingMode { PL_NH_ALTERNATE_RING = 0, PL_NH_EXHAUST_RING, PL_NH_PEERING_RING };
 
 // response codes container
 struct PLResponseCodes {
@@ -209,7 +218,7 @@ public:
   virtual const char *name()                                                                        = 0;
   virtual void next(TSHttpTxn txnp, void *strategyTxn, const char *exclude_hostname, size_t exclude_hostname_len,
                     in_port_t exclude_port, const char **out_hostname, size_t *out_hostname_len, in_port_t *out_port,
-                    bool *out_retry, time_t now = 0)                                                = 0;
+                    bool *out_retry, bool *out_no_cache, time_t now = 0)                            = 0;
   virtual void mark(TSHttpTxn txnp, void *strategyTxn, const char *hostname, const size_t hostname_len, const in_port_t port,
                     const PLNHCmd status, const time_t now = 0)                                     = 0;
   virtual bool nextHopExists(TSHttpTxn txnp)                                                        = 0;
@@ -234,9 +243,9 @@ public:
 
   virtual void next(TSHttpTxn txnp, void *strategyTxn, const char *exclude_hostname, size_t exclude_hostname_len,
                     in_port_t exclude_port, const char **out_hostname, size_t *out_hostname_len, in_port_t *out_port,
-                    bool *out_retry, time_t now = 0)            = 0;
+                    bool *out_retry, bool *out_no_cache, time_t now = 0) = 0;
   virtual void mark(TSHttpTxn txnp, void *strategyTxn, const char *hostname, const size_t hostname_len, const in_port_t port,
-                    const PLNHCmd status, const time_t now = 0) = 0;
+                    const PLNHCmd status, const time_t now = 0)          = 0;
   virtual bool nextHopExists(TSHttpTxn txnp);
   virtual bool codeIsFailure(TSHttpStatus response_code);
   virtual bool responseIsRetryable(unsigned int current_retry_attempts, TSHttpStatus response_code);
@@ -256,6 +265,7 @@ protected:
   bool go_direct          = true;
   bool parent_is_proxy    = true;
   bool ignore_self_detect = false;
+  bool cache_peer_result  = true;
   PLNHSchemeType scheme   = PL_NH_SCHEME_NONE;
   PLNHRingMode ring_mode  = PL_NH_ALTERNATE_RING;
   PLResponseCodes resp_codes;
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index a21483c..b0a794b 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -3658,9 +3658,16 @@ HttpTransact::handle_response_from_parent(State *s)
     }
     // the next hop strategy is configured not
     // to cache a response from a next hop peer.
-    if (s->parent_result.do_not_cache_response) {
-      TxnDebug("http_trans", "response is from a next hop peer, do not cache.");
-      s->cache_info.action = CACHE_DO_NO_ACTION;
+    if (s->response_action.handled) {
+      if (s->response_action.action.no_cache) {
+        TxnDebug("http_trans", "plugin set response_action.no_cache, do not cache.");
+        s->cache_info.action = CACHE_DO_NO_ACTION;
+      }
+    } else {
+      if (s->parent_result.do_not_cache_response) {
+        TxnDebug("http_trans", "response is from a next hop peer, do not cache.");
+        s->cache_info.action = CACHE_DO_NO_ACTION;
+      }
     }
     handle_forward_server_connection_open(s);
     break;