os/bluestore: separate omap per-pool vs per-pg alerts

Currently the health alert raised does not match the docs, and the docs
do not describe what the health alert indicates.

Octopus added per-pool omap storage.  This improves space accounting
and reporting.

Pacific added per-pg omap storage (object hash in key).  This speeds up
PG removal.

Separate everthing out into two distinct alerts raised from bluestore
and surfaced as health alerts, with corresponding config options to
disable, and update the docs accordingly.

Also update the fsck options for warn vs error, and raise separate
errors for the per-pg and per-pool cases.

Signed-off-by: Sage Weil <sage@newdream.net>

diff --git a/doc/rados/operations/health-checks.rst b/doc/rados/operations/health-checks.rst
index c84efa155197c918db463e8d85de7300d3e25e88..242858718d2d6ba9f723431b31c3e4bc1269b14d 100644 (file)
--- a/doc/rados/operations/health-checks.rst
+++ b/doc/rados/operations/health-checks.rst
@@ -511,6 +511,25 @@ This warning can be disabled with::
 
   ceph config set global bluestore_warn_on_no_per_pool_omap false
 
+BLUESTORE_NO_PER_PG_OMAP
+__________________________
+
+Starting with the Pacific release, BlueStore tracks omap space utilization
+by PG, and one or more OSDs have volumes that were created prior to
+Pacific.  Per-PG omap enables faster PG removal when PGs migrate.
+
+The older OSDs can be updated to track by PG by stopping each OSD,
+running a repair operation, and the restarting it.  For example, if
+``osd.123`` needed to be updated,::
+
+  systemctl stop ceph-osd@123
+  ceph-bluestore-tool repair --path /var/lib/ceph/osd/ceph-123
+  systemctl start ceph-osd@123
+
+This warning can be disabled with::
+
+  ceph config set global bluestore_warn_on_no_per_pg_omap false
+
 
 BLUESTORE_DISK_SIZE_MISMATCH
 ____________________________

diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h
index 12835cec3e4aff9a1f0f9be6a50939bfb227968c..ebcedd101734774f05f0216151171bf1b4cd3fc0 100644 (file)
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -1040,7 +1040,9 @@ OPTION(bluestore_warn_on_bluefs_spillover, OPT_BOOL)
 OPTION(bluestore_warn_on_legacy_statfs, OPT_BOOL)
 OPTION(bluestore_warn_on_spurious_read_errors, OPT_BOOL)
 OPTION(bluestore_fsck_error_on_no_per_pool_omap, OPT_BOOL)
+OPTION(bluestore_fsck_error_on_no_per_pg_omap, OPT_BOOL)
 OPTION(bluestore_warn_on_no_per_pool_omap, OPT_BOOL)
+OPTION(bluestore_warn_on_no_per_pg_omap, OPT_BOOL)
 OPTION(bluestore_log_op_age, OPT_DOUBLE)
 OPTION(bluestore_log_omap_iterator_age, OPT_DOUBLE)
 OPTION(bluestore_log_collection_list_age, OPT_DOUBLE)

diff --git a/src/common/options.cc b/src/common/options.cc
index 38c1d5adb02d36230fc6c9dc6943797197bc4c08..c706fc126197897e027a11c8dbf7ff9e50956d20 100644 (file)
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -4812,10 +4812,18 @@ std::vector<Option> get_global_options() {
     .set_default(false)
     .set_description("Make fsck error (instead of warn) when objects without per-pool omap are found"),
 
+    Option("bluestore_fsck_error_on_no_per_pg_omap", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
+    .set_default(false)
+    .set_description("Make fsck error (instead of warn) when objects without per-pg omap are found"),
+
     Option("bluestore_warn_on_no_per_pool_omap", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
     .set_default(true)
     .set_description("Enable health indication on lack of per-pool omap"),
 
+    Option("bluestore_warn_on_no_per_pg_omap", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
+    .set_default(false)
+    .set_description("Enable health indication on lack of per-pg omap"),
+
     Option("bluestore_log_op_age", Option::TYPE_FLOAT, Option::LEVEL_ADVANCED)
     .set_default(5)
     .set_description("log operation if it's slower than this age (seconds)"),

diff --git a/src/mon/PGMap.cc b/src/mon/PGMap.cc
index 8a2356d97de6e577e3bcde540453922a5259bc9e..ab59b3d28506ea8ce0450b75a5e32067d3226cfd 100644 (file)
--- a/src/mon/PGMap.cc
+++ b/src/mon/PGMap.cc
@@ -3244,7 +3244,9 @@ void PGMap::get_health_checks(
       } else if (asum.first == "BLUESTORE_DISK_SIZE_MISMATCH") {
        summary += " have dangerous mismatch between BlueStore block device and free list sizes";
       } else if (asum.first == "BLUESTORE_NO_PER_PG_OMAP") {
-       summary += " reporting legacy (not per-pg) BlueStore omap usage stats";
+       summary += " reporting legacy (not per-pg) BlueStore omap";
+      } else if (asum.first == "BLUESTORE_NO_PER_POOL_OMAP") {
+       summary += " reporting legacy (not per-pool) BlueStore omap usage stats";
       } else if (asum.first == "BLUESTORE_SPURIOUS_READ_ERRORS") {
         summary += " have spurious read errors";
       }

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 73d4b8ba277097ac3efac7734a252d99257b2874..0ffc5c47bfc773b0a0d4f5d9c16d7acb3b382056 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -4550,8 +4550,9 @@ void BlueStore::handle_conf_change(const ConfigProxy& conf,
   if (changed.count("bluestore_warn_on_legacy_statfs")) {
     _check_legacy_statfs_alert();
   }
-  if (changed.count("bluestore_warn_on_no_per_pool_omap")) {
-    _check_no_per_pg_omap_alert();
+  if (changed.count("bluestore_warn_on_no_per_pool_omap") ||
+      changed.count("bluestore_warn_on_no_per_pg_omap")) {
+    _check_no_per_pg_or_pool_omap_alert();
   }
 
   if (changed.count("bluestore_csum_type")) {
@@ -6227,7 +6228,7 @@ void BlueStore::_set_per_pool_omap()
   } else {
     dout(10) << __func__ << " per_pool_omap not present" << dendl;
   }
-  _check_no_per_pg_omap_alert();
+  _check_no_per_pg_or_pool_omap_alert();
 }
 
 void BlueStore::_open_statfs()
@@ -7740,7 +7741,31 @@ void BlueStore::_fsck_check_object_omap(FSCKDepth depth,
   auto repairer = ctx.repairer;
 
   ceph_assert(o->onode.has_omap());
-  if (!o->onode.is_perpg_omap() && !o->onode.is_pgmeta_omap()) {
+  if (!o->onode.is_perpool_omap() && !o->onode.is_pgmeta_omap()) {
+    if (per_pool_omap == OMAP_PER_POOL) {
+      fsck_derr(errors, MAX_FSCK_ERROR_LINES)
+        << "fsck error: " << o->oid
+        << " has omap that is not per-pool or pgmeta"
+        << fsck_dendl;
+      ++errors;
+    } else {
+      const char* w;
+      int64_t num;
+      if (cct->_conf->bluestore_fsck_error_on_no_per_pool_omap) {
+        ++errors;
+        num = errors;
+        w = "error";
+      } else {
+        ++warnings;
+        num = warnings;
+        w = "warning";
+      }
+      fsck_derr(num, MAX_FSCK_ERROR_LINES)
+        << "fsck " << w << ": " << o->oid
+        << " has omap that is not per-pool or pgmeta"
+        << fsck_dendl;
+    }
+  } else if (!o->onode.is_perpg_omap() && !o->onode.is_pgmeta_omap()) {
     if (per_pool_omap == OMAP_PER_PG) {
       fsck_derr(errors, MAX_FSCK_ERROR_LINES)
         << "fsck error: " << o->oid
@@ -7750,7 +7775,7 @@ void BlueStore::_fsck_check_object_omap(FSCKDepth depth,
     } else {
       const char* w;
       int64_t num;
-      if (cct->_conf->bluestore_fsck_error_on_no_per_pool_omap) {
+      if (cct->_conf->bluestore_fsck_error_on_no_per_pg_omap) {
         ++errors;
         num = errors;
         w = "error";
@@ -9252,16 +9277,24 @@ void BlueStore::_check_legacy_statfs_alert()
   legacy_statfs_alert = s;
 }
 
-void BlueStore::_check_no_per_pg_omap_alert()
+void BlueStore::_check_no_per_pg_or_pool_omap_alert()
 {
-  string s;
-  if (per_pool_omap != OMAP_PER_PG &&
-      cct->_conf->bluestore_warn_on_no_per_pool_omap) {
-    s = "legacy (not per-pg) omap detected, "
-      "suggest to run store repair to benefit from per-pool omap usage statistic and faster PG removal";
+  string per_pg, per_pool;
+  if (per_pool_omap != OMAP_PER_PG) {
+    if (cct->_conf->bluestore_warn_on_no_per_pg_omap) {
+      per_pg = "legacy (not per-pg) omap detected, "
+       "suggest to run store repair to benefit from faster PG removal";
+    }
+    if (per_pool_omap != OMAP_PER_POOL) {
+      if (cct->_conf->bluestore_warn_on_no_per_pool_omap) {
+       per_pool = "legacy (not per-pool) omap detected, "
+         "suggest to run store repair to benefit from per-pool omap usage statistics";
+      }
+    }
   }
   std::lock_guard l(qlock);
-  no_per_pg_omap_alert = s;
+  no_per_pg_omap_alert = per_pg;
+  no_per_pool_omap_alert = per_pool;
 }
 
 // ---------------
@@ -15952,6 +15985,11 @@ void BlueStore::_log_alerts(osd_alert_list_t& alerts)
       "BLUESTORE_NO_PER_PG_OMAP",
       no_per_pg_omap_alert);
   }
+  if (!no_per_pool_omap_alert.empty()) {
+    alerts.emplace(
+      "BLUESTORE_NO_PER_POOL_OMAP",
+      no_per_pool_omap_alert);
+  }
   string s0(failed_cmode);
 
   if (!failed_compressors.empty()) {

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 8f262367827935e4657cf3b83133dc86fcee31da..265ccbac8c901c71e77cacc43c58075b4ddd584b 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -3029,6 +3029,7 @@ private:
   std::set<std::string> failed_compressors;
   std::string spillover_alert;
   std::string legacy_statfs_alert;
+  std::string no_per_pool_omap_alert;
   std::string no_per_pg_omap_alert;
   std::string disk_size_mismatch_alert;
   std::string spurious_read_errors_alert;
@@ -3059,7 +3060,7 @@ private:
   }
 
   void _check_legacy_statfs_alert();
-  void _check_no_per_pg_omap_alert();
+  void _check_no_per_pg_or_pool_omap_alert();
   void _set_disk_size_mismatch_alert(const std::string& s) {
     std::lock_guard l(qlock);
     disk_size_mismatch_alert = s;