Merge pull request #11724 from liewegas/wip-bluestore-fsck

os/bluestore: deep fsck

Reviewed-by: Igor Fedotov <ifedotov@mirantis.com>

src/common/config_opts.h

@@ -1009,8 +1009,11 @@ OPTION(bluestore_bitmapallocator_blocks_per_zone, OPT_INT, 1024) // must be powe
 OPTION(bluestore_bitmapallocator_span_size, OPT_INT, 1024) // must be power of 2 aligned, e.g., 512, 1024, 2048...
 OPTION(bluestore_rocksdb_options, OPT_STR, "compression=kNoCompression,max_write_buffer_number=4,min_write_buffer_number_to_merge=1,recycle_log_file_num=4,write_buffer_size=268435456")
 OPTION(bluestore_fsck_on_mount, OPT_BOOL, false)
+OPTION(bluestore_fsck_on_mount_deep, OPT_BOOL, true)
 OPTION(bluestore_fsck_on_umount, OPT_BOOL, false)
+OPTION(bluestore_fsck_on_umount_deep, OPT_BOOL, true)
 OPTION(bluestore_fsck_on_mkfs, OPT_BOOL, true)
+OPTION(bluestore_fsck_on_mkfs_deep, OPT_BOOL, false)
 OPTION(bluestore_sync_submit_transaction, OPT_BOOL, true) // submit kv txn in queueing thread (not kv_sync_thread)
 OPTION(bluestore_sync_wal_apply, OPT_BOOL, true)     // perform initial wal work synchronously (possibly in combination with aio so we only *queue* ios)
 OPTION(bluestore_wal_threads, OPT_INT, 4)
@@ -1041,6 +1044,7 @@ OPTION(kstore_max_bytes, OPT_U64, 64*1024*1024)
 OPTION(kstore_backend, OPT_STR, "rocksdb")
 OPTION(kstore_rocksdb_options, OPT_STR, "compression=kNoCompression")
 OPTION(kstore_fsck_on_mount, OPT_BOOL, false)
+OPTION(kstore_fsck_on_mount_deep, OPT_BOOL, true)
 OPTION(kstore_nid_prealloc, OPT_U64, 1024)
 OPTION(kstore_sync_transaction, OPT_BOOL, false)
 OPTION(kstore_sync_submit_transaction, OPT_BOOL, false)

src/os/ObjectStore.h

@@ -1879,7 +1879,7 @@ class ObjectStore {
   virtual bool test_mount_in_use() = 0;
   virtual int mount() = 0;
   virtual int umount() = 0;
-  virtual int fsck() {
+  virtual int fsck(bool deep) {
     return -EOPNOTSUPP;
   }
 

src/os/bluestore/BlueStore.cc

@@ -3748,7 +3748,7 @@ int BlueStore::mkfs()
     if (r == 0) {
       dout(1) << __func__ << " already created" << dendl;
       if (g_conf->bluestore_fsck_on_mkfs) {
-        r = fsck();
+        r = fsck(g_conf->bluestore_fsck_on_mkfs_deep);
         if (r < 0) {
           derr << __func__ << " fsck found fatal error: " << cpp_strerror(r)
                << dendl;
@@ -3912,7 +3912,7 @@ int BlueStore::mkfs()
 
   if (r == 0 &&
       g_conf->bluestore_fsck_on_mkfs) {
-    int rc = fsck();
+    int rc = fsck(g_conf->bluestore_fsck_on_mkfs_deep);
     if (rc < 0)
       return rc;
     if (rc > 0) {
@@ -3954,7 +3954,7 @@ int BlueStore::mount()
   }
 
   if (g_conf->bluestore_fsck_on_mount) {
-    int rc = fsck();
+    int rc = fsck(g_conf->bluestore_fsck_on_mount_deep);
     if (rc < 0)
       return rc;
     if (rc > 0) {
@@ -4085,7 +4085,7 @@ int BlueStore::umount()
   _close_path();
 
   if (g_conf->bluestore_fsck_on_umount) {
-    int rc = fsck();
+    int rc = fsck(g_conf->bluestore_fsck_on_umount_deep);
     if (rc < 0)
       return rc;
     if (rc > 0) {
@@ -4115,7 +4115,8 @@ int BlueStore::_fsck_check_extents(
   const vector<bluestore_pextent_t>& extents,
   bool compressed,
   boost::dynamic_bitset<> &used_blocks,
-  store_statfs_t& expected_statfs)
+  store_statfs_t& expected_statfs,
+  bool deep)
 {
   dout(30) << __func__ << " oid " << oid << " extents " << extents << dendl;
   int errors = 0;
@@ -4149,9 +4150,9 @@ int BlueStore::_fsck_check_extents(
   return errors;
 }
 
-int BlueStore::fsck()
+int BlueStore::fsck(bool deep)
 {
-  dout(1) << __func__ << " start" << dendl;
+  dout(1) << __func__ << (deep ? " (deep)" : " (shallow)") << " start" << dendl;
   int errors = 0;
   set<uint64_t> used_nids;
   set<uint64_t> used_omap_head;
@@ -4421,9 +4422,19 @@ int BlueStore::fsck()
 	  errors += _fsck_check_extents(oid, blob.extents,
 					blob.is_compressed(),
 					used_blocks,
-					expected_statfs);
+					expected_statfs,
+					deep);
         }
       }
+      if (deep) {
+	bufferlist bl;
+	int r = _do_read(c.get(), o, 0, o->onode.size, bl, 0);
+	if (r < 0) {
+	  ++errors;
+	  derr << __func__ << " " << oid << " error during read: "
+	       << cpp_strerror(r) << dendl;
+	}
+      }
       // omap
       while (o->onode.omap_head) {
 	if (used_omap_head.count(o->onode.omap_head)) {
@@ -4472,9 +4483,9 @@ int BlueStore::fsck()
 	  extents.emplace_back(bluestore_pextent_t(r.first, r.second.length));
 	}
 	errors += _fsck_check_extents(p->second.oids.front(),
-			    extents,
-			    p->second.compressed,
-			    used_blocks, expected_statfs);
+				      extents,
+				      p->second.compressed,
+				      used_blocks, expected_statfs, deep);
 	sb_info.erase(p);
       }
     }

src/os/bluestore/BlueStore.h

@@ -1581,7 +1581,8 @@ class BlueStore : public ObjectStore,
     const vector<bluestore_pextent_t>& extents,
     bool compressed,
     boost::dynamic_bitset<> &used_blocks,
-    store_statfs_t& expected_statfs);
+    store_statfs_t& expected_statfs,
+    bool deep);
 
   void _buffer_cache_write(
     TransContext *txc,
@@ -1625,7 +1626,7 @@ class BlueStore : public ObjectStore,
   int umount() override;
   void _sync();
 
-  int fsck() override;
+  int fsck(bool deep) override;
 
   void set_cache_shards(unsigned num) override;
 

src/os/kstore/KStore.cc

@@ -973,7 +973,7 @@ int KStore::mount()
   dout(1) << __func__ << " path " << path << dendl;
 
   if (g_conf->kstore_fsck_on_mount) {
-    int rc = fsck();
+    int rc = fsck(g_conf->kstore_fsck_on_mount_deep);
     if (rc < 0)
       return rc;
   }
@@ -1044,7 +1044,7 @@ int KStore::umount()
   return 0;
 }
 
-int KStore::fsck()
+int KStore::fsck(bool deep)
 {
   dout(1) << __func__ << dendl;
   int errors = 0;

src/os/kstore/KStore.h

@@ -416,7 +416,7 @@ class KStore : public ObjectStore {
   int umount();
   void _sync();
 
-  int fsck();
+  int fsck(bool deep) override;
 
 
   int validate_hobject_key(const hobject_t &obj) const override {

src/test/objectstore/store_test.cc

@@ -4066,17 +4066,13 @@ class SyntheticWorkloadState {
     return status;
   }
 
-  void fsck() {
+  void fsck(bool deep) {
     Mutex::Locker locker(lock);
     EnterExit ee("fsck");
     while (in_flight)
       cond.Wait(lock);
     store->umount();
-    // fixme: this is bluestore specific, but...
-    if (!g_conf->bluestore_fsck_on_mount &&
-	!g_conf->bluestore_fsck_on_umount) {
-      store->fsck();
-    }
+    store->fsck(deep);
     store->mount();
   }
 
@@ -4235,6 +4231,10 @@ void doSyntheticTest(boost::scoped_ptr<ObjectStore>& store,
   gen_type rng(time(NULL));
   coll_t cid(spg_t(pg_t(0,555), shard_id_t::NO_SHARD));
 
+  g_ceph_context->_conf->set_val("bluestore_fsck_on_mount", "false");
+  g_ceph_context->_conf->set_val("bluestore_fsck_on_umount", "false");
+  g_ceph_context->_conf->apply_changes(NULL);
+
   SyntheticWorkloadState test_obj(store.get(), &gen, &rng, &osr, cid,
 				  max_obj, max_wr, align);
   test_obj.init();
@@ -4250,7 +4250,9 @@ void doSyntheticTest(boost::scoped_ptr<ObjectStore>& store,
     boost::uniform_int<> true_false(0, 999);
     int val = true_false(rng);
     if (val > 998) {
-      test_obj.fsck();
+      test_obj.fsck(true);
+    } else if (val > 997) {
+      test_obj.fsck(false);
     } else if (val > 970) {
       test_obj.scan();
     } else if (val > 950) {
@@ -4277,6 +4279,10 @@ void doSyntheticTest(boost::scoped_ptr<ObjectStore>& store,
   }
   test_obj.wait_for_done();
   test_obj.shutdown();
+
+  g_ceph_context->_conf->set_val("bluestore_fsck_on_mount", "true");
+  g_ceph_context->_conf->set_val("bluestore_fsck_on_umount", "true");
+  g_ceph_context->_conf->apply_changes(NULL);
 }
 
 TEST_P(StoreTest, Synthetic) {

src/tools/ceph_objectstore_tool.cc

@@ -2595,7 +2595,20 @@ int main(int argc, char **argv)
   }
 
   if (op == "fsck") {
-    int r = fs->fsck();
+    int r = fs->fsck(false);
+    if (r < 0) {
+      cerr << "fsck failed: " << cpp_strerror(r) << std::endl;
+      myexit(1);
+    }
+    if (r > 0) {
+      cerr << "fsck found " << r << " errors" << std::endl;
+      myexit(1);
+    }
+    cout << "fsck found no errors" << std::endl;
+    exit(0);
+  }
+  if (op == "fsck-deep") {
+    int r = fs->fsck(true);
     if (r < 0) {
       cerr << "fsck failed: " << cpp_strerror(r) << std::endl;
       myexit(1);