You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jr...@apache.org on 2020/12/18 14:50:20 UTC

[trafficserver] branch master updated: This PR aims to address some of the lock contention found and (#7377)

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 3cc66a8  This PR aims to address some of the lock contention found and (#7377)
3cc66a8 is described below

commit 3cc66a88481b44f076b3c861bc32f51342750559
Author: John J. Rushford <jr...@apache.org>
AuthorDate: Fri Dec 18 07:50:06 2020 -0700

    This PR aims to address some of the lock contention found and (#7377)
    
    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 568fba9..edd929d 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 ea43722..00ccb76 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);
   {