mds: clean up non-existent data pools in MDSMap

Older versions of Ceph weren't strict about preventing pool deletion when the
MDSMap referred to to-be-deleted pool. If we are dealing with a cluster
upgrade, we should try to gracefully handle that by cleaning out data pools
that have been removed.

Reproduced this by allowing CephFS pools to be deleted:

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 85c47c13da6..694b240cb9f 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -10962,7 +10962,7 @@ int OSDMonitor::_check_remove_pool(int64_t pool_id, const pg_pool_t& pool,
   FSMap const &pending_fsmap = mon->mdsmon()->get_pending();
   if (pending_fsmap.pool_in_use(pool_id)) {
     *ss << "pool '" << poolstr << "' is in use by CephFS";
-    return -EBUSY;
+    //return -EBUSY;
   }

   if (pool.tier_of >= 0) {

pdonnell@icewind ~/ceph/build$ bin/ceph osd pool create derp 4 4
pool 'derp' created
pdonnell@icewind ~/ceph/build$ bin/ceph fs add_data_pool cephfs_a derp
added data pool 3 to fsmap
pdonnell@icewind ~/ceph/build$ bin/ceph osd pool rm derp derp --yes-i-really-really-mean-it
pool 'derp' is in use by CephFSpool 'derp' removed
pdonnell@icewind ~/ceph/build$ bin/ceph fs ls
...
2017-10-03 12:50:48.409561 7f9e2e05b700 -1 /home/pdonnell/ceph/src/osd/OSDMap.h: In function 'const string& OSDMap::get_pool_name(int64_t) const' thread 7f9e2e05b700 time 2017-10-03 12:50:48.407897
/home/pdonnell/ceph/src/osd/OSDMap.h: 1184: FAILED assert(i != pool_name.end())

 ceph version 12.1.2-2624-g37884a41964 (37884a419640b446fffc1fa4d6074c97339fdd96) mimic (dev)
  1: (ceph::__ceph_assert_fail(char const*, char const*, int, char const*)+0xf5) [0x564ebb5420f5]
  2: (()+0x41dade) [0x564ebb3cbade]
  3: (MDSMonitor::preprocess_command(boost::intrusive_ptr<MonOpRequest>)+0x1fb9) [0x564ebb4cd119]

Note when testing this fix, use something like this after removing the data pool:

pdonnell@icewind ~/ceph/build$ bin/ceph fs set cephfs_a max_mds 2

Setting max_mds will cause a new FSMap to be created where MDSMap::sanitize is
called; this is simulating the initial load+sanitize of a Hammer legacy MDSMap
by the mons.

Fixes: http://tracker.ceph.com/issues/21568
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

diff --git a/src/mds/FSMap.cc b/src/mds/FSMap.cc
index cf72ce040ec9319f487704a73f056ea45d2c8abc..b224e11190d25c9e9edd17e8c36c1a95bfd7fb0a 100644 (file)
--- a/src/mds/FSMap.cc
+++ b/src/mds/FSMap.cc
@@ -621,6 +621,12 @@ void FSMap::decode(bufferlist::iterator& p)
   DECODE_FINISH(p);
 }
 
+void FSMap::sanitize(std::function<bool(int64_t pool)> pool_exists)
+{
+  for (auto &fs : filesystems) {
+    fs.second->mds_map.sanitize(pool_exists);
+  }
+}
 
 void Filesystem::encode(bufferlist& bl, uint64_t features) const
 {

diff --git a/src/mds/FSMap.h b/src/mds/FSMap.h
index ea102a712740c63045bd1573d59b7a2c6484b0d4..3bb97ee58aae94f737097844671a3e71cecd7286 100644 (file)
--- a/src/mds/FSMap.h
+++ b/src/mds/FSMap.h
@@ -493,6 +493,7 @@ public:
     bufferlist::iterator p = bl.begin();
     decode(p);
   }
+  void sanitize(std::function<bool(int64_t pool)> pool_exists);
 
   void print(ostream& out) const;
   void print_summary(Formatter *f, ostream *out) const;

diff --git a/src/mds/MDSMap.cc b/src/mds/MDSMap.cc
index 1d38f19f498e663b266cd500070bc69a46388a45..9dfce950f5a8d46fa2d410b198122e0d965e11c5 100644 (file)
--- a/src/mds/MDSMap.cc
+++ b/src/mds/MDSMap.cc
@@ -12,14 +12,16 @@
  * 
  */
 
+#include "common/debug.h"
+#include "mon/health_check.h"
 
 #include "MDSMap.h"
 
 #include <sstream>
 using std::stringstream;
 
-#include "mon/health_check.h"
-
+#define dout_context g_ceph_context
+#define dout_subsys ceph_subsys_
 
 // features
 CompatSet get_mdsmap_compat_set_all() {
@@ -635,6 +637,23 @@ void MDSMap::encode(bufferlist& bl, uint64_t features) const
   ENCODE_FINISH(bl);
 }
 
+void MDSMap::sanitize(std::function<bool(int64_t pool)> pool_exists)
+{
+  /* Before we did stricter checking, it was possible to remove a data pool
+   * without also deleting it from the MDSMap. Check for that here after
+   * decoding the data pools.
+   */
+
+  for (auto it = data_pools.begin(); it != data_pools.end();) {
+    if (!pool_exists(*it)) {
+      dout(0) << "removed non-existant data pool " << *it << " from MDSMap" << dendl;
+      it = data_pools.erase(it);
+    } else {
+      it++;
+    }
+  }
+}
+
 void MDSMap::decode(bufferlist::iterator& p)
 {
   std::map<mds_rank_t,int32_t> inc;  // Legacy field, parse and drop

diff --git a/src/mds/MDSMap.h b/src/mds/MDSMap.h
index 744e6423508f775242bde6d987b91060e57ccc64..454f422dde2288ee1ae4c70e1912f0ffab49bc03 100644 (file)
--- a/src/mds/MDSMap.h
+++ b/src/mds/MDSMap.h
@@ -660,7 +660,7 @@ public:
     bufferlist::iterator p = bl.begin();
     decode(p);
   }
-
+  void sanitize(std::function<bool(int64_t pool)> pool_exists);
 
   void print(ostream& out) const;
   void print_summary(Formatter *f, ostream *out) const;

diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc
index bae812a95da633aafb3e82ba550670a89c143a7f..882e13cccbcf8b862a21de9b842031ba27a646ab 100644 (file)
--- a/src/mon/MDSMonitor.cc
+++ b/src/mon/MDSMonitor.cc
@@ -117,6 +117,8 @@ void MDSMonitor::update_from_paxos(bool *need_bootstrap)
   assert(fsmap_bl.length() > 0);
   dout(10) << __func__ << " got " << version << dendl;
   fsmap.decode(fsmap_bl);
+  auto &osdmap = mon->osdmon()->osdmap;
+  fsmap.sanitize([&osdmap](int64_t pool){return osdmap.have_pg_pool(pool);});
 
   // new map
   dout(4) << "new map" << dendl;