Fixes an issue where NextHopSelectionStrategy did not implement a parent check.

proxy/http/HttpTransact.cc

@@ -243,6 +243,20 @@ markParentUp(HttpTransact::State *s)
   }
 }
 
+// wrapper to choose between a remap next hop strategy or use parent.config
+// remap next hop strategy is preferred
+inline static bool
+parentExists(HttpTransact::State *s)
+{
+  url_mapping *mp = s->url_map.getMapping();
+  if (mp && mp->strategy) {
+    return mp->strategy->nextHopExists(s->state_machine->sm_id);
+  } else if (s->parent_params) {
+    return s->parent_params->parentExists(&s->request_data);
+  }
+  return false;
+}
+
 // wrapper to choose between a remap next hop strategy or use parent.config
 // remap next hop strategy is preferred
 inline static void
@@ -1444,7 +1458,7 @@ HttpTransact::HandleRequest(State *s)
       ats_ip_copy(&s->request_data.dest_ip, &addr);
     }
 
-    if (s->parent_params->parentExists(&s->request_data)) {
+    if (parentExists(s)) {
       // If the proxy is behind and firewall and there is no
       //  DNS service available, we just want to forward the request
       //  the parent proxy.  In this case, we never find out the

proxy/http/remap/NextHopSelectionStrategy.cc

@@ -251,15 +251,15 @@ NextHopSelectionStrategy::markNextHopDown(const uint64_t sm_id, ParentResult &re
       }
       new_fail_count = old_count + 1;
     } // end of lock_guard
-    NH_Debug("parent_select", "[%" PRIu64 "] Parent fail count increased to %d for %s:%d", sm_id, new_fail_count,
-             h->hostname.c_str(), h->getPort(scheme));
+    NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] Parent fail count increased to %d for %s:%d", sm_id, new_fail_count, h->hostname.c_str(),
+             h->getPort(scheme));
   }
 
   if (new_fail_count >= fail_threshold) {
     h->set_unavailable();
     NH_Note("[%" PRIu64 "] Failure threshold met failcount:%d >= threshold:%" PRIu64 ", http parent proxy %s:%d marked down", sm_id,
             new_fail_count, fail_threshold, h->hostname.c_str(), h->getPort(scheme));
-    NH_Debug("parent_select", "[%" PRIu64 "] NextHop %s:%d marked unavailable, h->available=%s", sm_id, h->hostname.c_str(),
+    NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] NextHop %s:%d marked unavailable, h->available=%s", sm_id, h->hostname.c_str(),
              h->getPort(scheme), (h->available) ? "true" : "false");
   }
 }
@@ -289,6 +289,21 @@ NextHopSelectionStrategy::markNextHopUp(const uint64_t sm_id, ParentResult &resu
   }
 }
 
+bool
+NextHopSelectionStrategy::nextHopExists(const uint64_t sm_id)
+{
+  for (uint32_t gg = 0; gg < groups; gg++) {
+    for (uint32_t hh = 0; hh < host_groups[gg].size(); hh++) {
+      HostRecord *p = host_groups[gg][hh].get();
+      if (p->available) {
+        NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] found available next hop %s", sm_id, p->hostname.c_str());
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 namespace YAML
 {
 template <> struct convert<HostRecord> {

proxy/http/remap/NextHopSelectionStrategy.h

@@ -195,6 +195,7 @@ class NextHopSelectionStrategy
   void markNextHopDown(const uint64_t sm_id, ParentResult &result, const uint64_t fail_threshold, const uint64_t retry_time,
                        time_t now = 0);
   void markNextHopUp(const uint64_t sm_id, ParentResult &result);
+  bool nextHopExists(const uint64_t sm_id);
 
   std::string strategy_name;
   bool go_direct           = true;

proxy/http/remap/unit-tests/test_NextHopRoundRobin.cc

@@ -122,6 +122,9 @@ SCENARIO("Testing NextHopRoundRobin class, using policy 'rr-strict'", "[NextHopR
         strategy->findNextHop(10012, result, rdata, fail_threshold, retry_time);
         CHECK(result.result == ParentResultType::PARENT_DIRECT);
 
+        // check that nextHopExists() returns false when all parents are down.
+        CHECK(strategy->nextHopExists(10012) == false);
+
         // change the request time to trigger a retry.
         time_t now = (time(nullptr) + 5);
 
@@ -224,7 +227,7 @@ SCENARIO("Testing NextHopRoundRobin class, using policy 'rr-ip'", "[NextHopRound
       }
     }
 
-    WHEN("using the 'rr-stric' strategy.")
+    WHEN("using the 'rr-strict' strategy.")
     {
       uint64_t fail_threshold = 1;
       uint64_t retry_time     = 1;
@@ -235,23 +238,39 @@ SCENARIO("Testing NextHopRoundRobin class, using policy 'rr-ip'", "[NextHopRound
         REQUIRE(nhf.strategies_loaded == true);
         REQUIRE(strategy != nullptr);
 
+        // call and test parentExists(), this call should not affect
+        // findNextHop() round robin strict results
+        CHECK(strategy->nextHopExists(29000) == true);
+
         // first request.
         memcpy(&rdata.client_ip, &sa1, sizeof(sa1));
         ParentResult result;
         strategy->findNextHop(30000, result, rdata, fail_threshold, retry_time);
         CHECK(strcmp(result.hostname, "p4.foo.com") == 0);
 
+        // call and test parentExists(), this call should not affect
+        // findNextHop round robin strict results.
+        CHECK(strategy->nextHopExists(29000) == true);
+
         // second request.
         memcpy(&rdata.client_ip, &sa2, sizeof(sa2));
         result.reset();
         strategy->findNextHop(30001, result, rdata, fail_threshold, retry_time);
         CHECK(strcmp(result.hostname, "p3.foo.com") == 0);
 
+        // call and test parentExists(), this call should not affect
+        // findNextHop() round robin strict results
+        CHECK(strategy->nextHopExists(29000) == true);
+
         // third  request with same client ip, result should still be p3
         result.reset();
         strategy->findNextHop(30002, result, rdata, fail_threshold, retry_time);
         CHECK(strcmp(result.hostname, "p3.foo.com") == 0);
 
+        // call and test parentExists(), this call should not affect
+        // findNextHop() round robin strict results.
+        CHECK(strategy->nextHopExists(29000) == true);
+
         // fourth  request with same client ip and same result indicating a failure should result in p4
         // being selected.
         strategy->findNextHop(30003, result, rdata, fail_threshold, retry_time);