From 3ae56ab82b69929a22251f7686b60ef1ebf471d3 Mon Sep 17 00:00:00 2001
From: "Yan, Zheng" <zyan@redhat.com>
Date: Wed, 6 Mar 2019 16:08:05 +0800
Subject: [PATCH] mds: avoid calling clientreplay_done() prematurely

maybe_clientreplay_done() does not correctly handle the case that
replayed request is in the finished_queue (hasn't been dispatched)

Fixes: https://tracker.ceph.com/issues/38597
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
---
 src/mds/MDSRank.cc | 36 +++++++++++++++++++-----------------
 src/mds/MDSRank.h  |  2 ++
 src/mds/Server.cc  |  3 +--
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc
index be90b70f57aeb..8b622cab0f971 100644
--- a/src/mds/MDSRank.cc
+++ b/src/mds/MDSRank.cc
@@ -762,8 +762,6 @@ void MDSRankDispatcher::tick()
 
   if (is_reconnect())
     server->reconnect_tick();
-  if (is_clientreplay())
-    maybe_clientreplay_done();
 
   if (is_active()) {
     balancer->tick();
@@ -1931,10 +1929,12 @@ void MDSRank::rejoin_done()
     return;
   }
 
-  if (replay_queue.empty() && !server->get_num_pending_reclaim())
+  if (replay_queue.empty() && !server->get_num_pending_reclaim()) {
     request_state(MDSMap::STATE_ACTIVE);
-  else
+  } else {
+    replaying_requests_done = replay_queue.empty();
     request_state(MDSMap::STATE_CLIENTREPLAY);
+  }
 }
 
 void MDSRank::clientreplay_start()
@@ -1952,25 +1952,27 @@ bool MDSRank::queue_one_replay()
     replay_queue.pop_front();
     return true;
   }
-  // don't go to active if there are session waiting for being reclaimed
-  if (!server->get_num_pending_reclaim())
-    mdlog->wait_for_safe(new C_MDS_VoidFn(this, &MDSRank::clientreplay_done));
+  if (!replaying_requests_done) {
+    replaying_requests_done = true;
+    mdlog->flush();
+  }
+  maybe_clientreplay_done();
   return false;
 }
 
 void MDSRank::maybe_clientreplay_done()
 {
-  if (is_clientreplay() &&
-      get_want_state() == MDSMap::STATE_CLIENTREPLAY &&
-      replay_queue.empty()) {
-    int num_requests = mdcache->get_num_client_requests();
-    int num_reclaim = server->get_num_pending_reclaim();
-    if (!num_requests && !num_reclaim) {
-      clientreplay_done();
-    } else {
-      dout(1) << " still have " << num_requests << " active replay requests, "
-	      << num_reclaim << " sessions need to be reclaimed" << dendl;
+  if (is_clientreplay() && get_want_state() == MDSMap::STATE_CLIENTREPLAY) {
+
+    // don't go to active if there are session waiting for being reclaimed
+    if (replaying_requests_done && !server->get_num_pending_reclaim()) {
+      mdlog->wait_for_safe(new C_MDS_VoidFn(this, &MDSRank::clientreplay_done));
+      return;
     }
+
+    dout(1) << " still have " << replay_queue.size() + (int)!replaying_requests_done
+	    << " requests need to be replayed, " << server->get_num_pending_reclaim()
+	    << " sessions need to be reclaimed" << dendl;
   }
 }
 
diff --git a/src/mds/MDSRank.h b/src/mds/MDSRank.h
index 1bd598b886311..f53b764808ce9 100644
--- a/src/mds/MDSRank.h
+++ b/src/mds/MDSRank.h
@@ -283,6 +283,8 @@ class MDSRank {
 				waiting_for_reconnect, waiting_for_resolve;
     MDSContext::vec waiting_for_any_client_connection;
     MDSContext::que replay_queue;
+    bool replaying_requests_done = false;
+
     map<mds_rank_t, MDSContext::vec > waiting_for_active_peer;
     map<epoch_t, MDSContext::vec > waiting_for_mdsmap;
 
diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index f4fa5b3fcbd9d..92a9fdd9eb3c9 100644
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -1686,8 +1686,7 @@ void Server::journal_and_reply(MDRequestRef& mdr, CInode *in, CDentry *dn, LogEv
     if (mds->queue_one_replay()) {
       dout(10) << " queued next replay op" << dendl;
     } else {
-      dout(10) << " journaled last replay op, flushing" << dendl;
-      mdlog->flush();
+      dout(10) << " journaled last replay op" << dendl;
     }
   } else if (mdr->did_early_reply)
     mds->locker->drop_rdlocks_for_early_reply(mdr.get());
-- 
2.39.5