From: Sage Weil <sage@inktank.com>
Date: Fri, 7 Jun 2013 04:38:56 +0000 (-0700)
Subject: mds: do not double-queue file recovery in eval_gather
X-Git-Tag: v0.65~130^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F352%2Fhead;p=ceph.git

mds: do not double-queue file recovery in eval_gather

This fixes a specific case of double-queuing seen in #4832:

 - client goes stale, inode marked NEEDSRECOVER
 - eval does sync, queued, -> RECOVERING
 - client resumes
 - client goes stale (again), inode marked NEEDSRECOVER
 - eval_gather queues *again*

Note that a cursory look at the recovery code makes me think this needs
a much more serious overhaul.  In particular, I don't think we should
be triggering recovery when transitioning *from* a stable state, but
explicitly when we are flagged, or when gathering.  We should probably
also hold a wrlock over the recovery period and remove the force_wrlock
kludge from the final size check.  Opened ticket #5268.

Signed-off-by: Sage Weil <sage@inktank.com>
---

diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index 74a766256b4d..951ba764ca6e 100644
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -653,15 +653,14 @@ void Locker::eval_gather(SimpleLock *lock, bool first, bool *pneed_issue, list<C
 
     if (lock->get_sm() == &sm_filelock) {
       assert(in);
-      if (in->state_test(CInode::STATE_NEEDSRECOVER)) {
+      if (in->state_test(CInode::STATE_RECOVERING)) {
+	dout(7) << "eval_gather finished gather, but still recovering" << dendl;
+      } else if (in->state_test(CInode::STATE_NEEDSRECOVER)) {
 	dout(7) << "eval_gather finished gather, but need to recover" << dendl;
 	mds->mdcache->queue_file_recover(in);
 	mds->mdcache->do_file_recover();
       }
-      if (in->state_test(CInode::STATE_RECOVERING)) {
-	dout(7) << "eval_gather finished gather, but still recovering" << dendl;
-	return;
-      }
+      return;
     }
 
     if (!lock->get_parent()->is_auth()) {