]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
os/bluestore/compression: Fix Estimator::split_and_compress 63373/head
authorAdam Kupczyk <akupczyk@ibm.com>
Tue, 20 May 2025 07:27:26 +0000 (07:27 +0000)
committerAdam Kupczyk <akupczyk@ibm.com>
Wed, 28 May 2025 17:12:15 +0000 (17:12 +0000)
Fixed calculation on effective blob size.
When fully non-compressible data is passed,
it could cause losing few bytes in the end.
Example:
 -107> 2025-05-17T20:40:50.468+0000 7f267a42f640 15 bluestore(/var/lib/ceph/osd/ceph-4) _do_write_v2_compressed 200000~78002 -> 200000~78002
 -106> 2025-05-17T20:40:50.468+0000 7f267a42f640 20 blobs to put: 200000~f000(4d61) 20f000~f000(b51) 21e000~f000(b51) 22d000~f000(b51) 23c000~f000(b51) 24b000~f000(b51) 25a000~f000(b51) 269000~f000(b51)
In result we split 0x78002 into 8 * 0xf000, losing 0x2 in the process.

Calculations for original:
>>> size=0x78002
>>> blobs=(size+0xffff) / 0x10000
>>> blob_size = size / blobs
>>> print hex(size), blobs, hex(blob_size)
0x78002 8 0xf000 <-this means roundup is 0xf000

Calculations for fixed:
>>> size=0x78002
>>> blobs=(size+0xffff) / 0x10000
>>> blob_size = (size+blobs-1) / blobs
>>> print hex(size), blobs, hex(blob_size)
0x78002 8 0xf001 <-this meand roundup is 0x10000

Fixes: https://tracker.ceph.com/issues/71353
Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>
src/os/bluestore/Compression.cc

index 3b4b5f5692f677cf3b301b33229a6202575f4ba5..45552743d7f5697b175369f36fcaa3b72d3a0242 100644 (file)
@@ -188,12 +188,11 @@ int32_t Estimator::split_and_compress(
   uint32_t size = data_bl.length();
   ceph_assert(size > 0);
   uint32_t blobs = (size + wctx->target_blob_size - 1) / wctx->target_blob_size;
-  uint32_t blob_size = p2roundup(size / blobs, au_size);
-  std::vector<uint32_t> blob_sizes(blobs);
-  for (auto& i: blob_sizes) {
-    i = std::min(size, blob_size);
-    size -= i;
-  }
+  uint32_t blob_size = p2roundup((size + blobs - 1) / blobs, au_size);
+  // dividing 'size' to 'blobs'
+  // blobs[*] = blob_size; blobs[last] = whatever remains from 'size'
+  std::vector<uint32_t> blob_sizes(blobs, blob_size);
+  blob_sizes.back() = size - blob_size * (blobs - 1);
   int32_t disk_needed = 0;
   uint32_t bl_src_off = 0;
   for (auto& i: blob_sizes) {