From 4e8ff3cccfbb9702ead404d173f878268c8b0544 Mon Sep 17 00:00:00 2001 From: Igor Fedotov Date: Thu, 20 May 2021 02:17:21 +0300 Subject: [PATCH] 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 (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 --- src/os/bluestore/BlueStore.cc | 30 +++++++++++++++++++++--------- src/os/bluestore/BlueStore.h | 30 +++++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index c4bd42babeb28..793fb670d1d59 100644 --- 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 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& 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 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 159e92963e1f8..4b656460e19b5 100644 --- 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; @@ -3369,7 +3371,9 @@ public: FreelistManager* fm, uint64_t offset, uint64_t len); bool fix_bluefs_extents(std::atomic& out_of_sync_flag); - KeyValueDB::Transaction fix_spanning_blobs(KeyValueDB* db); + bool fix_spanning_blobs( + KeyValueDB* db, + std::function 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; } -- 2.39.5