os/bluestore: more robust handling for lack of per-pool stats cases.

Fixes: https://tracker.ceph.com/issues/37652
Signed-off-by: Igor Fedotov <ifedotov@suse.com>

diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h
index 270f1ec50f5e4eff638ebfa57cf93a0ba8e7f0bd..b1996b79da27e3ff87d978986a5f740c6eb28e6c 100644 (file)
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -1042,7 +1042,6 @@ OPTION(bluestore_fsck_on_umount, OPT_BOOL)
 OPTION(bluestore_fsck_on_umount_deep, OPT_BOOL)
 OPTION(bluestore_fsck_on_mkfs, OPT_BOOL)
 OPTION(bluestore_fsck_on_mkfs_deep, OPT_BOOL)
-OPTION(bluestore_fsck_error_on_legacy_stats, OPT_BOOL)
 OPTION(bluestore_sync_submit_transaction, OPT_BOOL) // submit kv txn in queueing thread (not kv_sync_thread)
 OPTION(bluestore_throttle_bytes, OPT_U64)
 OPTION(bluestore_throttle_deferred_bytes, OPT_U64)
@@ -1072,7 +1071,7 @@ OPTION(bluestore_debug_permit_any_bdev_label, OPT_BOOL)
 OPTION(bluestore_debug_random_read_err, OPT_DOUBLE)
 OPTION(bluestore_debug_inject_bug21040, OPT_BOOL)
 OPTION(bluestore_debug_inject_csum_err_probability, OPT_FLOAT)
-OPTION(bluestore_debug_no_per_pool_stats, OPT_BOOL)
+OPTION(bluestore_no_per_pool_stats_tolerance, OPT_STR)
 
 OPTION(kstore_max_ops, OPT_U64)
 OPTION(kstore_max_bytes, OPT_U64)

diff --git a/src/common/options.cc b/src/common/options.cc
index 4e9b426aadcdd1a2350f192740d7c6f1ed7b7364..ceb9a7098360e4fdc4c1c25174051c2bea1f83d6 100644 (file)
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -4459,10 +4459,6 @@ std::vector<Option> get_global_options() {
     .set_default(false)
     .set_description("Run deep fsck after mkfs"),
 
-    Option("bluestore_fsck_error_on_legacy_stats", Option::TYPE_BOOL, Option::LEVEL_DEV)
-    .set_default(false)
-    .set_description("Popup errors on legacy (store-wide only) stats detection"),
-
     Option("bluestore_sync_submit_transaction", Option::TYPE_BOOL, Option::LEVEL_DEV)
     .set_default(false)
     .set_description("Try to submit metadata transaction to rocksdb in queuing thread context"),
@@ -4591,10 +4587,15 @@ std::vector<Option> get_global_options() {
     .set_default(0.0)
     .set_description("inject crc verification errors into bluestore device reads"),
 
-    Option("bluestore_debug_no_per_pool_stats", Option::TYPE_BOOL, Option::LEVEL_DEV)
-    .set_default(false)
-    .set_description(""),
-
+    Option("bluestore_no_per_pool_stats_tolerance", Option::TYPE_STR, Option::LEVEL_ADVANCED)
+    .set_default("until_repair")
+    .set_flag(Option::FLAG_RUNTIME)
+    .set_enum_allowed({"enforce", "until_repair", "until_fsck"})
+    .set_description("Specified how to treat lack of per-pool stats, e.g. caused by an upgrade")
+    .set_long_description(
+      "'until_fsck' will tolerate the case for regular ops and fail on fsck or repair, the latter will fix the issue, "
+      "'until_repair' will tolerate for regular ops and fsck. Repair indicates and fixes the issue, "
+      "'enforce' will unconditionally use global stats mode."),
     // -----------------------------------------
     // kstore
 

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 08c6f57907a6987f6102130596cce9deb9482f91..db402b045e64b6b8e6797150a71d4a11ceed521b 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -5572,7 +5572,7 @@ void BlueStore::_open_statfs()
     } else {
       dout(10) << __func__ << " store_statfs is corrupt, using empty" << dendl;
     }
-  } else if (cct->_conf->bluestore_debug_no_per_pool_stats) {
+  } else if (cct->_conf->bluestore_no_per_pool_stats_tolerance == "enforce") {
     per_pool_stat_collection = false;
     dout(10) << __func__ << " store_statfs is requested but missing, using empty" << dendl;
   } else {
@@ -6499,24 +6499,26 @@ int BlueStore::_fsck_check_extents(
 
 void BlueStore::_fsck_check_pool_statfs(
   BlueStore::per_pool_statfs& expected_pool_statfs,
+  bool need_per_pool_stats,
   int& errors,
   BlueStoreRepairer* repairer)
 {
-  if (!per_pool_stat_collection) {
-    return;
-  }
   auto it = db->get_iterator(PREFIX_STAT);
   if (it) {
     for (it->lower_bound(string()); it->valid(); it->next()) {
       string key = it->key();
       if (key == BLUESTORE_GLOBAL_STATFS_KEY) {
         if (repairer) {
-         derr << "fsck error: legacy statfs record found, removing" << dendl;
-         repairer->remove_key(db, PREFIX_STAT, BLUESTORE_GLOBAL_STATFS_KEY);
-         errors++;
+         if (need_per_pool_stats) {
+           ++errors;
+           repairer->remove_key(db, PREFIX_STAT, BLUESTORE_GLOBAL_STATFS_KEY);
+           derr << "fsck error: " << "legacy statfs record found, removing" << dendl;
+         } else {
+           derr << "fsck warning: " << "legacy statfs record found, bypassing" << dendl;
+         }
        } else {
          const char* s = "fsck warning: ";
-          if (cct->_conf->bluestore_fsck_error_on_legacy_stats) {
+          if (need_per_pool_stats) {
            ++errors;
            s = "fsck error: ";
          }
@@ -6526,6 +6528,9 @@ void BlueStore::_fsck_check_pool_statfs(
        }
        continue;
       }
+      if (!need_per_pool_stats) {
+       continue;
+      }
       uint64_t pool_id;
       if (get_key_pool_stat(key, &pool_id) < 0) {
        derr << "fsck error: bad key " << key
@@ -6690,6 +6695,10 @@ int BlueStore::_fsck(bool deep, bool repair)
   store_statfs_t* expected_statfs = nullptr;
 
   utime_t start = ceph_clock_now();
+  const auto& no_pps_mode = cct->_conf->bluestore_no_per_pool_stats_tolerance;
+  bool need_per_pool_stats = no_pps_mode == "until_fsck" ||
+    (no_pps_mode == "until_repair" && repair);
+  bool enforce_no_per_pool_stats = no_pps_mode == "enforce";
 
   int r = _open_path();
   if (r < 0)
@@ -6800,11 +6809,7 @@ int BlueStore::_fsck(bool deep, bool repair)
   actual_statfs.internal_metadata = 0;
   actual_statfs.omap_allocated = 0;
 
-  // switch to per-pool stats if not explicitly prohibited
-  if (!per_pool_stat_collection &&
-        !cct->_conf->bluestore_debug_no_per_pool_stats) {
-    per_pool_stat_collection = true;
-  }
+  need_per_pool_stats = per_pool_stat_collection || need_per_pool_stats;
 
   // walk PREFIX_OBJ
   dout(1) << __func__ << " walking object keyspace" << dendl;
@@ -6891,7 +6896,7 @@ int BlueStore::_fsck(bool deep, bool repair)
        auto pool_id = c->cid.is_pg(&pgid) ? pgid.pool() : META_POOL_ID;
        dout(20) << __func__ << "  collection " << c->cid << " " << c->cnode
                 << dendl;
-       if (per_pool_stat_collection) {
+       if (need_per_pool_stats) {
          expected_statfs = &expected_pool_statfs[pool_id];
        }
 
@@ -7178,7 +7183,7 @@ int BlueStore::_fsck(bool deep, bool repair)
        for (auto &r : shared_blob.ref_map.ref_map) {
          extents.emplace_back(bluestore_pextent_t(r.first, r.second.length));
        }
-       if (per_pool_stat_collection) {
+       if (need_per_pool_stats) {
          expected_statfs = &expected_pool_statfs[sbi.pool_id];
        }
        errors += _fsck_check_extents(sbi.cid,
@@ -7238,7 +7243,7 @@ int BlueStore::_fsck(bool deep, bool repair)
            continue;
          }
          auto pool_id = c->cid.is_pg(&pgid) ? pgid.pool() : META_POOL_ID;
-         if (per_pool_stat_collection) {
+         if (need_per_pool_stats) {
            expected_statfs = &expected_pool_statfs[pool_id];
          }
        }
@@ -7416,7 +7421,8 @@ int BlueStore::_fsck(bool deep, bool repair)
   }
   sb_info.clear();
 
-  if (!per_pool_stat_collection) {
+  // check global stats if no-pps is enforced only
+  if (!need_per_pool_stats) {
     if (!(actual_statfs == expected_store_statfs)) {
       derr << "fsck error: actual " << actual_statfs
           << " != expected " << expected_store_statfs << dendl;
@@ -7426,10 +7432,11 @@ int BlueStore::_fsck(bool deep, bool repair)
       }
       ++errors;
     }
-  } else {
+  }
+  if (!enforce_no_per_pool_stats) {
     dout(1) << __func__ << " checking pool_statfs" << dendl;
-    _fsck_check_pool_statfs(expected_pool_statfs, errors,
-      repair ? &repairer : nullptr);
+    _fsck_check_pool_statfs(expected_pool_statfs, need_per_pool_stats,
+      errors, repair ? &repairer : nullptr);
   }
 
   dout(1) << __func__ << " checking for stray omap data" << dendl;
@@ -9434,7 +9441,7 @@ void BlueStore::_txc_update_store_statfs(TransContext *txc)
     vstatfs += txc->statfs_delta; //non-persistent in this mode
 
   } else {
-    txc->t->merge(PREFIX_STAT, "bluestore_statfs", bl);
+    txc->t->merge(PREFIX_STAT, BLUESTORE_GLOBAL_STATFS_KEY, bl);
 
     std::lock_guard l(vstatfs_lock);
     vstatfs += txc->statfs_delta;

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index cea062a84e95d69ce7b9a9ffee1bbd8a66acccab..a263257785dc71fbc64746954540f6da1326c6fb 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -2278,8 +2278,11 @@ private:
 
   using  per_pool_statfs =
     mempool::bluestore_fsck::map<uint64_t, store_statfs_t>;
-  void _fsck_check_pool_statfs(per_pool_statfs& expected_pool_statfs,
-    int& errors, BlueStoreRepairer* repairer);
+  void _fsck_check_pool_statfs(
+    per_pool_statfs& expected_pool_statfs,
+    bool need_per_pool_stats,
+    int& errors,
+    BlueStoreRepairer* repairer);
 
   void _buffer_cache_write(
     TransContext *txc,

diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc
index e718ada1260d255aad641c98a9fe57d27baa1430..862534d0857eb3ec3defd79e1edf2b0d1edce527 100644 (file)
--- a/src/test/objectstore/store_test.cc
+++ b/src/test/objectstore/store_test.cc
@@ -7287,8 +7287,7 @@ TEST_P(StoreTestSpecificAUSize, BluestoreRepairTest) {
   SetVal(g_conf(), "bluestore_max_blob_size", 
     stringify(2 * offs_base).c_str());
   SetVal(g_conf(), "bluestore_extent_map_shard_max_size", "12000");
-  SetVal(g_conf(), "bluestore_debug_no_per_pool_stats", "true");
-  SetVal(g_conf(), "bluestore_fsck_error_on_legacy_stats", "true");
+  SetVal(g_conf(), "bluestore_no_per_pool_stats_tolerance", "enforce");
 
   StartDeferred(0x10000);
 
@@ -7432,14 +7431,13 @@ TEST_P(StoreTestSpecificAUSize, BluestoreRepairTest) {
 
   // enable per-pool stats collection hence causing fsck to fail
   cerr << "per-pool statfs" << std::endl;
-  SetVal(g_conf(), "bluestore_debug_no_per_pool_stats", "false");
+  SetVal(g_conf(), "bluestore_no_per_pool_stats_tolerance", "until_fsck");
   g_ceph_context->_conf.apply_changes(nullptr);
 
   ASSERT_EQ(bstore->fsck(false), 2);
   ASSERT_EQ(bstore->repair(false), 0);
   ASSERT_EQ(bstore->fsck(false), 0);
 
-
   cerr << "Completing" << std::endl;
   bstore->mount();