From 5be40f35c3365265b76a12a3bd0e17c3f8761bb9 Mon Sep 17 00:00:00 2001
From: "Yan, Zheng" <zyan@redhat.com>
Date: Thu, 26 Apr 2018 15:50:39 +0800
Subject: [PATCH] mds: properly trim log segments after scrub repairs something

Before calling MDLog::trim_all(), we need to make sure mdlog get flushed
Otherwise, there can be zero expiring log segments. This can cause
MDSGatherBuilder to malfunction. (MDSGatherBuilder::activate() does not
execute empty MDSGatherBuilder's finish context)

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
http://tracker.ceph.com/issues/23880
---
 src/mds/MDCache.cc | 50 +++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index 6803eeadee2ff..7ce2ec02f4b0e 100644
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -12308,36 +12308,40 @@ void MDCache::enqueue_scrub_work(MDRequestRef& mdr)
   // If the scrub did some repair, then flush the journal at the end of
   // the scrub.  Otherwise in the case of e.g. rewriting a backtrace
   // the on disk state will still look damaged.
-  auto expiry_fin = new FunctionContext([this, header, fin](int r){
-      if (header->get_repaired()) {
-        dout(4) << "Flushing journal because scrub did some repairs" << dendl;
-        mds->mdlog->start_new_segment();
-        mds->mdlog->trim_all();
-        if (fin) {
-          MDSGatherBuilder expiry_gather(g_ceph_context);
-          const std::set<LogSegment*> &expiring_segments = mds->mdlog->get_expiring_segments();
-          for (std::set<LogSegment*>::const_iterator i = expiring_segments.begin();
-               i != expiring_segments.end(); ++i) {
-            (*i)->wait_for_expiry(expiry_gather.new_sub());
-          }
-          expiry_gather.set_finisher(new MDSInternalContextWrapper(mds, fin));
-          expiry_gather.activate();
-        }
-      } else {
-        if (fin) {
-          fin->complete(r);
-        }
+  auto scrub_finish = new FunctionContext([this, header, fin](int r){
+    if (!header->get_repaired()) {
+      if (fin)
+        fin->complete(r);
+      return;
+    }
+
+    auto flush_finish = new FunctionContext([this, fin](int r){
+      dout(4) << "Expiring log segments because scrub did some repairs" << dendl;
+      mds->mdlog->trim_all();
+
+      if (fin) {
+	MDSGatherBuilder gather(g_ceph_context);
+	auto& expiring_segments = mds->mdlog->get_expiring_segments();
+	for (auto logseg : expiring_segments)
+	  logseg->wait_for_expiry(gather.new_sub());
+	assert(gather.has_subs());
+	gather.set_finisher(new MDSInternalContextWrapper(mds, fin));
+	gather.activate();
       }
+    });
+
+    dout(4) << "Flushing journal because scrub did some repairs" << dendl;
+    mds->mdlog->start_new_segment();
+    mds->mdlog->flush();
+    mds->mdlog->wait_for_safe(new MDSInternalContextWrapper(mds, flush_finish));
   });
 
   if (!header->get_recursive()) {
     mds->scrubstack->enqueue_inode_top(in, header,
-				       new MDSInternalContextWrapper(mds,
-                                         expiry_fin));
+				       new MDSInternalContextWrapper(mds, scrub_finish));
   } else {
     mds->scrubstack->enqueue_inode_bottom(in, header, 
-				       new MDSInternalContextWrapper(mds,
-                                         expiry_fin));
+				       new MDSInternalContextWrapper(mds, scrub_finish));
   }
 
   mds->server->respond_to_request(mdr, 0);
-- 
2.39.5