From ebdb3923176ac6d60a3e69736b1709c4e6c1e3f7 Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Sat, 1 Aug 2020 22:44:48 +0800
Subject: [PATCH] common/HeartbeatMap: use std::atomic<time_point> for timeout

since we now depends on GCC-7 and up for C++17 support, we can now use
std::atomic<time_point> for better readability.

Signed-off-by: Kefu Chai <kchai@redhat.com>
---
 src/common/HeartbeatMap.cc | 34 ++++++++++++++++------------------
 src/common/HeartbeatMap.h  | 22 +++++++++++++---------
 src/common/WorkQueue.cc    | 13 ++++++++-----
 src/common/WorkQueue.h     |  4 ++--
 4 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/src/common/HeartbeatMap.cc b/src/common/HeartbeatMap.cc
index aef9677d9fac3..6069b0b351d7d 100644
--- a/src/common/HeartbeatMap.cc
+++ b/src/common/HeartbeatMap.cc
@@ -67,17 +67,17 @@ void HeartbeatMap::remove_worker(const heartbeat_handle_d *h)
 }
 
 bool HeartbeatMap::_check(const heartbeat_handle_d *h, const char *who,
-			  ceph::coarse_mono_clock::rep now)
+			  ceph::coarse_mono_time now)
 {
   bool healthy = true;
-  auto was = h->timeout.load();
-  if (was && was < now) {
+  if (auto was = h->timeout.load();
+      !clock::is_zero(was) && was < now) {
     ldout(m_cct, 1) << who << " '" << h->name << "'"
 		    << " had timed out after " << h->grace << dendl;
     healthy = false;
   }
-  was = h->suicide_timeout;
-  if (was && was < now) {
+  if (auto was = h->suicide_timeout.load();
+      !clock::is_zero(was) && was < now) {
     ldout(m_cct, 1) << who << " '" << h->name << "'"
 		    << " had suicide timed out after " << h->suicide_grace << dendl;
     pthread_kill(h->thread_id, SIGABRT);
@@ -88,33 +88,32 @@ bool HeartbeatMap::_check(const heartbeat_handle_d *h, const char *who,
 }
 
 void HeartbeatMap::reset_timeout(heartbeat_handle_d *h,
-				 ceph::coarse_mono_clock::rep grace,
-				 ceph::coarse_mono_clock::rep suicide_grace)
+				 ceph::timespan grace,
+				 ceph::timespan suicide_grace)
 {
   ldout(m_cct, 20) << "reset_timeout '" << h->name << "' grace " << grace
 		   << " suicide " << suicide_grace << dendl;
-  auto now = duration_cast<seconds>(coarse_mono_clock::now()
-				    .time_since_epoch()).count();
+  const auto now = clock::now();
   _check(h, "reset_timeout", now);
 
   h->timeout = now + grace;
   h->grace = grace;
 
-  if (suicide_grace)
+  if (suicide_grace > ceph::timespan::zero()) {
     h->suicide_timeout = now + suicide_grace;
-  else
-    h->suicide_timeout = 0;
+  } else {
+    h->suicide_timeout = clock::zero();
+  }
   h->suicide_grace = suicide_grace;
 }
 
 void HeartbeatMap::clear_timeout(heartbeat_handle_d *h)
 {
   ldout(m_cct, 20) << "clear_timeout '" << h->name << "'" << dendl;
-  auto now = duration_cast<seconds>(coarse_mono_clock::now()
-				    .time_since_epoch()).count();
+  auto now = clock::now();
   _check(h, "clear_timeout", now);
-  h->timeout = 0;
-  h->suicide_timeout = 0;
+  h->timeout = clock::zero();
+  h->suicide_timeout = clock::zero();
 }
 
 bool HeartbeatMap::is_healthy()
@@ -141,8 +140,7 @@ bool HeartbeatMap::is_healthy()
        p != m_workers.end();
        ++p) {
     heartbeat_handle_d *h = *p;
-    auto epoch = duration_cast<seconds>(now.time_since_epoch()).count();
-    if (!_check(h, "is_healthy", epoch)) {
+    if (!_check(h, "is_healthy", now)) {
       healthy = false;
       unhealthy++;
     }
diff --git a/src/common/HeartbeatMap.h b/src/common/HeartbeatMap.h
index 65f8f94e97871..6f486b21ca864 100644
--- a/src/common/HeartbeatMap.h
+++ b/src/common/HeartbeatMap.h
@@ -39,14 +39,17 @@ namespace ceph {
 
 struct heartbeat_handle_d {
   const std::string name;
-  pthread_t thread_id;
-  // TODO: use atomic<time_point>, once we can ditch GCC 4.8
-  std::atomic<unsigned> timeout = { 0 }, suicide_timeout = { 0 };
-  time_t grace, suicide_grace;
+  pthread_t thread_id = 0;
+  using clock = ceph::coarse_mono_clock;
+  using time = ceph::coarse_mono_time;
+  std::atomic<time> timeout = clock::zero();
+  std::atomic<time> suicide_timeout = clock::zero();
+  ceph::timespan grace = ceph::timespan::zero();
+  ceph::timespan suicide_grace = ceph::timespan::zero();
   std::list<heartbeat_handle_d*>::iterator list_item;
 
   explicit heartbeat_handle_d(const std::string& n)
-    : name(n), thread_id(0), grace(0), suicide_grace(0)
+    : name(n)
   { }
 };
 
@@ -58,8 +61,8 @@ class HeartbeatMap {
 
   // reset the timeout so that it expects another touch within grace amount of time
   void reset_timeout(heartbeat_handle_d *h,
-		     ceph::coarse_mono_clock::rep grace,
-		     ceph::coarse_mono_clock::rep suicide_grace);
+		     ceph::timespan grace,
+		     ceph::timespan suicide_grace);
   // clear the timeout so that it's not checked on
   void clear_timeout(heartbeat_handle_d *h);
 
@@ -79,16 +82,17 @@ class HeartbeatMap {
   ~HeartbeatMap();
 
  private:
+  using clock = ceph::coarse_mono_clock;
   CephContext *m_cct;
   ceph::shared_mutex m_rwlock =
     ceph::make_shared_mutex("HeartbeatMap::m_rwlock");
-  ceph::coarse_mono_clock::time_point m_inject_unhealthy_until;
+  clock::time_point m_inject_unhealthy_until;
   std::list<heartbeat_handle_d*> m_workers;
   std::atomic<unsigned> m_unhealthy_workers = { 0 };
   std::atomic<unsigned> m_total_workers = { 0 };
 
   bool _check(const heartbeat_handle_d *h, const char *who,
-	      ceph::coarse_mono_clock::rep now);
+	      ceph::coarse_mono_time now);
 };
 
 }
diff --git a/src/common/WorkQueue.cc b/src/common/WorkQueue.cc
index c1b8c18e09729..9e68b28a76702 100644
--- a/src/common/WorkQueue.cc
+++ b/src/common/WorkQueue.cc
@@ -133,8 +133,8 @@ void ThreadPool::worker(WorkThread *wt)
     ldout(cct,20) << "worker waiting" << dendl;
     cct->get_heartbeat_map()->reset_timeout(
       hb,
-      cct->_conf->threadpool_default_timeout,
-      0);
+      ceph::make_timespan(cct->_conf->threadpool_default_timeout),
+      ceph::make_timespan(0));
     auto wait = std::chrono::seconds(
       cct->_conf->threadpool_empty_queue_max_wait);
     _cond.wait_for(ul, wait);
@@ -278,7 +278,8 @@ void ShardedThreadPool::shardedthreadpool_worker(uint32_t thread_index)
       while (pause_threads) {
        cct->get_heartbeat_map()->reset_timeout(
 	        hb,
-	        wq->timeout_interval, wq->suicide_interval);
+	        ceph::make_timespan(wq->timeout_interval),
+		ceph::make_timespan(wq->suicide_interval));
        shardedpool_cond.wait_for(
 	 ul,
 	 std::chrono::seconds(cct->_conf->threadpool_empty_queue_max_wait));
@@ -293,7 +294,8 @@ void ShardedThreadPool::shardedthreadpool_worker(uint32_t thread_index)
         while (drain_threads) {
 	  cct->get_heartbeat_map()->reset_timeout(
 	    hb,
-	    wq->timeout_interval, wq->suicide_interval);
+	    ceph::make_timespan(wq->timeout_interval),
+	    ceph::make_timespan(wq->suicide_interval));
           shardedpool_cond.wait_for(
 	    ul,
 	    std::chrono::seconds(cct->_conf->threadpool_empty_queue_max_wait));
@@ -304,7 +306,8 @@ void ShardedThreadPool::shardedthreadpool_worker(uint32_t thread_index)
 
     cct->get_heartbeat_map()->reset_timeout(
       hb,
-      wq->timeout_interval, wq->suicide_interval);
+      ceph::make_timespan(wq->timeout_interval),
+      ceph::make_timespan(wq->suicide_interval));
     wq->_process(thread_index, hb);
 
   }
diff --git a/src/common/WorkQueue.h b/src/common/WorkQueue.h
index 0c16cfc31f200..281fececc0a2d 100644
--- a/src/common/WorkQueue.h
+++ b/src/common/WorkQueue.h
@@ -59,8 +59,8 @@ public:
     friend class ThreadPool;
     CephContext *cct;
     ceph::heartbeat_handle_d *hb;
-    ceph::coarse_mono_clock::rep grace;
-    ceph::coarse_mono_clock::rep suicide_grace;
+    ceph::timespan grace;
+    ceph::timespan suicide_grace;
   public:
     TPHandle(
       CephContext *cct,
-- 
2.39.5