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>
(cherry picked from commit f755e353e88b65fb923289464935ec635daf13b2)

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 88114a8ae9756a80864d24e7b1dfad33cad472af..5f5a575fc7b18cf2781ebf81969b3c8155dc92ff 100644 (file)
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -1041,7 +1041,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 6f33372873aef58e5d30d40cb7647f0ec4ac4a2b..f166a5e3d6908d012c4181608086ce170c108601 100644 (file)
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -4816,10 +4816,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 6b4a1935e51ddbcdd37494c54241bffff9fbf015..55f72d35476076733ebc1a2f2a2535373753a6e4 100644 (file)
--- a/src/mon/PGMap.cc
+++ b/src/mon/PGMap.cc
@@ -3272,7 +3272,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 65cdc34aa8d3759ffe9df41303945574fb0e7a5e..b933b7aff149d8494d263220d7b7d1b5b3ac7347 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";
@@ -9251,16 +9276,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;
 }
 
 // ---------------
@@ -16022,6 +16055,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 a251b82f7bf2cb82716dc6642991b02b8a5e44cc..ce48331b223b6706b4a668071e6ae46a1658b264 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -3052,6 +3052,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;
@@ -3082,7 +3083,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;