From 16e21c7b1cf80d52a53a0da1bdbb7a27e34a4a7d Mon Sep 17 00:00:00 2001 From: John Rushford Date: Wed, 29 Sep 2021 21:09:19 +0000 Subject: [PATCH] Fixes issue #8329 crash in NextHopConsistentHash where when the only host in a strategy is unavailable due to DNS lookup failures. - refactored NextHopConsistentHash::findNextHop() to fix the crash and simplify it. - fixed the unit tests in test_NextHopConsistentHash.cc so that test failure checks using strcmp() do not crash with a nullptr hostname result. --- proxy/http/remap/NextHopConsistentHash.cc | 356 +++++++++--------- proxy/http/remap/NextHopConsistentHash.h | 5 +- .../unit-tests/test_NextHopConsistentHash.cc | 53 +-- 3 files changed, 212 insertions(+), 202 deletions(-) diff --git a/proxy/http/remap/NextHopConsistentHash.cc b/proxy/http/remap/NextHopConsistentHash.cc index f6f304aa798..33b0dea8fe1 100644 --- a/proxy/http/remap/NextHopConsistentHash.cc +++ b/proxy/http/remap/NextHopConsistentHash.cc @@ -23,7 +23,6 @@ #include -#include "tscore/HashSip.h" #include "HttpSM.h" #include "I_Machine.h" #include "YamlCfg.h" @@ -37,25 +36,46 @@ constexpr std::string_view hash_key_path_query = "path+query"; constexpr std::string_view hash_key_path_fragment = "path+fragment"; constexpr std::string_view hash_key_cache = "cache_key"; -static HostRecord * -chash_lookup(const std::shared_ptr &ring, uint64_t hash_key, ATSConsistentHashIter *iter, bool *wrapped, - ATSHash64Sip24 *hash, bool *hash_init, bool *mapWrapped, uint64_t sm_id) +static bool +isWrapped(std::vector &wrap_around, uint32_t groups) { - HostRecord *host_rec = nullptr; + bool all_wrapped = true; + for (uint32_t c = 0; c < groups; c++) { + if (wrap_around[c] == false) { + all_wrapped = false; + } + } + return all_wrapped; +} + +std::shared_ptr +NextHopConsistentHash::chashLookup(const std::shared_ptr &ring, uint32_t cur_ring, ParentResult &result, + HttpRequestData &request_info, bool *wrapped, uint64_t sm_id) +{ + uint64_t hash_key = 0; + ATSHash64Sip24 hash; + HostRecord *host_rec = nullptr; + ATSConsistentHashIter *iter = &result.chashIter[cur_ring]; - if (*hash_init == false) { - host_rec = static_cast(ring->lookup_by_hashval(hash_key, iter, wrapped)); - *hash_init = true; + if (result.chash_init[cur_ring] == false) { + hash_key = getHashKey(sm_id, request_info, &hash); + host_rec = static_cast(ring->lookup_by_hashval(hash_key, iter, wrapped)); + result.chash_init[cur_ring] = true; } else { - host_rec = static_cast(ring->lookup(nullptr, iter, wrapped, hash)); + host_rec = static_cast(ring->lookup(nullptr, iter, wrapped, &hash)); } bool wrap_around = *wrapped; - *wrapped = (*mapWrapped && *wrapped) ? true : false; - if (!*mapWrapped && wrap_around) { - *mapWrapped = true; + *wrapped = (result.mapWrapped[cur_ring] && *wrapped) ? true : false; + if (!result.mapWrapped[cur_ring] && wrap_around) { + result.mapWrapped[cur_ring] = true; } - return host_rec; + if (host_rec == nullptr) { + return nullptr; + } else { + std::shared_ptr h = host_groups[host_rec->group_index][host_rec->host_index]; + return h; + } } NextHopConsistentHash::~NextHopConsistentHash() @@ -126,9 +146,9 @@ NextHopConsistentHash::Init(ts::Yaml::Map &n) // returns a hash key calculated from the request and 'hash_key' configuration // parameter. uint64_t -NextHopConsistentHash::getHashKey(uint64_t sm_id, HttpRequestData *hrdata, ATSHash64 *h) +NextHopConsistentHash::getHashKey(uint64_t sm_id, const HttpRequestData &hrdata, ATSHash64 *h) { - URL *url = hrdata->hdr->url_get(); + URL *url = hrdata.hdr->url_get(); URL *ps_url = nullptr; int len = 0; const char *url_string_ref = nullptr; @@ -177,7 +197,7 @@ NextHopConsistentHash::getHashKey(uint64_t sm_id, HttpRequestData *hrdata, ATSHa break; // use the cache key created by the cache-key plugin. case NH_CACHE_HASH_KEY: - ps_url = *(hrdata->cache_info_parent_selection_url); + ps_url = *(hrdata.cache_info_parent_selection_url); if (ps_url) { url_string_ref = ps_url->string_get_ref(&len); if (url_string_ref && len > 0) { @@ -214,244 +234,230 @@ NextHopConsistentHash::findNextHop(TSHttpTxn txnp, void *ih, time_t now) { uint32_t const NO_RING_USE_POST_REMAP = uint32_t(0) - 1; - HttpSM *sm = reinterpret_cast(txnp); - ParentResult *result = &sm->t_state.parent_result; - HttpRequestData request_info = sm->t_state.request_data; - int64_t sm_id = sm->sm_id; - int64_t retry_time = sm->t_state.txn_conf->parent_retry_time; - time_t _now = now; - bool firstcall = false; - bool nextHopRetry = false; - bool wrapped = false; + HttpSM *sm = reinterpret_cast(txnp); + ParentResult &result = sm->t_state.parent_result; + HttpRequestData &request_info = sm->t_state.request_data; + int64_t sm_id = sm->sm_id; + int64_t retry_time = sm->t_state.txn_conf->parent_retry_time; + time_t _now = now; + bool firstcall = false; + bool nextHopRetry = false; + bool wrapped = false; std::vector wrap_around(groups, false); - uint32_t cur_ring = 0; // there is a hash ring for each host group - uint64_t hash_key = 0; - uint32_t lookups = 0; - ATSHash64Sip24 hash; - HostRecord *hostRec = nullptr; + uint32_t cur_ring = 0; // there is a hash ring for each host group + uint32_t lookups = 0; std::shared_ptr pRec = nullptr; HostStatus &pStatus = HostStatus::instance(); TSHostStatus host_stat = TSHostStatus::TS_HOST_STATUS_INIT; HostStatRec *hst = nullptr; Machine *machine = Machine::instance(); + std::string_view first_call_host; + int first_call_port = 0; - if (result->line_number == -1 && result->result == PARENT_UNDEFINED) { + if (result.line_number == -1 && result.result == PARENT_UNDEFINED) { firstcall = true; } + // firstcall indicates that this is the first time the state machine has called findNextHop() for this + // particular transaction so, a parent will be looked up using a hash from the request to locate a + // parent on the consistent hash ring. If not first call, then the transaction was unable to use the parent + // returned from the "firstcall" due to some error so, subsequent calls will not search using a hash but, + // will instead just increment the hash table iterator to find the next parent on the ring if (firstcall) { - NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] firstcall, line_number: %d, result: %s", sm_id, result->line_number, - ParentResultStr[result->result]); - result->line_number = distance; - cur_ring = 0; + NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] firstcall, line_number: %d, result: %s", sm_id, result.line_number, + ParentResultStr[result.result]); + result.line_number = distance; + cur_ring = 0; for (uint32_t i = 0; i < groups; i++) { - result->chash_init[i] = false; - wrap_around[i] = false; + result.chash_init[i] = false; + wrap_around[i] = false; } } else { - NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] not firstcall, line_number: %d, result: %s", sm_id, result->line_number, - ParentResultStr[result->result]); + // not first call, save the previously tried parent. + if (result.hostname) { + first_call_host = result.hostname; + first_call_port = result.port; + } + NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] not firstcall, line_number: %d, result: %s", sm_id, result.line_number, + ParentResultStr[result.result]); switch (ring_mode) { case NH_ALTERNATE_RING: if (groups > 1) { - cur_ring = (result->last_group + 1) % groups; + cur_ring = (result.last_group + 1) % groups; } else { - cur_ring = result->last_group; + cur_ring = result.last_group; } break; case NH_PEERING_RING: if (groups == 1) { - result->last_group = cur_ring = NO_RING_USE_POST_REMAP; + result.last_group = cur_ring = NO_RING_USE_POST_REMAP; } else { ink_assert(groups == 2); // look for the next parent on the // upstream ring. - result->last_group = cur_ring = 1; + result.last_group = cur_ring = 1; } break; case NH_EXHAUST_RING: default: if (!wrapped) { - cur_ring = result->last_group; + cur_ring = result.last_group; } else if (groups > 1) { - cur_ring = (result->last_group + 1) % groups; + cur_ring = (result.last_group + 1) % groups; } break; } } if (cur_ring != NO_RING_USE_POST_REMAP) { - // Do the initial parent look-up. - hash_key = getHashKey(sm_id, &request_info, &hash); + do { + // all host groups have been searched and there are no available parents found + if (isWrapped(wrap_around, groups)) { + NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] No available parents.", sm_id); + pRec = nullptr; + break; + } - do { // search until we've selected a different parent if !firstcall + // search for available parent std::shared_ptr r = rings[cur_ring]; - hostRec = chash_lookup(r, hash_key, &result->chashIter[cur_ring], &wrapped, &hash, &result->chash_init[cur_ring], - &result->mapWrapped[cur_ring], sm_id); - wrap_around[cur_ring] = wrapped; + pRec = chashLookup(r, cur_ring, result, request_info, &wrapped, sm_id); + hst = (pRec) ? pStatus.getHostStatus(pRec->hostname.c_str()) : nullptr; + wrap_around[cur_ring] = wrapped; lookups++; - // the 'available' flag is maintained in 'host_groups' and not the hash ring. - if (hostRec) { - pRec = host_groups[hostRec->group_index][hostRec->host_index]; + + // found a parent + if (pRec) { + bool is_self = machine->is_self(pRec->hostname.c_str()); + host_stat = (hst) ? hst->status : TSHostStatus::TS_HOST_STATUS_UP; + + // if the config ignore_self_detect is set to true and the host is down due to SELF_DETECT reason + // ignore the down status and mark it as available + if ((host_stat == TS_HOST_STATUS_DOWN && is_self && ignore_self_detect)) { + if (hst->reasons == Reason::SELF_DETECT) { + host_stat = TS_HOST_STATUS_UP; + } + } + if (firstcall) { - hst = (pRec) ? pStatus.getHostStatus(pRec->hostname.c_str()) : nullptr; - result->first_choice_status = (hst) ? hst->status : TSHostStatus::TS_HOST_STATUS_UP; - // if peering and the selected host is myself, change rings and search for an upstream - // parent. - if (ring_mode == NH_PEERING_RING && (pRec->self || machine->is_self(pRec->hostname.c_str()))) { + result.first_choice_status = (hst) ? hst->status : TSHostStatus::TS_HOST_STATUS_UP; + // if peering and the selected host is myself, change rings and search for an upstream parent. + if (ring_mode == NH_PEERING_RING && (pRec->self || is_self)) { if (groups == 1) { // use host from post-remap URL cur_ring = NO_RING_USE_POST_REMAP; pRec = nullptr; + break; } else { - // switch to the upstream ring. + // switch to and search the upstream ring. cur_ring = 1; + pRec = nullptr; continue; } } + } else { + // not first call, make sure we're not re-using the same parent, search again if we are. + if (first_call_host.size() > 0 && first_call_host == pRec->hostname && first_call_port == pRec->getPort(scheme)) { + pRec = nullptr; + continue; + } + } + // if the parent is not available check to see if the retry window has expired making it available + // for retry. + if (!pRec->available.load() && host_stat == TS_HOST_STATUS_UP) { + _now == 0 ? _now = time(nullptr) : _now = now; + if ((pRec->failedAt.load() + retry_time) < static_cast(_now)) { + NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] next hop %s, retriers: %d", sm_id, pRec->hostname.c_str(), pRec->retriers()); + if (pRec->retriers.inc(max_retriers)) { + nextHopRetry = true; + result.last_parent = pRec->host_index; + result.last_lookup = pRec->group_index; + result.retry = nextHopRetry; + result.result = PARENT_SPECIFIED; + NH_Debug(NH_DEBUG_TAG, + "[%" PRIu64 "] next hop %s is now retryable, marked it available, retriers: %d, max_retriers: %d.", sm_id, + pRec->hostname.c_str(), pRec->retriers(), max_retriers); + break; + } + } + } + + // use the available selected parent + if (pRec->available.load() && host_stat == TS_HOST_STATUS_UP) { break; } - } else { - pRec = nullptr; } - } while (pRec && result->hostname && strcmp(pRec->hostname.c_str(), result->hostname) == 0); - - NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] Initial parent lookups: %d", sm_id, lookups); - - // ---------------------------------------------------------------------------------------------------- - // Validate initial parent look-up and perform additional look-ups if required. - // ---------------------------------------------------------------------------------------------------- - - if (cur_ring != NO_RING_USE_POST_REMAP) { - hst = (pRec) ? pStatus.getHostStatus(pRec->hostname.c_str()) : nullptr; - host_stat = (hst) ? hst->status : TSHostStatus::TS_HOST_STATUS_UP; - // if the config ignore_self_detect is set to true and the host is down due to SELF_DETECT reason - // ignore the down status and mark it as available - if ((pRec && ignore_self_detect) && (hst && hst->status == TS_HOST_STATUS_DOWN)) { - if (hst->reasons == Reason::SELF_DETECT) { - host_stat = TS_HOST_STATUS_UP; + // try other rings per per the ring mode + switch (ring_mode) { + case NH_ALTERNATE_RING: + if (pRec && groups > 0) { + cur_ring = (pRec->group_index + 1) % groups; + } else { + cur_ring = 0; + } + break; + case NH_EXHAUST_RING: + default: + if (wrap_around[cur_ring] && groups > 1) { + cur_ring = (cur_ring + 1) % groups; } + break; } - if (!pRec || (pRec && !pRec->available.load()) || host_stat == TS_HOST_STATUS_DOWN) { - do { - // check if an unavailable server is now retryable, use it. - if (pRec && !pRec->available.load() && host_stat == TS_HOST_STATUS_UP) { - // check if the host is retryable. It's retryable if the retry window has elapsed - _now == 0 ? _now = time(nullptr) : _now = now; - if ((pRec->failedAt.load() + retry_time) < static_cast(_now)) { - NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] next hop %s, retriers: %d", sm_id, pRec->hostname.c_str(), pRec->retriers()); - if (pRec->retriers.inc(max_retriers)) { - nextHopRetry = true; - result->last_parent = pRec->host_index; - result->last_lookup = pRec->group_index; - result->retry = nextHopRetry; - result->result = PARENT_SPECIFIED; - NH_Debug(NH_DEBUG_TAG, - "[%" PRIu64 "] next hop %s is now retryable, marked it available, retriers: %d, max_retriers: %d.", sm_id, - pRec->hostname.c_str(), pRec->retriers(), max_retriers); - break; - } - } - } - switch (ring_mode) { - case NH_ALTERNATE_RING: - if (pRec && groups > 0) { - cur_ring = (pRec->group_index + 1) % groups; - } else { - cur_ring = 0; - } - break; - case NH_EXHAUST_RING: - default: - if (wrap_around[cur_ring] && groups > 1) { - cur_ring = (cur_ring + 1) % groups; - } - break; - } - std::shared_ptr r = rings[cur_ring]; - hostRec = chash_lookup(r, hash_key, &result->chashIter[cur_ring], &wrapped, &hash, &result->chash_init[cur_ring], - &result->mapWrapped[cur_ring], sm_id); - wrap_around[cur_ring] = wrapped; - lookups++; - if (hostRec) { - pRec = host_groups[hostRec->group_index][hostRec->host_index]; - hst = (pRec) ? pStatus.getHostStatus(pRec->hostname.c_str()) : nullptr; - host_stat = (hst) ? hst->status : TSHostStatus::TS_HOST_STATUS_UP; - // if the config ignore_self_detect is set to true and the host is down due to SELF_DETECT reason - // ignore the down status and mark it as available - if ((pRec && ignore_self_detect) && (hst && hst->status == TS_HOST_STATUS_DOWN)) { - if (hst->reasons == Reason::SELF_DETECT) { - host_stat = TS_HOST_STATUS_UP; - } - } - NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] Selected a new parent: %s, available: %s, wrapped: %s, lookups: %d.", sm_id, - pRec->hostname.c_str(), (pRec->available.load()) ? "true" : "false", (wrapped) ? "true" : "false", lookups); - // use available host. - if (pRec->available.load() && host_stat == TS_HOST_STATUS_UP) { - break; - } - } else { - pRec = nullptr; - } - bool all_wrapped = true; - for (uint32_t c = 0; c < groups; c++) { - if (wrap_around[c] == false) { - all_wrapped = false; - } - } - if (all_wrapped) { - NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] No available parents.", sm_id); - if (pRec) { - pRec = nullptr; - } - break; - } - } while (!pRec || (pRec && !pRec->available.load()) || host_stat == TS_HOST_STATUS_DOWN); + + if (pRec) { + // if the selected host is down, search again. + if (!pRec->available || host_stat == TS_HOST_STATUS_DOWN) { + NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] hostname: %s, available: %s, host_stat: %s", sm_id, pRec->hostname.c_str(), + pRec->available ? "true" : "false", HostStatusNames[host_stat]); + pRec = nullptr; + continue; + } } - } + } while (!pRec); + + NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] Initial parent lookups: %d", sm_id, lookups); } // ---------------------------------------------------------------------------------------------------- // Validate and return the final result. // ---------------------------------------------------------------------------------------------------- - if (pRec && host_stat == TS_HOST_STATUS_UP && (pRec->available.load() || result->retry)) { - result->result = PARENT_SPECIFIED; - result->hostname = pRec->hostname.c_str(); - result->last_parent = pRec->host_index; - result->last_lookup = result->last_group = cur_ring; + if (pRec && host_stat == TS_HOST_STATUS_UP && (pRec->available.load() || result.retry)) { + result.result = PARENT_SPECIFIED; + result.hostname = pRec->hostname.c_str(); + result.last_parent = pRec->host_index; + result.last_lookup = result.last_group = cur_ring; switch (scheme) { case NH_SCHEME_NONE: case NH_SCHEME_HTTP: - result->port = pRec->getPort(scheme); + result.port = pRec->getPort(scheme); break; case NH_SCHEME_HTTPS: - result->port = pRec->getPort(scheme); + result.port = pRec->getPort(scheme); break; } - result->retry = nextHopRetry; + result.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. if (ring_mode == NH_PEERING_RING && !cache_peer_result && cur_ring == 0) { - result->do_not_cache_response = true; + result.do_not_cache_response = true; NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] setting do not cache response from a peer per config: %s", sm_id, - (result->do_not_cache_response) ? "true" : "false"); + (result.do_not_cache_response) ? "true" : "false"); } - ink_assert(result->hostname != nullptr); - ink_assert(result->port != 0); - NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] result->result: %s Chosen parent: %s.%d", sm_id, ParentResultStr[result->result], - result->hostname, result->port); + ink_assert(result.hostname != nullptr); + ink_assert(result.port != 0); + NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] result->result: %s Chosen parent: %s.%d", sm_id, ParentResultStr[result.result], + result.hostname, result.port); } else { if (go_direct == true) { - result->result = PARENT_DIRECT; + result.result = PARENT_DIRECT; } else { - result->result = PARENT_FAIL; + result.result = PARENT_FAIL; } - result->hostname = nullptr; - result->port = 0; - result->retry = false; - NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] result->result: %s set hostname null port 0 retry false", sm_id, - ParentResultStr[result->result]); + result.hostname = nullptr; + result.port = 0; + result.retry = false; + NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] result.result: %s set hostname null port 0 retry false", sm_id, + ParentResultStr[result.result]); } return; diff --git a/proxy/http/remap/NextHopConsistentHash.h b/proxy/http/remap/NextHopConsistentHash.h index c2adf12da4f..7802fdc2022 100644 --- a/proxy/http/remap/NextHopConsistentHash.h +++ b/proxy/http/remap/NextHopConsistentHash.h @@ -25,6 +25,7 @@ #include #include +#include "tscore/HashSip.h" #include "NextHopSelectionStrategy.h" enum NHHashKeyType { @@ -40,7 +41,7 @@ class NextHopConsistentHash : public NextHopSelectionStrategy { std::vector> rings; - uint64_t getHashKey(uint64_t sm_id, HttpRequestData *hrdata, ATSHash64 *h); + uint64_t getHashKey(uint64_t sm_id, const HttpRequestData &hrdata, ATSHash64 *h); public: NHHashKeyType hash_key = NH_PATH_HASH_KEY; @@ -50,4 +51,6 @@ class NextHopConsistentHash : public NextHopSelectionStrategy ~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/unit-tests/test_NextHopConsistentHash.cc b/proxy/http/remap/unit-tests/test_NextHopConsistentHash.cc index ad4f0d24b10..460d648abac 100644 --- a/proxy/http/remap/unit-tests/test_NextHopConsistentHash.cc +++ b/proxy/http/remap/unit-tests/test_NextHopConsistentHash.cc @@ -94,7 +94,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", result->reset(); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "p1.foo.com") == 0); // mark down p1.foo.com. markNextHop looks at the 'result' @@ -107,7 +107,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", build_request(10002, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "p2.foo.com") == 0); // mark down p2.foo.com @@ -118,7 +118,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", build_request(10003, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "s2.bar.com") == 0); // mark down s2.bar.com @@ -129,7 +129,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", build_request(10004, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "s1.bar.com") == 0); // mark down s1.bar.com. @@ -140,7 +140,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", build_request(10005, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "q1.bar.com") == 0); // mark down q1.bar.com @@ -150,7 +150,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", build_request(10006, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "q2.bar.com") == 0); // mark down q2.bar.com @@ -170,7 +170,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", // simulating a failure triggers a search for another parent, not firstcall. build_request(10008, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp, nullptr, now); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "q2.bar.com") == 0); // free up request resources. @@ -225,7 +225,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co build_request(20001, &sm, nullptr, "rabbit.net", nullptr); result->reset(); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "p1.foo.com") == 0); // mark down p1.foo.com @@ -234,7 +234,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co build_request(20002, &sm, nullptr, "rabbit.net", nullptr); result->reset(); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "p2.foo.com") == 0); // mark down p2.foo.com @@ -244,7 +244,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co result->reset(); build_request(20003, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "s2.bar.com") == 0); // mark down s2.bar.com @@ -254,7 +254,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co result->reset(); build_request(20004, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "s1.bar.com") == 0); // mark down s1.bar.com @@ -264,7 +264,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co result->reset(); build_request(20005, &sm, nullptr, "rabbit.net/asset1", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "q1.bar.com") == 0); // sixth request - wait and p1 should now become available @@ -272,7 +272,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co result->reset(); build_request(20006, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp, nullptr, now); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "p1.foo.com") == 0); } // free up request resources. @@ -372,7 +372,7 @@ SCENARIO("Testing NextHop ignore_self_detect true", "[NextHopConsistentHash]") build_request(10001, &sm, nullptr, "rabbit.net", nullptr); result->reset(); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "localhost") == 0); CHECK(result->port == 8000); br_destroy(sm); @@ -422,7 +422,7 @@ SCENARIO("Testing NextHopConsistentHash same host different port markdown", "[Ne result->reset(); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "localhost") == 0); CHECK(result->port == 8000); @@ -431,7 +431,7 @@ SCENARIO("Testing NextHopConsistentHash same host different port markdown", "[Ne build_request(10002, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "localhost") == 0); CHECK(result->port == 8002); @@ -440,7 +440,7 @@ SCENARIO("Testing NextHopConsistentHash same host different port markdown", "[Ne build_request(10003, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "localhost") == 0); CHECK(result->port == 8004); br_destroy(sm); @@ -492,7 +492,7 @@ SCENARIO("Testing NextHopConsistentHash hash_string override", "[NextHopConsiste // We happen to know that 'foo.test' will be first if the hostname is the hash // and foo.test will be first for the hash 'first' and the bar.test hash 'second'. // So, if the hash_string override isn't getting applied, this will fail. - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "bar.test") == 0); CHECK(result->port == 80); @@ -501,6 +501,7 @@ SCENARIO("Testing NextHopConsistentHash hash_string override", "[NextHopConsiste build_request(10002, &sm, nullptr, "rabbit.net", nullptr); strategy->findNextHop(txnp); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "foo.test") == 0); CHECK(result->port == 80); br_destroy(sm); @@ -550,7 +551,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy build_request(30001, &sm, nullptr, "bunny.net/asset1", nullptr); result->reset(); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "c2.foo.com") == 0); // simulated failure, mark c2 down and retry request @@ -559,7 +560,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy // second request build_request(30002, &sm, nullptr, "bunny.net.net/asset1", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "c3.bar.com") == 0); // mark down c3.bar.com @@ -569,7 +570,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy build_request(30003, &sm, nullptr, "bunny.net/asset2", nullptr); result->reset(); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "c6.bar.com") == 0); // just mark it down and retry request @@ -577,7 +578,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy // fourth request build_request(30004, &sm, nullptr, "bunny.net/asset2", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "c1.foo.com") == 0); // mark it down @@ -586,7 +587,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy build_request(30005, &sm, nullptr, "bunny.net/asset3", nullptr); result->reset(); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "c4.bar.com") == 0); // mark it down and retry @@ -595,7 +596,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy result->reset(); build_request(30006, &sm, nullptr, "bunny.net/asset3", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "c5.bar.com") == 0); // mark it down @@ -604,7 +605,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy result->reset(); build_request(30007, &sm, nullptr, "bunny.net/asset4", nullptr); strategy->findNextHop(txnp); - CHECK(result->result == ParentResultType::PARENT_FAIL); + REQUIRE(result->result == ParentResultType::PARENT_FAIL); CHECK(result->hostname == nullptr); // eighth request - retry after waiting for the retry window to expire. @@ -612,7 +613,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy result->reset(); build_request(30008, &sm, nullptr, "bunny.net/asset4", nullptr); strategy->findNextHop(txnp, nullptr, now); - CHECK(result->result == ParentResultType::PARENT_SPECIFIED); + REQUIRE(result->result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result->hostname, "c2.foo.com") == 0); } // free up request resources.