diff --git a/plugins/experimental/parent_select/consistenthash.cc b/plugins/experimental/parent_select/consistenthash.cc index 3eeac446113..4546e0ab5a3 100644 --- a/plugins/experimental/parent_select/consistenthash.cc +++ b/plugins/experimental/parent_select/consistenthash.cc @@ -106,8 +106,6 @@ PLNextHopConsistentHash::chashLookup(const std::shared_ptr &r } } -PLNextHopConsistentHash::PLNextHopConsistentHash(const std::string_view name) : PLNextHopSelectionStrategy(name) {} - PLNextHopConsistentHash::~PLNextHopConsistentHash() { PL_NH_Debug(PL_NH_DEBUG_TAG, "destructor called for strategy named: %s", strategy_name.c_str()); @@ -115,10 +113,10 @@ PLNextHopConsistentHash::~PLNextHopConsistentHash() #define PLUGIN_NAME "pparent_select" -bool -PLNextHopConsistentHash::Init(const YAML::Node &n) +PLNextHopConsistentHash::PLNextHopConsistentHash(const std::string_view name, const YAML::Node &n) + : PLNextHopSelectionStrategy(name, n) { - TSDebug("pparent_select", "PLNextHopConsistentHash Init calling."); + TSDebug("pparent_select", "PLNextHopConsistentHash constructor calling."); ATSHash64Sip24 hash; @@ -144,24 +142,10 @@ PLNextHopConsistentHash::Init(const YAML::Node &n) } } } catch (std::exception &ex) { - TSDebug(PLUGIN_NAME, "Error parsing the strategy named '%s' due to '%s', this strategy will be ignored.", strategy_name.c_str(), - ex.what()); - - PL_NH_Note("Error parsing the strategy named '%s' due to '%s', this strategy will be ignored.", strategy_name.c_str(), - ex.what()); - return false; + throw std::invalid_argument("Error parsing the strategy named '" + strategy_name + "' due to '" + ex.what() + + "', this strategy will be ignored."); } - TSDebug(PLUGIN_NAME, "PLNextHopConsistentHash::Init strat Init calling."); - - bool result = PLNextHopSelectionStrategy::Init(n); - if (!result) { - TSDebug(PLUGIN_NAME, "PLNextHopConsistentHash::Init strat Init false."); - return false; - } - - TSDebug(PLUGIN_NAME, "PLNextHopConsistentHash::Init strat Init called."); - // load up the hash rings. for (uint32_t i = 0; i < groups; i++) { std::shared_ptr hash_ring = std::make_shared(); @@ -187,22 +171,15 @@ PLNextHopConsistentHash::Init(const YAML::Node &n) if (ring_mode == PL_NH_PEERING_RING) { if (groups == 1) { if (!go_direct) { - PL_NH_Error("when ring mode is '%s', go_direct must be true when there is only one host group.", peering_rings.data()); - return false; + throw std::invalid_argument("ring mode '" + std::string(peering_rings) + + "' go_direct must be true when there is only one host group"); } } else if (groups != 2) { - PL_NH_Error("when ring mode is '%s', requires two host groups (peering group and an upstream group)," - " or just a single peering group with go_direct.", - peering_rings.data()); - return false; + throw std::invalid_argument( + "ring mode '" + std::string(peering_rings) + + "' requires two host groups (peering group and an upstream group), or a single peering group with go_direct"); } - // if (policy_type != PL_NH_CONSISTENT_HASH) { - // PL_NH_Error("ring mode '%s', is only implemented for a 'consistent_hash' policy.", peering_rings.data()); - // return false; - // } } - - return true; } // returns a hash key calculated from the request and 'hash_key' configuration diff --git a/plugins/experimental/parent_select/consistenthash.h b/plugins/experimental/parent_select/consistenthash.h index 6232f07fc29..c24a2fb4e03 100644 --- a/plugins/experimental/parent_select/consistenthash.h +++ b/plugins/experimental/parent_select/consistenthash.h @@ -73,9 +73,8 @@ class PLNextHopConsistentHash : public PLNextHopSelectionStrategy PLNHHashKeyType hash_key = PL_NH_PATH_HASH_KEY; PLNextHopConsistentHash() = delete; - PLNextHopConsistentHash(const std::string_view name); + PLNextHopConsistentHash(const std::string_view name, const YAML::Node &n); ~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, bool *out_no_cache, time_t now = 0) override; diff --git a/plugins/experimental/parent_select/consistenthash_config.cc b/plugins/experimental/parent_select/consistenthash_config.cc index 825b1e15b59..544e07b42fb 100644 --- a/plugins/experimental/parent_select/consistenthash_config.cc +++ b/plugins/experimental/parent_select/consistenthash_config.cc @@ -66,14 +66,14 @@ TSNextHopSelectionStrategy * createStrategy(const std::string &name, const YAML::Node &node) { TSDebug(PLUGIN_NAME, "createStrategy %s calling.", name.c_str()); - PLNextHopConsistentHash *st = new PLNextHopConsistentHash(name); - TSDebug(PLUGIN_NAME, "createStrategy %s newed %p.", name.c_str(), (void *)st); - if (!st->Init(node)) { - TSDebug(PLUGIN_NAME, "createStrategy %s init failed, returning nullptr.", name.c_str()); + try { + PLNextHopConsistentHash *st = new PLNextHopConsistentHash(name, node); + TSDebug(PLUGIN_NAME, "createStrategy %s succeeded, returning object", name.c_str()); + return st; + } catch (std::exception &ex) { + TSError("[%s] creating strategies '%s' threw '%s', returning nullptr", PLUGIN_NAME, name.c_str(), ex.what()); return nullptr; } - TSDebug(PLUGIN_NAME, "createStrategy %s init succeeded, returning obj.", name.c_str()); - return st; } // Caller takes ownership of the returned pointers in the map, and must call delete on them. diff --git a/plugins/experimental/parent_select/strategy.cc b/plugins/experimental/parent_select/strategy.cc index 270ff8cc853..385f04c0e2e 100644 --- a/plugins/experimental/parent_select/strategy.cc +++ b/plugins/experimental/parent_select/strategy.cc @@ -55,18 +55,9 @@ const std::string_view peering_rings = "peering_ring"; const std::string_view active_health_check = "active"; const std::string_view passive_health_check = "passive"; -PLNextHopSelectionStrategy::PLNextHopSelectionStrategy(const std::string_view &name) +PLNextHopSelectionStrategy::PLNextHopSelectionStrategy(const std::string_view &name, const YAML::Node &n) : strategy_name(name) { - strategy_name = name; -} - -// -// parse out the data for this strategy. -// -bool -PLNextHopSelectionStrategy::Init(const YAML::Node &n) -{ - PL_NH_Debug(PL_NH_DEBUG_TAG, "calling Init()"); + PL_NH_Debug(PL_NH_DEBUG_TAG, "PLNextHopSelectionStrategy constructor calling"); std::string self_host; bool self_host_used = false; @@ -249,30 +240,9 @@ PLNextHopSelectionStrategy::Init(const YAML::Node &n) throw std::invalid_argument("self host (" + self_host + ") does not appear in the first (peer) group"); } } catch (std::exception &ex) { - PL_NH_Note("Error parsing the strategy named '%s' due to '%s', this strategy will be ignored.", strategy_name.c_str(), - ex.what()); - return false; + throw std::invalid_argument("Error parsing strategy named '" + strategy_name + "' due to '" + ex.what() + + "', this strategy will be ignored."); } - - if (ring_mode == PL_NH_PEERING_RING) { - if (groups == 1) { - if (!go_direct) { - PL_NH_Error("when ring mode is '%s', go_direct must be true when there is only one host group.", peering_rings.data()); - return false; - } - } else if (groups != 2) { - PL_NH_Error("when ring mode is '%s', requires two host groups (peering group and an upstream group)," - " or just a single peering group with go_direct.", - peering_rings.data()); - return false; - } - // if (policy_type != PL_NH_CONSISTENT_HASH) { - // PL_NH_Error("ring mode '%s', is only implemented for a 'consistent_hash' policy.", peering_rings.data()); - // return false; - // } - } - - return true; } bool diff --git a/plugins/experimental/parent_select/strategy.h b/plugins/experimental/parent_select/strategy.h index 844e75a7a7f..e2db4fcc29d 100644 --- a/plugins/experimental/parent_select/strategy.h +++ b/plugins/experimental/parent_select/strategy.h @@ -240,10 +240,9 @@ class TSNextHopSelectionStrategy class PLNextHopSelectionStrategy : public TSNextHopSelectionStrategy { public: - PLNextHopSelectionStrategy(); - PLNextHopSelectionStrategy(const std::string_view &name); + PLNextHopSelectionStrategy() = delete; + PLNextHopSelectionStrategy(const std::string_view &name, const YAML::Node &n); virtual ~PLNextHopSelectionStrategy(){}; - 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, bool *out_no_cache, diff --git a/proxy/http/remap/NextHopConsistentHash.cc b/proxy/http/remap/NextHopConsistentHash.cc index b94bb1e15f9..e49e6b11a32 100644 --- a/proxy/http/remap/NextHopConsistentHash.cc +++ b/proxy/http/remap/NextHopConsistentHash.cc @@ -83,8 +83,8 @@ NextHopConsistentHash::~NextHopConsistentHash() NH_Debug(NH_DEBUG_TAG, "destructor called for strategy named: %s", strategy_name.c_str()); } -bool -NextHopConsistentHash::Init(ts::Yaml::Map &n) +NextHopConsistentHash::NextHopConsistentHash(const std::string_view name, const NHPolicyType &policy, ts::Yaml::Map &n) + : NextHopSelectionStrategy(name, policy, n) { ATSHash64Sip24 hash; @@ -110,13 +110,8 @@ NextHopConsistentHash::Init(ts::Yaml::Map &n) } } } catch (std::exception &ex) { - NH_Note("Error parsing the strategy named '%s' due to '%s', this strategy will be ignored.", strategy_name.c_str(), ex.what()); - return false; - } - - bool result = NextHopSelectionStrategy::Init(n); - if (!result) { - return false; + throw std::invalid_argument("Error parsing the strategy named '" + strategy_name + "' due to '" + ex.what() + + "', this strategy will be ignored."); } // load up the hash rings. @@ -140,7 +135,6 @@ NextHopConsistentHash::Init(ts::Yaml::Map &n) hash.clear(); rings.push_back(std::move(hash_ring)); } - return true; } // returns a hash key calculated from the request and 'hash_key' configuration diff --git a/proxy/http/remap/NextHopConsistentHash.h b/proxy/http/remap/NextHopConsistentHash.h index 7802fdc2022..f439352e9f1 100644 --- a/proxy/http/remap/NextHopConsistentHash.h +++ b/proxy/http/remap/NextHopConsistentHash.h @@ -47,9 +47,9 @@ class NextHopConsistentHash : public NextHopSelectionStrategy NHHashKeyType hash_key = NH_PATH_HASH_KEY; NextHopConsistentHash() = delete; - NextHopConsistentHash(const std::string_view name, const NHPolicyType &policy) : NextHopSelectionStrategy(name, policy) {} + NextHopConsistentHash(const std::string_view name, const NHPolicyType &policy, ts::Yaml::Map &n); ~NextHopConsistentHash(); - bool Init(ts::Yaml::Map &n); + void findNextHop(TSHttpTxn txnp, void *ih = nullptr, time_t now = 0) override; std::shared_ptr chashLookup(const std::shared_ptr &ring, uint32_t cur_ring, ParentResult &result, HttpRequestData &request_info, bool *wrapped, uint64_t sm_id); diff --git a/proxy/http/remap/NextHopRoundRobin.h b/proxy/http/remap/NextHopRoundRobin.h index 024efee70d7..a5d08c7df6b 100644 --- a/proxy/http/remap/NextHopRoundRobin.h +++ b/proxy/http/remap/NextHopRoundRobin.h @@ -33,12 +33,10 @@ class NextHopRoundRobin : public NextHopSelectionStrategy public: NextHopRoundRobin() = delete; - NextHopRoundRobin(const std::string_view &name, const NHPolicyType &policy) : NextHopSelectionStrategy(name, policy) {} - ~NextHopRoundRobin(); - bool - Init(ts::Yaml::Map &n) + NextHopRoundRobin(const std::string_view &name, const NHPolicyType &policy, ts::Yaml::Map &n) + : NextHopSelectionStrategy(name, policy, n) { - return NextHopSelectionStrategy::Init(n); } + ~NextHopRoundRobin(); void findNextHop(TSHttpTxn txnp, void *ih = nullptr, time_t now = 0) override; }; diff --git a/proxy/http/remap/NextHopSelectionStrategy.cc b/proxy/http/remap/NextHopSelectionStrategy.cc index 0ae5677f314..db96a48fc88 100644 --- a/proxy/http/remap/NextHopSelectionStrategy.cc +++ b/proxy/http/remap/NextHopSelectionStrategy.cc @@ -41,21 +41,12 @@ constexpr std::string_view passive_health_check = "passive"; constexpr const char *policy_strings[] = {"NH_UNDEFINED", "NH_FIRST_LIVE", "NH_RR_STRICT", "NH_RR_IP", "NH_RR_LATCHED", "NH_CONSISTENT_HASH"}; -NextHopSelectionStrategy::NextHopSelectionStrategy(const std::string_view &name, const NHPolicyType &policy) +NextHopSelectionStrategy::NextHopSelectionStrategy(const std::string_view &name, const NHPolicyType &policy, ts::Yaml::Map &n) + : strategy_name(name), policy_type(policy) { - strategy_name = name; - policy_type = policy; - + NH_Debug(NH_DEBUG_TAG, "NextHopSelectionStrategy calling constructor"); NH_Debug(NH_DEBUG_TAG, "Using a selection strategy of type %s", policy_strings[policy]); -} -// -// parse out the data for this strategy. -// -bool -NextHopSelectionStrategy::Init(ts::Yaml::Map &n) -{ - NH_Debug(NH_DEBUG_TAG, "calling Init()"); std::string self_host; bool self_host_used = false; @@ -235,29 +226,26 @@ NextHopSelectionStrategy::Init(ts::Yaml::Map &n) throw std::invalid_argument("self host (" + self_host + ") does not appear in the first (peer) group"); } } catch (std::exception &ex) { - NH_Error("Error parsing the strategy named '%s' due to '%s', this strategy will be ignored.", strategy_name.c_str(), ex.what()); - return false; + throw std::invalid_argument("Error parsing the strategy named '" + strategy_name + "' due to '" + ex.what() + + "', this strategy will be ignored."); } if (ring_mode == NH_PEERING_RING) { if (groups == 1) { if (!go_direct) { - NH_Error("when ring mode is '%s', go_direct must be true when there is only one host group.", peering_rings.data()); - return false; + throw std::invalid_argument("ring mode '" + std::string(peering_rings) + + "' go_direct must be true when there is only one host group"); } } else if (groups != 2) { - NH_Error("when ring mode is '%s', requires two host groups (peering group and an upstream group)," - " or just a single peering group with go_direct.", - peering_rings.data()); - return false; + throw std::invalid_argument( + "ring mode '" + std::string(peering_rings) + + "' requires two host groups (peering group and an upstream group), or a single peering group with go_direct"); } if (policy_type != NH_CONSISTENT_HASH) { - NH_Error("ring mode '%s', is only implemented for a 'consistent_hash' policy.", peering_rings.data()); - return false; + throw std::invalid_argument("ring mode '" + std::string(peering_rings) + + "' is only implemented for a 'consistent_hash' policy"); } } - - return true; } void diff --git a/proxy/http/remap/NextHopSelectionStrategy.h b/proxy/http/remap/NextHopSelectionStrategy.h index 667b68e5b61..54c0df5fcef 100644 --- a/proxy/http/remap/NextHopSelectionStrategy.h +++ b/proxy/http/remap/NextHopSelectionStrategy.h @@ -193,10 +193,9 @@ class NextHopHealthStatus : public NHHealthStatus class NextHopSelectionStrategy { public: - NextHopSelectionStrategy(); - NextHopSelectionStrategy(const std::string_view &name, const NHPolicyType &type); + NextHopSelectionStrategy() = delete; + NextHopSelectionStrategy(const std::string_view &name, const NHPolicyType &type, ts::Yaml::Map &n); virtual ~NextHopSelectionStrategy(){}; - bool Init(ts::Yaml::Map &n); virtual void findNextHop(TSHttpTxn txnp, void *ih = nullptr, time_t now = 0) = 0; void markNextHop(TSHttpTxn txnp, const char *hostname, const int port, const NHCmd status, void *ih = nullptr, const time_t now = 0); diff --git a/proxy/http/remap/NextHopStrategyFactory.cc b/proxy/http/remap/NextHopStrategyFactory.cc index 2d7ce793e14..faa67b0244e 100644 --- a/proxy/http/remap/NextHopStrategyFactory.cc +++ b/proxy/http/remap/NextHopStrategyFactory.cc @@ -139,29 +139,25 @@ NextHopStrategyFactory::createStrategy(const std::string &name, const NHPolicyTy return; } - switch (policy_type) { - case NH_FIRST_LIVE: - case NH_RR_STRICT: - case NH_RR_IP: - case NH_RR_LATCHED: - strat_rr = std::make_shared(name, policy_type); - if (strat_rr->Init(node)) { + try { + switch (policy_type) { + case NH_FIRST_LIVE: + case NH_RR_STRICT: + case NH_RR_IP: + case NH_RR_LATCHED: + strat_rr = std::make_shared(name, policy_type, node); _strategies.emplace(std::make_pair(std::string(name), strat_rr)); - } else { - strat.reset(); - } - break; - case NH_CONSISTENT_HASH: - strat_chash = std::make_shared(name, policy_type); - if (strat_chash->Init(node)) { + break; + case NH_CONSISTENT_HASH: + strat_chash = std::make_shared(name, policy_type, node); _strategies.emplace(std::make_pair(std::string(name), strat_chash)); - } else { - strat_chash.reset(); - } - break; - default: // handles P_UNDEFINED, no strategy is added - break; - }; + break; + default: // handles P_UNDEFINED, no strategy is added + break; + }; + } catch (std::exception &ex) { + strat.reset(); + } } std::shared_ptr