From 1f13d8ac5b879134942cac2f5aca00669f24581f Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Mon, 29 Jul 2013 09:36:04 -0700 Subject: [PATCH] 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 Reviewed-by: Sage Weil --- src/common/WorkQueue.cc | 5 +++++ src/common/WorkQueue.h | 1 + src/osd/OSD.cc | 4 ++-- src/osd/OSD.h | 2 +- src/osd/PG.cc | 7 +++++++ src/osd/PG.h | 2 ++ 6 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/common/WorkQueue.cc b/src/common/WorkQueue.cc index a57c07820309d..6b648a780215f 100644 --- 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 d936d77abef4a..b2742accdce08 100644 --- 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 a71c1e9af217e..89aa1db34eb1e 100644 --- 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 5bcff7442d7a6..478f766d14542 100644 --- 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 9f957b8e05444..f731441e8a45f 100644 --- 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 10e9a2544a9b0..8f572c75e1969 100644 --- 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; -- 2.39.5