osd,OSDMonitor: try to protect against ec overwrites with filestore

This isn't perfect, but it's better than nothing. Prevent enabling the
allow_ec_overwrites flag if any of a sample of pgs in the pool map to
osds using filestore. This mainly protects filestore-only clusters
from enabling it.

If a filestore osd is started later, warn in the cluster log when it
gets a pg with ec overwrites enabled.

Signed-off-by: Josh Durgin <jdurgin@redhat.com>

diff --git a/qa/workunits/cephtool/test.sh b/qa/workunits/cephtool/test.sh
index 94c11aeebf24b18352b2408bf5b2a9612e4c8a9e..4398ab6114604a4adf129036bd74fc2d3b0441f1 100755 (executable)
--- a/qa/workunits/cephtool/test.sh
+++ b/qa/workunits/cephtool/test.sh
@@ -1320,7 +1320,19 @@ function test_mon_osd_pool()
   # should fail because the type is not the same
   expect_false ceph osd pool create replicated 12 12 erasure
   ceph osd lspools | grep replicated
+  ceph osd pool create ec_test 1 1 erasure
+  set +e
+  ceph osd metadata | grep osd_objectstore_type | grep -qc bluestore
+  if [ $? -eq 0 ]; then
+      ceph osd pool set ec_test allow_ec_overwrites true >& $TMPFILE
+      check_response $? 22 "pool must only be stored on bluestore for scrubbing to work"
+  else
+      ceph osd pool set ec_test allow_ec_overwrites true || return 1
+      expect_false ceph osd pool set ec_test allow_ec_overwrites false
+  fi
+  set -e
   ceph osd pool delete replicated replicated --yes-i-really-really-mean-it
+  ceph osd pool delete ec_test ec_test --yes-i-really-really-mean-it
 }
 
 function test_mon_osd_pool_quota()

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 5cdbac9866ccb053e0b391af6e197d181ba050df..1b48c8edccb6304a55edd960aa85382c0e195dc0 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -1232,6 +1232,48 @@ int OSDMonitor::load_metadata(int osd, map<string, string>& m, ostream *err)
   return 0;
 }
 
+int OSDMonitor::get_osd_objectstore_type(int osd, string *type)
+{
+  map<string, string> metadata;
+  int r = load_metadata(osd, metadata, nullptr);
+  if (r < 0)
+    return r;
+
+  auto it = metadata.find("osd_objectstore");
+  if (it == metadata.end())
+    return -ENOENT;
+  *type = it->second;
+  return 0;
+}
+
+bool OSDMonitor::is_pool_currently_all_bluestore(int64_t pool_id,
+                                                const pg_pool_t &pool,
+                                                ostream *err)
+{
+  // just check a few pgs for efficiency - this can't give a guarantee anyway,
+  // since filestore osds could always join the pool later
+  set<int> checked_osds;
+  for (unsigned ps = 0; ps < MIN(8, pool.get_pg_num()); ++ps) {
+    vector<int> up, acting;
+    pg_t pgid(ps, pool_id, -1);
+    osdmap.pg_to_up_acting_osds(pgid, up, acting);
+    for (int osd : up) {
+      if (checked_osds.find(osd) != checked_osds.end())
+       continue;
+      string objectstore_type;
+      int r = get_osd_objectstore_type(osd, &objectstore_type);
+      // allow with missing metadata, e.g. due to an osd never booting yet
+      if (r < 0 || objectstore_type == "bluestore") {
+       checked_osds.insert(osd);
+       continue;
+      }
+      *err << "osd." << osd << " uses " << objectstore_type;
+      return false;
+    }
+  }
+  return true;
+}
+
 int OSDMonitor::dump_osd_metadata(int osd, Formatter *f, ostream *err)
 {
   map<string,string> m;
@@ -5782,6 +5824,11 @@ int OSDMonitor::prepare_command_pool_set(map<string,cmd_vartype> &cmdmap,
       ss << "expecting value 'true', 'false', '0', or '1'";
       return -EINVAL;
     }
+    stringstream err;
+    if (!is_pool_currently_all_bluestore(pool, p, &err)) {
+      ss << "pool must only be stored on bluestore for scrubbing to work: " << err.str();
+      return -EINVAL;
+    }
   } else if (var == "target_max_objects") {
     if (interr.length()) {
       ss << "error parsing int '" << val << "': " << interr;

diff --git a/src/mon/OSDMonitor.h b/src/mon/OSDMonitor.h
index 3ccbeecec76d73c667b27fcd2596ba1b0c03e819..5144e89b6d1bd3a780e35989846cbc55b91c0b33 100644 (file)
--- a/src/mon/OSDMonitor.h
+++ b/src/mon/OSDMonitor.h
@@ -411,6 +411,9 @@ private:
   OpTracker op_tracker;
 
   int load_metadata(int osd, map<string, string>& m, ostream *err);
+  int get_osd_objectstore_type(int osd, std::string *type);
+  bool is_pool_currently_all_bluestore(int64_t pool_id, const pg_pool_t &pool,
+                                      ostream *err);
 
   // when we last received PG stats from each osd
   map<int,utime_t> last_osd_report;

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 1082c66e5dbcfe2433dc1b4be2acec70cba89c37..809e8f21e5d4c17e1aff93fde689503094136f22 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -3894,6 +3894,13 @@ int OSD::handle_pg_peering_evt(
     switch (result) {
     case RES_NONE: {
       const pg_pool_t* pp = osdmap->get_pg_pool(pgid.pool());
+      if (pp->has_flag(pg_pool_t::FLAG_EC_OVERWRITES) &&
+         store->get_type() != "bluestore") {
+       clog->warn() << "pg " << pgid
+                    << " is at risk of silent data corruption: "
+                    << "the pool allows ec overwrites but is not stored in "
+                    << "bluestore, so deep scrubbing will not detect bitrot";
+      }
       PG::_create(*rctx.transaction, pgid, pgid.get_split_bits(pp->get_pg_num()));
       PG::_init(*rctx.transaction, pgid, pp);