From b04cbd8212f3e083d0b6809f256ac6b98b565a82 Mon Sep 17 00:00:00 2001 From: John Rushford Date: Tue, 8 Dec 2020 18:52:50 +0000 Subject: [PATCH] This PR aims to address some of the lock contention found and documented in Issue #7375. In ParentConsistentHash::selectParent() redunant calls to getHostStatus() are removed and HostStatus::getHostStatus() has been changed to eliminate locking altogether if the hosts_statuses map is empty ie, no hosts have been marked down using traffic_ctl. --- proxy/ParentConsistentHash.cc | 9 ++++----- src/traffic_server/HostStatus.cc | 9 +++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/proxy/ParentConsistentHash.cc b/proxy/ParentConsistentHash.cc index 568fba97d92..edd929d9bac 100644 --- a/proxy/ParentConsistentHash.cc +++ b/proxy/ParentConsistentHash.cc @@ -202,8 +202,6 @@ ParentConsistentHash::selectParent(bool first_call, ParentResult *result, Reques pRec = nullptr; } if (firstCall) { - HostStatRec *hst = (pRec) ? pStatus.getHostStatus(pRec->hostname) : nullptr; - result->first_choice_status = (hst) ? hst->status : HostStatus_t::HOST_STATUS_UP; break; } } while (pRec && !firstCall && last_lookup == PRIMARY && strcmp(pRec->hostname, result->hostname) == 0); @@ -217,6 +215,10 @@ ParentConsistentHash::selectParent(bool first_call, ParentResult *result, Reques // didn't find a parent or the parent is marked unavailable or the parent is marked down HostStatRec *hst = (pRec) ? pStatus.getHostStatus(pRec->hostname) : nullptr; host_stat = (hst) ? hst->status : HostStatus_t::HOST_STATUS_UP; + if (firstCall) { + result->first_choice_status = host_stat; + } + // 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 avaialble if ((pRec && result->rec->ignore_self_detect) && (hst && hst->status == HOST_STATUS_DOWN)) { @@ -313,9 +315,6 @@ ParentConsistentHash::selectParent(bool first_call, ParentResult *result, Reques // Validate and return the final result. // ---------------------------------------------------------------------------------------------------- - // use the available or marked for retry parent. - hst = (pRec) ? pStatus.getHostStatus(pRec->hostname) : nullptr; - host_stat = (hst) ? hst->status : HostStatus_t::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 avaialble if ((pRec && result->rec->ignore_self_detect) && (hst && hst->status == HOST_STATUS_DOWN)) { diff --git a/src/traffic_server/HostStatus.cc b/src/traffic_server/HostStatus.cc index ea4372209a5..00ccb7626a4 100644 --- a/src/traffic_server/HostStatus.cc +++ b/src/traffic_server/HostStatus.cc @@ -365,6 +365,15 @@ HostStatus::getHostStatus(const char *name) time_t now = time(0); bool lookup = false; + // if host_statuses is empty, just return + // a nullptr as there is no need to lock + // and search. A return of nullptr indicates + // to the caller that the host is available, + // HOST_STATUS_UP. + if (hosts_statuses.empty()) { + return _status; + } + // the hash table value pointer has the HostStatus_t value. ink_rwlock_rdlock(&host_status_rwlock); {