This is an automated email from the ASF dual-hosted git repository.
jrushford pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 504cc9f Fixes an issue where NextHopSelectionStrategy did not
implement an available nexthop or parent check when
proxy.config.http.no_dns_just_forward_to_parent is enabled.
504cc9f is described below
commit 504cc9fc924e9600f971d81231c5316b1c57e236
Author: John Rushford <[email protected]>
AuthorDate: Wed Jan 22 16:45:48 2020 +0000
Fixes an issue where NextHopSelectionStrategy did not implement an available
nexthop or parent check when proxy.config.http.no_dns_just_forward_to_parent
is enabled.
---
proxy/http/HttpTransact.cc | 16 +++++++++++++++-
proxy/http/remap/NextHopSelectionStrategy.cc | 21 ++++++++++++++++++---
proxy/http/remap/NextHopSelectionStrategy.h | 1 +
.../http/remap/unit-tests/test_NextHopRoundRobin.cc | 21 ++++++++++++++++++++-
4 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 1ae438c..ffd5814 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -245,6 +245,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
nextParent(HttpTransact::State *s)
{
@@ -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
diff --git a/proxy/http/remap/NextHopSelectionStrategy.cc
b/proxy/http/remap/NextHopSelectionStrategy.cc
index a101dfe..0507350 100644
--- a/proxy/http/remap/NextHopSelectionStrategy.cc
+++ b/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> {
diff --git a/proxy/http/remap/NextHopSelectionStrategy.h
b/proxy/http/remap/NextHopSelectionStrategy.h
index 200c94c..77a8b11 100644
--- a/proxy/http/remap/NextHopSelectionStrategy.h
+++ b/proxy/http/remap/NextHopSelectionStrategy.h
@@ -195,6 +195,7 @@ public:
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;
diff --git a/proxy/http/remap/unit-tests/test_NextHopRoundRobin.cc
b/proxy/http/remap/unit-tests/test_NextHopRoundRobin.cc
index df47f3c..05c260c 100644
--- a/proxy/http/remap/unit-tests/test_NextHopRoundRobin.cc
+++ b/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);
