os/bluestore: introduce multithireading sync for bluestore's repairer

In quick-fix mode bluestore uses 2 threads by default to perform the
repair. Due to lacking synchronization they might corrupt repair
transaction batch.

Fixes: https://tracker.ceph.com/issues/50017
Signed-off-by: Igor Fedotov <ifedotov@suse.com>
(cherry picked from commit 38c5b04235402a7908bc4713f617d767ca9fdc56)

 Conflicts:
	src/os/bluestore/BlueStore.cc - future stuff attempted to sneak
in
	src/os/bluestore/BlueStore.h - the same as above

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index c4bd42babeb2804a4007dafccbe2720fcb1b73b7..793fb670d1d59725d0000d0b9eea5f3723d4efcf 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -7166,7 +7166,7 @@ int BlueStore::_fsck_check_extents(
            bs.set(pos);
         });
         if (repairer) {
-         repairer->get_space_usage_tracker().set_used( e.offset, e.length, cid, oid);
+         repairer->set_space_used(e.offset, e.length, cid, oid);
         }
 
       if (e.end() > bdev->get_size()) {
@@ -7490,8 +7490,11 @@ BlueStore::OnodeRef BlueStore::fsck_check_objects_shallow(
            << " zombie spanning blob(s) found, the first one: "
            << *first_broken << dendl;
       if(repairer) {
-        auto txn = repairer->fix_spanning_blobs(db);
-       _record_onode(o, txn);
+        repairer->fix_spanning_blobs(
+         db,
+         [&](KeyValueDB::Transaction txn) {
+           _record_onode(o, txn);
+         });
       }
     }
   }
@@ -8168,7 +8171,7 @@ int BlueStore::_fsck_on_open(BlueStore::FSCKDepth depth, bool repair)
     }
   );
   if (repair) {
-    repairer.get_space_usage_tracker().init(
+    repairer.init_space_usage_tracker(
       bdev->get_size(),
       min_alloc_size);
   }
@@ -8348,7 +8351,6 @@ int BlueStore::_fsck_on_open(BlueStore::FSCKDepth depth, bool repair)
   if (repair && repairer.preprocess_misreference(db)) {
 
     dout(1) << __func__ << " sorting out misreferenced extents" << dendl;
-    auto& space_tracker = repairer.get_space_usage_tracker();
     auto& misref_extents = repairer.get_misreferences();
     interval_set<uint64_t> to_release;
     it = db->get_iterator(PREFIX_OBJ);
@@ -8370,7 +8372,7 @@ int BlueStore::_fsck_on_open(BlueStore::FSCKDepth depth, bool repair)
 
        ghobject_t oid;
        int r = get_key_object(it->key(), &oid);
-       if (r < 0 || !space_tracker.is_used(oid)) {
+       if (r < 0 || !repairer.is_used(oid)) {
          continue;
        }
 
@@ -8393,7 +8395,7 @@ int BlueStore::_fsck_on_open(BlueStore::FSCKDepth depth, bool repair)
            expected_statfs = &expected_pool_statfs[pool_id];
          }
        }
-       if (!space_tracker.is_used(c->cid)) {
+       if (!repairer.is_used(c->cid)) {
          continue;
        }
 
@@ -14984,6 +14986,7 @@ bool BlueStoreRepairer::remove_key(KeyValueDB *db,
                                   const string& prefix,
                                   const string& key)
 {
+  std::lock_guard l(lock);
   if (!remove_key_txn) {
     remove_key_txn = db->get_transaction();
   }
@@ -14998,6 +15001,7 @@ bool BlueStoreRepairer::fix_shared_blob(
   uint64_t sbid,
   const bufferlist* bl)
 {
+  std::lock_guard l(lock); // possibly redundant
   KeyValueDB::Transaction txn;
   if (fix_misreferences_txn) { // reuse this txn
     txn = fix_misreferences_txn;
@@ -15023,6 +15027,7 @@ bool BlueStoreRepairer::fix_statfs(KeyValueDB *db,
                                   const string& key,
                                   const store_statfs_t& new_statfs)
 {
+  std::lock_guard l(lock);
   if (!fix_statfs_txn) {
     fix_statfs_txn = db->get_transaction();
   }
@@ -15039,6 +15044,7 @@ bool BlueStoreRepairer::fix_leaked(KeyValueDB *db,
                                   FreelistManager* fm,
                                   uint64_t offset, uint64_t len)
 {
+  std::lock_guard l(lock);
   if (!fix_fm_leaked_txn) {
     fix_fm_leaked_txn = db->get_transaction();
   }
@@ -15050,6 +15056,7 @@ bool BlueStoreRepairer::fix_false_free(KeyValueDB *db,
                                       FreelistManager* fm,
                                       uint64_t offset, uint64_t len)
 {
+  std::lock_guard l(lock);
   if (!fix_fm_false_free_txn) {
     fix_fm_false_free_txn = db->get_transaction();
   }
@@ -15067,17 +15074,22 @@ bool BlueStoreRepairer::fix_bluefs_extents(std::atomic<uint64_t>& out_of_sync_fl
   ++to_repair_cnt;
   return true;
 }
-KeyValueDB::Transaction BlueStoreRepairer::fix_spanning_blobs(KeyValueDB* db)
+bool BlueStoreRepairer::fix_spanning_blobs(
+  KeyValueDB* db,
+  std::function<void(KeyValueDB::Transaction)> f)
 {
+  std::lock_guard l(lock);
   if (!fix_onode_txn) {
     fix_onode_txn = db->get_transaction();
   }
+  f(fix_onode_txn);
   ++to_repair_cnt;
-  return fix_onode_txn;
+  return true;
 }
 
 bool BlueStoreRepairer::preprocess_misreference(KeyValueDB *db)
 {
+  //NB: not for use in multithreading mode!!!
   if (misreferenced_extents.size()) {
     size_t n = space_usage_tracker.filter_out(misreferenced_extents);
     ceph_assert(n > 0);

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 159e92963e1f8344ce732a1e9a4e528c92fe65f8..4b656460e19b51b52f4dee46a16089155bef265b 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -3227,6 +3227,8 @@ static inline void intrusive_ptr_release(BlueStore::OpSequencer *o) {
 
 class BlueStoreRepairer
 {
+  ceph::mutex lock = ceph::make_mutex("BlueStore::BlueStoreRepairer::lock");
+
 public:
   // to simplify future potential migration to mempools
   using fsck_interval = interval_set<uint64_t>;
@@ -3369,7 +3371,9 @@ public:
                      FreelistManager* fm,
                      uint64_t offset, uint64_t len);
   bool fix_bluefs_extents(std::atomic<uint64_t>& out_of_sync_flag);
-  KeyValueDB::Transaction fix_spanning_blobs(KeyValueDB* db);
+  bool fix_spanning_blobs(
+    KeyValueDB* db,
+    std::function<void(KeyValueDB::Transaction)> f);
 
   void init(uint64_t total_space, uint64_t lres_tracking_unit_size);
 
@@ -3378,6 +3382,7 @@ public:
   unsigned apply(KeyValueDB* db);
 
   void note_misreference(uint64_t offs, uint64_t len, bool inc_error) {
+    std::lock_guard l(lock);
     misreferenced_extents.union_insert(offs, len);
     if (inc_error) {
       ++to_repair_cnt;
@@ -3387,13 +3392,32 @@ public:
     ++to_repair_cnt;
   }  
 
-  StoreSpaceTracker& get_space_usage_tracker() {
-    return space_usage_tracker;
+  void init_space_usage_tracker(
+    uint64_t total_space, uint64_t lres_tracking_unit_size)
+  {
+    //NB: not for use in multithreading mode!!!
+    space_usage_tracker.init(total_space, lres_tracking_unit_size);
+  }
+  void set_space_used(uint64_t offset, uint64_t len,
+    const coll_t& cid, const ghobject_t& oid) {
+    std::lock_guard l(lock);
+    space_usage_tracker.set_used(offset, len, cid, oid);
   }
+  inline bool is_used(const coll_t& cid) const {
+    //NB: not for use in multithreading mode!!!
+    return space_usage_tracker.is_used(cid);
+  }
+  inline bool is_used(const ghobject_t& oid) const {
+    //NB: not for use in multithreading mode!!!
+    return space_usage_tracker.is_used(oid);
+  }
+
   const fsck_interval& get_misreferences() const {
+    //NB: not for use in multithreading mode!!!
     return misreferenced_extents;
   }
   KeyValueDB::Transaction get_fix_misreferences_txn() {
+    //NB: not for use in multithreading mode!!!
     return fix_misreferences_txn;
   }