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 <gbenhano@redhat.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 94e61f7d9ab386a2a4f3a000bf32243ac774621e..b6a6e333828f7227b35ee6f2f353a85d63a2bacf 100644 (file)
--- 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 97125b559e84659cab93f5af173089fee8cf6e9c..29ff87d5ff13ea2b18177154b04fc0595bb21448 100644 (file)
--- 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