OSD: suspend tp timeout while taking pg lock in OpWQ

If N op_tp threads are configured, and recovery_max_active
is set to a sufficiently large number, all N op_tp threads
might grab a MOSDPGPush op off of the queue for the same PG.
The last thread to get the lock will have waited
N*time_to_handle_push before completing its item and pinging
the heartbeat timeout.  If that time exceeds the timeout
and there are enough ops waiting, each thread subsequently
will end up exceeding the timeout before completeing an
item preventing the OSD from heartbeating indefinitely.

We prevent this by suspending the timeout while we try to
get the PG lock.  Even if we do block for an excessive
period of time attempting to get the lock, hopefully,
the thread holding the lock will cause the threadpool
to time out.

Signed-off-by: Samuel Just <sam.just@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>

diff --git a/src/common/WorkQueue.cc b/src/common/WorkQueue.cc
index a57c07820309d7bed71c017e3b0b78abd1bdec53..6b648a780215f12d303e812e4aea9ff358e2049a 100644 (file)
--- a/src/common/WorkQueue.cc
+++ b/src/common/WorkQueue.cc
@@ -49,6 +49,11 @@ ThreadPool::ThreadPool(CephContext *cct_, string nm, int n, const char *option)
   }
 }
 
+void ThreadPool::TPHandle::suspend_tp_timeout()
+{
+  cct->get_heartbeat_map()->clear_timeout(hb);
+}
+
 void ThreadPool::TPHandle::reset_tp_timeout()
 {
   cct->get_heartbeat_map()->reset_timeout(

diff --git a/src/common/WorkQueue.h b/src/common/WorkQueue.h
index d936d77abef4a761991042982781eb1a55c9411d..b2742accdce08e31d7b395973bfeacd0cdf3a641 100644 (file)
--- a/src/common/WorkQueue.h
+++ b/src/common/WorkQueue.h
@@ -49,6 +49,7 @@ public:
       : cct(cct), hb(hb), grace(grace), suicide_grace(suicide_grace) {}
   public:
     void reset_tp_timeout();
+    void suspend_tp_timeout();
   };
 private:
 

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index a71c1e9af217e11183cc4fd0723638dfd6b54ddc..89aa1db34eb1e0c0aab9af53793c4d5749c50da1 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -7050,9 +7050,9 @@ PGRef OSD::OpWQ::_dequeue()
   return pg;
 }
 
-void OSD::OpWQ::_process(PGRef pg)
+void OSD::OpWQ::_process(PGRef pg, ThreadPool::TPHandle &handle)
 {
-  pg->lock();
+  pg->lock_suspend_timeout(handle);
   OpRequestRef op;
   {
     Mutex::Locker l(qlock);

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index 5bcff7442d7a62d628808a7405910a9ca2b12c72..478f766d1454213fbe2f7e8b49fdbfed253457ec 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -911,7 +911,7 @@ private:
     bool _empty() {
       return pqueue.empty();
     }
-    void _process(PGRef pg);
+    void _process(PGRef pg, ThreadPool::TPHandle &handle);
   } op_wq;
 
   void enqueue_op(PG *pg, OpRequestRef op);

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 9f957b8e05444806be8342b6d73a3705099771e1..f731441e8a45f3952de984970ba832c8f9fb06ea 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -193,6 +193,13 @@ PG::~PG()
 #endif
 }
 
+void PG::lock_suspend_timeout(ThreadPool::TPHandle &handle)
+{
+  handle.suspend_tp_timeout();
+  lock();
+  handle.reset_tp_timeout();
+}
+
 void PG::lock(bool no_lockdep)
 {
   _lock.Lock(no_lockdep);

diff --git a/src/osd/PG.h b/src/osd/PG.h
index 10e9a2544a9b0951e940d1605a64589859710f4f..8f572c75e19691b911e7bedb9ce117fda4cd4211 100644 (file)
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -245,6 +245,8 @@ protected:
 public:
   bool deleting;  // true while in removing or OSD is shutting down
 
+
+  void lock_suspend_timeout(ThreadPool::TPHandle &handle);
   void lock(bool no_lockdep = false);
   void unlock() {
     //generic_dout(0) << this << " " << info.pgid << " unlock" << dendl;