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 540a628 Issue 1597: refactor ParentSelection to remove duplicated
markParentDown() and markParentUp() code.
540a628 is described below
commit 540a628f8c997ae0871ff337b15d04c91df524b7
Author: jrushford <jrushf...@apache.org>
AuthorDate: Mon Sep 18 17:03:04 2017 +0000
Issue 1597: refactor ParentSelection to remove duplicated markParentDown()
and markParentUp() code.
---
iocore/net/Makefile.am | 4 +-
proxy/Makefile.am | 1 +
proxy/ParentConsistentHash.cc | 73 ---------------------
proxy/ParentConsistentHash.h | 5 ++
proxy/ParentRoundRobin.cc | 136 +++++----------------------------------
proxy/ParentRoundRobin.h | 9 ++-
proxy/ParentSelection.h | 37 +++++------
proxy/ParentSelectionStrategy.cc | 127 ++++++++++++++++++++++++++++++++++++
8 files changed, 174 insertions(+), 218 deletions(-)
diff --git a/iocore/net/Makefile.am b/iocore/net/Makefile.am
index aff1cf5..1f28b06 100644
--- a/iocore/net/Makefile.am
+++ b/iocore/net/Makefile.am
@@ -45,7 +45,8 @@ test_certlookup_SOURCES = \
test_certlookup_LDADD = \
@OPENSSL_LIBS@ \
$(top_builddir)/lib/ts/libtsutil.la \
- $(top_builddir)/iocore/eventsystem/libinkevent.a
+ $(top_builddir)/iocore/eventsystem/libinkevent.a \
+ $(top_builddir)/proxy/ParentSelectionStrategy.o
test_UDPNet_CPPFLAGS = \
$(AM_CPPFLAGS) \
@@ -68,6 +69,7 @@ test_UDPNet_LDADD = \
$(top_builddir)/mgmt/libmgmt_p.la \
$(top_builddir)/lib/records/librecords_p.a \
$(top_builddir)/lib/ts/libtsutil.la \
+ $(top_builddir)/proxy/ParentSelectionStrategy.o \
@LIBTCL@ @HWLOC_LIBS@ @OPENSSL_LIBS@
test_UDPNet_SOURCES = \
diff --git a/proxy/Makefile.am b/proxy/Makefile.am
index 0b5e394..e031613 100644
--- a/proxy/Makefile.am
+++ b/proxy/Makefile.am
@@ -148,6 +148,7 @@ traffic_server_SOURCES = \
ParentConsistentHash.h \
ParentRoundRobin.cc \
ParentRoundRobin.h \
+ ParentSelectionStrategy.cc \
ParentSelection.cc \
ParentSelection.h \
Plugin.cc \
diff --git a/proxy/ParentConsistentHash.cc b/proxy/ParentConsistentHash.cc
index 75ccb26..d2d988d 100644
--- a/proxy/ParentConsistentHash.cc
+++ b/proxy/ParentConsistentHash.cc
@@ -243,79 +243,6 @@ ParentConsistentHash::selectParent(bool first_call,
ParentResult *result, Reques
return;
}
-void
-ParentConsistentHash::markParentDown(ParentResult *result, unsigned int
fail_threshold, unsigned int retry_time)
-{
- time_t now;
- pRecord *pRec;
- int new_fail_count = 0;
-
- Debug("parent_select", "Starting ParentConsistentHash::markParentDown()");
-
- // Make sure that we are being called back with with a
- // result structure with a parent
- ink_assert(result->result == PARENT_SPECIFIED);
- if (result->result != PARENT_SPECIFIED) {
- return;
- }
- // If we were set through the API we currently have not failover
- // so just return fail
- if (result->is_api_result()) {
- return;
- }
-
- ink_assert((result->last_parent) < numParents(result));
- pRec = parents[result->last_lookup] + result->last_parent;
-
- // If the parent has already been marked down, just increment
- // the failure count. If this is the first mark down on a
- // parent we need to both set the failure time and set
- // count to one. It's possible for the count and time get out
- // sync due there being no locks. Therefore the code should
- // handle this condition. If this was the result of a retry, we
- // must update move the failedAt timestamp to now so that we continue
- // negative cache the parent
- if (pRec->failedAt == 0 || result->retry == true) {
- // Reread the current time. We want this to be accurate since
- // it relates to how long the parent has been down.
- now = time(nullptr);
-
- // Mark the parent failure time.
- ink_atomic_swap(&pRec->failedAt, now);
-
- // If this is clean mark down and not a failed retry, we
- // must set the count to reflect this
- if (result->retry == false) {
- new_fail_count = pRec->failCount = 1;
- }
-
- Note("Parent %s marked as down %s:%d", (result->retry) ? "retry" :
"initially", pRec->hostname, pRec->port);
-
- } else {
- int old_count = 0;
- now = time(nullptr);
-
- // if the last failure was outside the retry window, set the failcount to 1
- // and failedAt to now.
- if ((pRec->failedAt + retry_time) < now) {
- ink_atomic_swap(&pRec->failCount, 1);
- ink_atomic_swap(&pRec->failedAt, now);
- } else {
- old_count = ink_atomic_increment(&pRec->failCount, 1);
- }
-
- Debug("parent_select", "Parent fail count increased to %d for %s:%d",
old_count + 1, pRec->hostname, pRec->port);
- new_fail_count = old_count + 1;
- }
-
- if (new_fail_count > 0 && new_fail_count >=
static_cast<int>(fail_threshold)) {
- Note("Failure threshold met failcount:%d >= threshold:%d, http parent
proxy %s:%d marked down", new_fail_count, fail_threshold,
- pRec->hostname, pRec->port);
- ink_atomic_swap(&pRec->available, false);
- Debug("parent_select", "Parent %s:%d marked unavailable,
pRec->available=%d", pRec->hostname, pRec->port, pRec->available);
- }
-}
-
uint32_t
ParentConsistentHash::numParents(ParentResult *result) const
{
diff --git a/proxy/ParentConsistentHash.h b/proxy/ParentConsistentHash.h
index 567599f..607a7e4 100644
--- a/proxy/ParentConsistentHash.h
+++ b/proxy/ParentConsistentHash.h
@@ -52,6 +52,11 @@ public:
static const int SECONDARY = 1;
ParentConsistentHash(ParentRecord *_parent_record);
~ParentConsistentHash();
+ pRecord *
+ getParents(ParentResult *result)
+ {
+ return parents[result->last_lookup];
+ }
uint64_t getPathHash(HttpRequestData *hrdata, ATSHash64 *h);
void selectParent(bool firstCall, ParentResult *result, RequestData *rdata,
unsigned int fail_threshold, unsigned int retry_time);
void markParentDown(ParentResult *result, unsigned int fail_threshold,
unsigned int retry_time);
diff --git a/proxy/ParentRoundRobin.cc b/proxy/ParentRoundRobin.cc
index 377427c..7b2ba6e 100644
--- a/proxy/ParentRoundRobin.cc
+++ b/proxy/ParentRoundRobin.cc
@@ -26,6 +26,8 @@ ParentRoundRobin::ParentRoundRobin(ParentRecord
*parent_record, ParentRR_t _roun
{
round_robin_type = _round_robin_type;
latched_parent = 0;
+ parents = parent_record->parents;
+ num_parents = parent_record->num_parents;
if (is_debug_tag_set("parent_select")) {
switch (round_robin_type) {
@@ -64,10 +66,10 @@ ParentRoundRobin::selectParent(bool first_call,
ParentResult *result, RequestDat
HttpRequestData *request_info = static_cast<HttpRequestData *>(rdata);
- ink_assert(numParents(result) > 0 || result->rec->go_direct == true);
+ ink_assert(num_parents > 0 || result->rec->go_direct == true);
if (first_call) {
- if (result->rec->parents == nullptr) {
+ if (parents == nullptr) {
// We should only get into this state if
// if we are supposed to go direct
ink_assert(result->rec->go_direct == true);
@@ -90,14 +92,14 @@ ParentRoundRobin::selectParent(bool first_call,
ParentResult *result, RequestDat
// preserved for now anyway as ats_ip_hash returns the 32-bit address
in
// that case.
if (rdata->get_client_ip() != nullptr) {
- cur_index = result->start_parent =
ntohl(ats_ip_hash(rdata->get_client_ip())) % result->rec->num_parents;
+ cur_index = result->start_parent =
ntohl(ats_ip_hash(rdata->get_client_ip())) % num_parents;
} else {
cur_index = 0;
}
break;
case P_STRICT_ROUND_ROBIN:
cur_index = ink_atomic_increment((int32_t *)&result->rec->rr_next, 1);
- cur_index = result->start_parent = cur_index %
result->rec->num_parents;
+ cur_index = result->start_parent = cur_index % num_parents;
break;
case P_NO_ROUND_ROBIN:
cur_index = result->start_parent = 0;
@@ -111,7 +113,7 @@ ParentRoundRobin::selectParent(bool first_call,
ParentResult *result, RequestDat
}
} else {
// Move to next parent due to failure
- latched_parent = cur_index = (result->last_parent + 1) %
result->rec->num_parents;
+ latched_parent = cur_index = (result->last_parent + 1) % num_parents;
// Check to see if we have wrapped around
if ((unsigned int)cur_index == result->start_parent) {
@@ -134,22 +136,19 @@ ParentRoundRobin::selectParent(bool first_call,
ParentResult *result, RequestDat
do {
Debug("parent_select", "cur_index: %d, result->start_parent: %d",
cur_index, result->start_parent);
// DNS ParentOnly inhibits bypassing the parent so always return that t
- if ((result->rec->parents[cur_index].failedAt == 0) ||
- (result->rec->parents[cur_index].failCount <
static_cast<int>(fail_threshold))) {
+ if ((parents[cur_index].failedAt == 0) || (parents[cur_index].failCount <
static_cast<int>(fail_threshold))) {
Debug("parent_select", "FailThreshold = %d", fail_threshold);
Debug("parent_select", "Selecting a parent due to little failCount
(faileAt: %u failCount: %d)",
- (unsigned)result->rec->parents[cur_index].failedAt,
result->rec->parents[cur_index].failCount);
+ (unsigned)parents[cur_index].failedAt,
parents[cur_index].failCount);
parentUp = true;
} else {
- if ((result->wrap_around) || ((result->rec->parents[cur_index].failedAt
+ retry_time) < request_info->xact_start)) {
+ if ((result->wrap_around) || ((parents[cur_index].failedAt + retry_time)
< request_info->xact_start)) {
Debug("parent_select", "Parent[%d].failedAt = %u, retry =
%u,xact_start = %" PRId64 " but wrap = %d", cur_index,
- (unsigned)result->rec->parents[cur_index].failedAt, retry_time,
(int64_t)request_info->xact_start,
- result->wrap_around);
+ (unsigned)parents[cur_index].failedAt, retry_time,
(int64_t)request_info->xact_start, result->wrap_around);
// Reuse the parent
parentUp = true;
parentRetry = true;
- Debug("parent_select", "Parent marked for retry %s:%d",
result->rec->parents[cur_index].hostname,
- result->rec->parents[cur_index].port);
+ Debug("parent_select", "Parent marked for retry %s:%d",
parents[cur_index].hostname, parents[cur_index].port);
} else {
parentUp = false;
}
@@ -157,8 +156,8 @@ ParentRoundRobin::selectParent(bool first_call,
ParentResult *result, RequestDat
if (parentUp == true) {
result->result = PARENT_SPECIFIED;
- result->hostname = result->rec->parents[cur_index].hostname;
- result->port = result->rec->parents[cur_index].port;
+ result->hostname = parents[cur_index].hostname;
+ result->port = parents[cur_index].port;
result->last_parent = cur_index;
result->retry = parentRetry;
ink_assert(result->hostname != nullptr);
@@ -166,7 +165,7 @@ ParentRoundRobin::selectParent(bool first_call,
ParentResult *result, RequestDat
Debug("parent_select", "Chosen parent = %s.%d", result->hostname,
result->port);
return;
}
- latched_parent = cur_index = (cur_index + 1) % result->rec->num_parents;
+ latched_parent = cur_index = (cur_index + 1) % num_parents;
} while ((unsigned int)cur_index != result->start_parent);
if (result->rec->go_direct == true && result->rec->parent_is_proxy == true) {
@@ -182,108 +181,5 @@ ParentRoundRobin::selectParent(bool first_call,
ParentResult *result, RequestDat
uint32_t
ParentRoundRobin::numParents(ParentResult *result) const
{
- return result->rec->num_parents;
-}
-
-void
-ParentRoundRobin::markParentDown(ParentResult *result, unsigned int
fail_threshold, unsigned int retry_time)
-{
- time_t now;
- pRecord *pRec;
- int new_fail_count = 0;
-
- Debug("parent_select", "Starting ParentRoundRobin::markParentDown()");
- // Make sure that we are being called back with with a
- // result structure with a parent
- ink_assert(result->result == PARENT_SPECIFIED);
- if (result->result != PARENT_SPECIFIED) {
- return;
- }
- // If we were set through the API we currently have not failover
- // so just return fail
- if (result->is_api_result()) {
- return;
- }
-
- ink_assert((int)(result->last_parent) < result->rec->num_parents);
- pRec = result->rec->parents + result->last_parent;
-
- // If the parent has already been marked down, just increment
- // the failure count. If this is the first mark down on a
- // parent we need to both set the failure time and set
- // count to one. It's possible for the count and time get out
- // sync due there being no locks. Therefore the code should
- // handle this condition. If this was the result of a retry, we
- // must update move the failedAt timestamp to now so that we continue
- // negative cache the parent
- if (pRec->failedAt == 0 || result->retry == true) {
- // Reread the current time. We want this to be accurate since
- // it relates to how long the parent has been down.
- now = time(nullptr);
-
- // Mark the parent failure time.
- ink_atomic_swap(&pRec->failedAt, now);
-
- // If this is clean mark down and not a failed retry, we
- // must set the count to reflect this
- if (result->retry == false) {
- new_fail_count = pRec->failCount = 1;
- }
-
- Note("Parent %s marked as down %s:%d", (result->retry) ? "retry" :
"initially", pRec->hostname, pRec->port);
-
- } else {
- int old_count = 0;
- now = time(nullptr);
-
- // if the last failure was outside the retry window, set the failcount to 1
- // and failedAt to now.
- if ((pRec->failedAt + retry_time) < now) {
- ink_atomic_swap(&pRec->failCount, 1);
- ink_atomic_swap(&pRec->failedAt, now);
- } else {
- old_count = ink_atomic_increment(&pRec->failCount, 1);
- }
-
- Debug("parent_select", "Parent fail count increased to %d for %s:%d",
old_count + 1, pRec->hostname, pRec->port);
- new_fail_count = old_count + 1;
- }
-
- if (new_fail_count > 0 && new_fail_count >=
static_cast<int>(fail_threshold)) {
- Note("Failure threshold met failcount:%d >= threshold:%d, http parent
proxy %s:%d marked down", new_fail_count, fail_threshold,
- pRec->hostname, pRec->port);
- ink_atomic_swap(&pRec->available, false);
- Debug("parent_select", "Parent marked unavailable, pRec->available=%d",
pRec->available);
- }
-}
-
-void
-ParentRoundRobin::markParentUp(ParentResult *result)
-{
- pRecord *pRec;
-
- // Make sure that we are being called back with with a
- // result structure with a parent that is being retried
- ink_release_assert(result->retry == true);
- ink_assert(result->result == PARENT_SPECIFIED);
- if (result->result != PARENT_SPECIFIED) {
- return;
- }
- // If we were set through the API we currently have not failover
- // so just return fail
- if (result->is_api_result()) {
- ink_assert(0);
- return;
- }
-
- ink_assert((int)(result->last_parent) < result->rec->num_parents);
- pRec = result->rec->parents + result->last_parent;
- ink_atomic_swap(&pRec->available, true);
-
- ink_atomic_swap(&pRec->failedAt, (time_t)0);
- int old_count = ink_atomic_swap(&pRec->failCount, 0);
-
- if (old_count > 0) {
- Note("http parent proxy %s:%d restored", pRec->hostname, pRec->port);
- }
+ return num_parents;
}
diff --git a/proxy/ParentRoundRobin.h b/proxy/ParentRoundRobin.h
index b7995ec..3175940 100644
--- a/proxy/ParentRoundRobin.h
+++ b/proxy/ParentRoundRobin.h
@@ -36,14 +36,19 @@ class ParentRoundRobin : public ParentSelectionStrategy
{
ParentRR_t round_robin_type;
int latched_parent;
+ pRecord *parents;
+ int num_parents;
public:
ParentRoundRobin(ParentRecord *_parent_record, ParentRR_t _round_robin_type);
~ParentRoundRobin();
+ pRecord *
+ getParents(ParentResult *result)
+ {
+ return parents;
+ }
void selectParent(bool firstCall, ParentResult *result, RequestData *rdata,
unsigned int fail_threshold, unsigned int retry_time);
- void markParentDown(ParentResult *result, unsigned int fail_threshold,
unsigned int retry_time);
uint32_t numParents(ParentResult *result) const;
- void markParentUp(ParentResult *result);
};
#endif
diff --git a/proxy/ParentSelection.h b/proxy/ParentSelection.h
index a0387a8..b1d3144 100644
--- a/proxy/ParentSelection.h
+++ b/proxy/ParentSelection.h
@@ -260,6 +260,7 @@ private:
friend class ParentRoundRobin;
friend class ParentConfigParams;
friend class ParentRecord;
+ friend class ParentSelectionStrategy;
};
struct ParentSelectionPolicy {
@@ -274,6 +275,9 @@ struct ParentSelectionPolicy {
class ParentSelectionStrategy
{
public:
+ //
+ // Return the pRecord.
+ virtual pRecord *getParents(ParentResult *result) = 0;
// void selectParent(bool firstCall, ParentResult *result, RequestData
*rdata, unsigned int fail_threshold, unsigned int
// retry_time)
//
@@ -282,24 +286,13 @@ public:
virtual void selectParent(bool firstCall, ParentResult *result, RequestData
*rdata, unsigned int fail_threshold,
unsigned int retry_time) = 0;
- // void markParentDown(ParentResult *result, unsigned int fail_threshold,
unsigned int retry_time)
- //
- // Marks the parent pointed to by result as down
- //
- virtual void markParentDown(ParentResult *result, unsigned int
fail_threshold, unsigned int retry_time) = 0;
-
// uint32_t numParents(ParentResult *result);
//
// Returns the number of parent records in a strategy.
//
virtual uint32_t numParents(ParentResult *result) const = 0;
-
- // void markParentUp
- //
- // After a successful retry, http calls this function
- // to clear the bits indicating the parent is down
- //
- virtual void markParentUp(ParentResult *result) = 0;
+ void markParentDown(ParentResult *result, unsigned int fail_threshold,
unsigned int retry_time);
+ void markParentUp(ParentResult *result);
// virtual destructor.
virtual ~ParentSelectionStrategy(){};
@@ -335,6 +328,15 @@ public:
}
}
+ void
+ markParentUp(ParentResult *result)
+ {
+ if (!result->is_api_result()) {
+ ink_release_assert(result != NULL);
+ result->rec->selection_strategy->markParentUp(result);
+ }
+ }
+
uint32_t
numParents(ParentResult *result)
{
@@ -346,15 +348,6 @@ public:
}
}
- void
- markParentUp(ParentResult *result)
- {
- if (!result->is_api_result()) {
- ink_release_assert(result != NULL);
- result->rec->selection_strategy->markParentUp(result);
- }
- }
-
P_table *parent_table;
ParentRecord *DefaultParent;
ParentSelectionPolicy policy;
diff --git a/proxy/ParentSelectionStrategy.cc b/proxy/ParentSelectionStrategy.cc
new file mode 100644
index 0000000..e7cc08b
--- /dev/null
+++ b/proxy/ParentSelectionStrategy.cc
@@ -0,0 +1,127 @@
+/** @file
+
+ Implementation of Parent Proxy routing
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+#include "ParentSelection.h"
+
+void
+ParentSelectionStrategy::markParentDown(ParentResult *result, unsigned int
fail_threshold, unsigned int retry_time)
+{
+ time_t now;
+ pRecord *pRec, *parents =
result->rec->selection_strategy->getParents(result);
+ int new_fail_count = 0;
+
+ // Make sure that we are being called back with with a
+ // result structure with a parent
+ ink_assert(result->result == PARENT_SPECIFIED);
+ if (result->result != PARENT_SPECIFIED) {
+ return;
+ }
+ // If we were set through the API we currently have not failover
+ // so just return fail
+ if (result->is_api_result()) {
+ return;
+ }
+
+ ink_assert((result->last_parent) < numParents(result));
+ pRec = (parents + result->last_parent);
+
+ // If the parent has already been marked down, just increment
+ // the failure count. If this is the first mark down on a
+ // parent we need to both set the failure time and set
+ // count to one. It's possible for the count and time get out
+ // sync due there being no locks. Therefore the code should
+ // handle this condition. If this was the result of a retry, we
+ // must update move the failedAt timestamp to now so that we continue
+ // negative cache the parent
+ if (pRec->failedAt == 0 || result->retry == true) {
+ // Reread the current time. We want this to be accurate since
+ // it relates to how long the parent has been down.
+ now = time(nullptr);
+
+ // Mark the parent failure time.
+ ink_atomic_swap(&pRec->failedAt, now);
+
+ // If this is clean mark down and not a failed retry, we
+ // must set the count to reflect this
+ if (result->retry == false) {
+ new_fail_count = pRec->failCount = 1;
+ }
+
+ Note("Parent %s marked as down %s:%d", (result->retry) ? "retry" :
"initially", pRec->hostname, pRec->port);
+
+ } else {
+ int old_count = 0;
+ now = time(nullptr);
+
+ // if the last failure was outside the retry window, set the failcount to 1
+ // and failedAt to now.
+ if ((pRec->failedAt + retry_time) < now) {
+ ink_atomic_swap(&pRec->failCount, 1);
+ ink_atomic_swap(&pRec->failedAt, now);
+ } else {
+ old_count = ink_atomic_increment(&pRec->failCount, 1);
+ }
+
+ Debug("parent_select", "Parent fail count increased to %d for %s:%d",
old_count + 1, pRec->hostname, pRec->port);
+ new_fail_count = old_count + 1;
+ }
+
+ if (new_fail_count > 0 && new_fail_count >=
static_cast<int>(fail_threshold)) {
+ Note("Failure threshold met failcount:%d >= threshold:%d, http parent
proxy %s:%d marked down", new_fail_count, fail_threshold,
+ pRec->hostname, pRec->port);
+ ink_atomic_swap(&pRec->available, false);
+ Debug("parent_select", "Parent %s:%d marked unavailable,
pRec->available=%d", pRec->hostname, pRec->port, pRec->available);
+ }
+}
+
+void
+ParentSelectionStrategy::markParentUp(ParentResult *result)
+{
+ pRecord *pRec, *parents =
result->rec->selection_strategy->getParents(result);
+ int num_parents = result->rec->selection_strategy->numParents(result);
+
+ // Make sure that we are being called back with with a
+ // result structure with a parent that is being retried
+ ink_release_assert(result->retry == true);
+ ink_assert(result->result == PARENT_SPECIFIED);
+ if (result->result != PARENT_SPECIFIED) {
+ return;
+ }
+ // If we were set through the API we currently have not failover
+ // so just return fail
+ if (result->is_api_result()) {
+ ink_assert(0);
+ return;
+ }
+
+ ink_assert((int)(result->last_parent) < num_parents);
+ pRec = parents + result->last_parent;
+ ink_atomic_swap(&pRec->available, true);
+
+ ink_atomic_swap(&pRec->failedAt, (time_t)0);
+ int old_count = ink_atomic_swap(&pRec->failCount, 0);
+
+ if (old_count > 0) {
+ Note("http parent proxy %s:%d restored", pRec->hostname, pRec->port);
+ }
+}
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].
