From d33acd8c914cfa1f4cc8e84061bf5b3aa6cb6e43 Mon Sep 17 00:00:00 2001 From: Gabriel BenHanokh Date: Tue, 31 Aug 2021 13:56:25 +0300 Subject: [PATCH] os/bluestore/BlueStore::NCB - removed bogus assert for overlapping extents Fixes: https://tracker.ceph.com/issues/52138 BUG-FIX: NCB code was reporting Bogus error when we had an overlapped extent between shared-blobs on the same Onode (which is a legal case) The error check was refined to skip shared-blobs I also added an assert for copy-allocator spillover (should never happen) and removed an early exit when an empty extent was found (we report and skip it) Signed-off-by: Gabriel Benhanokh --- src/os/bluestore/BlueStore.cc | 48 +++++++++++++++++------------------ src/os/bluestore/BlueStore.h | 2 ++ 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 94e61f7d9ab38..b6a6e333828f7 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -17129,7 +17129,6 @@ int BlueStore::copy_allocator(Allocator* src_alloc, Allocator* dest_alloc, uint6 } uint64_t idx = 0; - bool null_extent = false; auto copy_entries = [&](uint64_t extent_offset, uint64_t extent_length) { if (extent_length > 0) { if (idx < *p_num_entries) { @@ -17138,7 +17137,6 @@ int BlueStore::copy_allocator(Allocator* src_alloc, Allocator* dest_alloc, uint6 idx++; } else { - null_extent = true; derr << "zero length extent!!! offset=" << extent_offset << ", index=" << idx << dendl; } }; @@ -17147,14 +17145,9 @@ int BlueStore::copy_allocator(Allocator* src_alloc, Allocator* dest_alloc, uint6 dout(5) << "copy num_entries=" << idx << dendl; if (idx > *p_num_entries) { derr << "****spillover, num_entries=" << *p_num_entries << ", spillover=" << (idx - *p_num_entries) << dendl; - return -1; + ceph_assert(idx <= *p_num_entries); } - - if (null_extent) { - derr << "null entries were found!" << dendl; - return -1; - } - + *p_num_entries = idx; for (idx = 0; idx < *p_num_entries; idx++) { @@ -17538,6 +17531,10 @@ void BlueStore::read_allocation_from_single_onode( stats.compressed_blob_count++; } + if (blob.is_shared()) { + stats.shared_blobs_count++; + } + // process all physical extent in this blob for (auto p_extent = p_extent_vec.begin(); p_extent != p_extent_vec.end(); p_extent++) { auto offset = p_extent->offset; @@ -17549,25 +17546,28 @@ void BlueStore::read_allocation_from_single_onode( continue; } - // skip repeating extents - auto lcl_itr = lcl_extnt_map.find(offset); - if (lcl_itr != lcl_extnt_map.end()) { - // repeated extents must have the same length! - - // --Note-- - // This asserts triggers because of a corruption which was hidden until now - // It was not introduced by this PR (we merely report it now) - // Don't shoot me I'm only the messenger :-) - ceph_assert(lcl_extnt_map[offset] == length); - stats.skipped_repeated_extent++; - ceph_assert(blobs_count > 0); + if (!blob.is_shared()) { + // skip repeating extents + auto lcl_itr = lcl_extnt_map.find(offset); + // extents using shared blobs might have differnt length + if (lcl_itr != lcl_extnt_map.end() ) { + // repeated extents must have the same length! + ceph_assert(lcl_extnt_map[offset] == length); + stats.skipped_repeated_extent++; + } else { + lcl_extnt_map[offset] = length; + allocator->init_rm_free(offset, length); + stats.extent_count++; + } } else { - lcl_extnt_map[offset] = length; + // extents using shared blobs might have differnt length allocator->init_rm_free(offset, length); stats.extent_count++; } - } - } + + } // physical-extents loop + + } // logical-extents loop if (blobs_count < MAX_BLOBS_IN_ONODE) { stats.blobs_in_onode[blobs_count]++; diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h index 97125b559e846..29ff87d5ff13e 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -3487,6 +3487,7 @@ private: uint32_t collection_search = 0; uint32_t pad_limit_count = 0; + uint64_t shared_blobs_count = 0; uint64_t compressed_blob_count = 0; uint64_t spanning_blob_count = 0; uint64_t insert_count = 0; @@ -3504,6 +3505,7 @@ private: out << "==========================================================" << std::endl; out << "NCB::onode_count = " ;out.width(10);out << stats.onode_count << std::endl << "NCB::shard_count = " ;out.width(10);out << stats.shard_count << std::endl + << "NCB::shared_blobs_count = " ;out.width(10);out << stats.shared_blobs_count << std::endl << "NCB::compressed_blob_count = " ;out.width(10);out << stats.compressed_blob_count << std::endl << "NCB::spanning_blob_count = " ;out.width(10);out << stats.spanning_blob_count << std::endl << "NCB::collection search = " ;out.width(10);out << stats.collection_search << std::endl -- 2.39.5