From fd2e1f3974512995e2cefa17eb1c2d5227ceac2b Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@redhat.com>
Date: Mon, 29 Oct 2018 11:17:26 -0500
Subject: [PATCH] os/filestore: fix merge_collection replay guards

The guards are based on spos, which does not change across the call to
_merge_collection, which means we can't open and close multiple guards
within this call.  Instead, use a single set of set_guards and closes
surrounding both the collection and temp.

Also, remove the conditional guard around _remove_collection, which we
want to (try to) do (again) as long as we didn't complete our global
replay guard.

Fixes: http://tracker.ceph.com/issues/36610
Signed-off-by: Sage Weil <sage@redhat.com>
---
 src/os/filestore/FileStore.cc | 51 +++++++++++++----------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc
index d1cf1a5aceb..46a27b05875 100644
--- a/src/os/filestore/FileStore.cc
+++ b/src/os/filestore/FileStore.cc
@@ -5790,19 +5790,20 @@ int FileStore::_merge_collection(const coll_t& cid,
   bool is_pg = dest.is_pg(&pgid);
   ceph_assert(is_pg);
 
+  int dstcmp = _check_replay_guard(dest, spos);
+  if (dstcmp < 0)
+    return 0;
+
+  int srccmp = _check_replay_guard(cid, spos);
+  if (srccmp < 0)
+    return 0;
+
+  _set_global_replay_guard(cid, spos);
+  _set_replay_guard(cid, spos, true);
+  _set_replay_guard(dest, spos, true);
+
+  // main collection
   {
-    int dstcmp = _check_replay_guard(dest, spos);
-    if (dstcmp < 0)
-      return 0;
-
-    int srccmp = _check_replay_guard(cid, spos);
-    if (srccmp < 0)
-      return 0;
-
-    _set_global_replay_guard(cid, spos);
-    _set_replay_guard(cid, spos, true);
-    _set_replay_guard(dest, spos, true);
-
     Index from;
     r = get_index(cid, &from);
 
@@ -5819,25 +5820,10 @@ int FileStore::_merge_collection(const coll_t& cid,
 
       r = from->merge(bits, to.index);
     }
-
-    _close_replay_guard(cid, spos);
-    _close_replay_guard(dest, spos);
   }
 
   // temp too
   {
-    int dstcmp = _check_replay_guard(dest.get_temp(), spos);
-    if (dstcmp < 0)
-      return 0;
-
-    int srccmp = _check_replay_guard(cid.get_temp(), spos);
-    if (srccmp < 0)
-      return 0;
-
-    _set_global_replay_guard(cid.get_temp(), spos);
-    _set_replay_guard(cid.get_temp(), spos, true);
-    _set_replay_guard(dest.get_temp(), spos, true);
-
     Index from;
     r = get_index(cid.get_temp(), &from);
 
@@ -5854,15 +5840,14 @@ int FileStore::_merge_collection(const coll_t& cid,
 
       r = from->merge(bits, to.index);
     }
-
-    _close_replay_guard(cid.get_temp(), spos);
-    _close_replay_guard(dest.get_temp(), spos);
-
   }
 
   // remove source
-  if (_check_replay_guard(cid, spos) > 0)
-    r = _destroy_collection(cid);
+  _destroy_collection(cid);
+
+  _close_replay_guard(dest, spos);
+  _close_replay_guard(dest.get_temp(), spos);
+  // no need to close guards on cid... it's removed.
 
   if (!r && cct->_conf->filestore_debug_verify_split) {
     vector<ghobject_t> objects;