From 0200d19e86b2a8cf868e806195deaa839424903e Mon Sep 17 00:00:00 2001 From: Robert Butts Date: Wed, 5 Jan 2022 14:21:34 -0700 Subject: [PATCH] Ports #7897 from core strategies to parent_select plugin. 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. --- include/ts/parentselectdefs.h | 1 + .../parent_select/consistenthash.cc | 45 ++++++++++++++++--- .../parent_select/consistenthash.h | 4 +- .../parent_select/parent_select.cc | 12 +++-- .../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 021b91858af..e4ca8b4ed72 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 c50b5993edb..0f950b98044 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 { @@ -531,6 +554,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, @@ -545,6 +575,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]); } @@ -552,6 +583,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 7625b198c40..a8c46af9977 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 @@ class PLNextHopConsistentHash : public PLNextHopSelectionStrategy ~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 6641cbb0d19..1a32ead6cfd 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. @@ -419,6 +424,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); @@ -434,7 +440,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 3435b467ca8..b329c7e52ef 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(); } + if (n["cache_peer_result"]) { + cache_peer_result = n["cache_peer_result"].as(); + } + // 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 bd2be7742ff..62d27a3d74d 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 @@ class TSNextHopSelectionStrategy 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 @@ class PLNextHopSelectionStrategy : public TSNextHopSelectionStrategy 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 @@ class PLNextHopSelectionStrategy : public TSNextHopSelectionStrategy 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; // simple retry codes diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index bb277fa19b9..1039ee7a29c 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -3642,9 +3642,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;