Merge pull request #46193 from ljflores/wip-zero-detection-off-by-default

os/bluestore: turn bluestore zero block detection off by default

PendingReleaseNotes

@@ -43,3 +43,13 @@
   * system
   * truncated
   * user_stats_sync
+
+>=17.2.1
+
+* The "BlueStore zero block detection" feature (first introduced to Quincy in
+https://github.com/ceph/ceph/pull/43337) has been turned off by default with a
+new global configuration called `bluestore_zero_block_detection`. This feature,
+intended for large-scale synthetic testing, does not interact well with some RBD
+and CephFS features. Any side effects experienced in previous Quincy versions
+would no longer occur, provided that the configuration remains set to false.
+Relevant tracker: https://tracker.ceph.com/issues/55521

qa/config/rados.yaml

@@ -6,5 +6,6 @@ overrides:
         osd op queue cut off: debug_random
         osd debug verify missing on start: true
         osd debug verify cached snaps: true
+        bluestore zero block detection: true
       mon:
         mon scrub interval: 300

src/common/options/global.yaml.in

@@ -5438,6 +5438,17 @@ options:
   flags:
   - runtime
   with_legacy: true
+- name: bluestore_zero_block_detection
+  type: bool
+  level: dev
+  desc: punch holes instead of writing zeros
+  long_desc: Intended for large-scale synthetic testing. Currently this is implemented
+    with punch hole semantics, affecting the logical extent map of the object. This does
+    not interact well with some RBD and CephFS features.
+  default: false
+  flags:
+  - runtime
+  with_legacy: true
 - name: kstore_max_ops
   type: uint
   level: advanced

src/os/bluestore/BlueStore.cc

@@ -14604,7 +14604,7 @@ void BlueStore::_do_write_small(
     o->extent_map.punch_hole(c, offset, length, &wctx->old_extents);
 
     // Zero detection -- small block
-    if (!bl.is_zero()) {
+    if (!cct->_conf->bluestore_zero_block_detection || !bl.is_zero()) {
       BlobRef b = c->new_blob();
       _pad_zeros(&bl, &b_off0, min_alloc_size);
       wctx->write(offset, b, alloc_len, b_off0, bl, b_off, length, false, true);
@@ -14843,7 +14843,7 @@ void BlueStore::_do_write_small(
 	    o->extent_map.punch_hole(c, offset, length, &wctx->old_extents);
 
 	    // Zero detection -- small block
-	    if (!bl.is_zero()) {
+	    if (!cct->_conf->bluestore_zero_block_detection || !bl.is_zero()) {
 	      _pad_zeros(&bl, &b_off0, chunk_size);
 
 	      dout(20) << __func__ << " reuse blob " << *b << std::hex
@@ -14905,7 +14905,7 @@ void BlueStore::_do_write_small(
 	  o->extent_map.punch_hole(c, offset, length, &wctx->old_extents);
 
 	  // Zero detection -- small block
-	  if (!bl.is_zero()) {
+	  if (!cct->_conf->bluestore_zero_block_detection || !bl.is_zero()) {
 	    uint64_t chunk_size = b->get_blob().get_chunk_size(block_size);
 	    _pad_zeros(&bl, &b_off0, chunk_size);
 
@@ -14961,7 +14961,7 @@ void BlueStore::_do_write_small(
   o->extent_map.punch_hole(c, offset, length, &wctx->old_extents);
 
   // Zero detection -- small block
-  if (!bl.is_zero()) {
+  if (!cct->_conf->bluestore_zero_block_detection || !bl.is_zero()) {
     // new blob.
     BlobRef b = c->new_blob();
     _pad_zeros(&bl, &b_off0, block_size);
@@ -15294,7 +15294,7 @@ void BlueStore::_do_write_big(
     blp.copy(l, t);
 
     // Zero detection -- big block
-    if (!t.is_zero()) {
+    if (!cct->_conf->bluestore_zero_block_detection || !t.is_zero()) {
       wctx->write(offset, b, l, b_off, t, b_off, l, false, new_blob);
 
       dout(20) << __func__ << " schedule write big: 0x"

src/test/objectstore/store_test.cc

@@ -7259,8 +7259,9 @@ TEST_P(StoreTestSpecificAUSize, BlobReuseOnOverwrite) {
 }
 
 TEST_P(StoreTestSpecificAUSize, ZeroBlockDetectionSmallAppend) {
-  if (string(GetParam()) != "bluestore") {
-    return;
+  CephContext *cct = (new CephContext(CEPH_ENTITY_TYPE_CLIENT))->get();
+  if (string(GetParam()) != "bluestore" || !cct->_conf->bluestore_zero_block_detection) {
+    GTEST_SKIP() << "not bluestore or bluestore_zero_block_detection=false, skipping";
   }
 
   size_t block_size = 65536;
@@ -7334,12 +7335,12 @@ TEST_P(StoreTestSpecificAUSize, ZeroBlockDetectionSmallAppend) {
 }
 
 TEST_P(StoreTestSpecificAUSize, ZeroBlockDetectionSmallOverwrite) {
-  if (string(GetParam()) != "bluestore") {
-    return;
+  CephContext *cct = (new CephContext(CEPH_ENTITY_TYPE_CLIENT))->get();
+  if (string(GetParam()) != "bluestore" || !cct->_conf->bluestore_zero_block_detection) {
+    GTEST_SKIP() << "not bluestore or bluestore_zero_block_detection=false, skipping";
   }
   if (smr) {
-    cout << "SKIP" << std::endl;
-    return;
+    GTEST_SKIP() << "smr, skipping";
   }
 
   size_t block_size = 65536;
@@ -7435,8 +7436,9 @@ TEST_P(StoreTestSpecificAUSize, ZeroBlockDetectionSmallOverwrite) {
 }
 
 TEST_P(StoreTestSpecificAUSize, ZeroBlockDetectionBigAppend) {
-  if (string(GetParam()) != "bluestore") {
-    return;
+  CephContext *cct = (new CephContext(CEPH_ENTITY_TYPE_CLIENT))->get();
+  if (string(GetParam()) != "bluestore" || !cct->_conf->bluestore_zero_block_detection) {
+    GTEST_SKIP() << "not bluestore or bluestore_zero_block_detection=false, skipping";
   }
 
   size_t block_size = 4096;
@@ -7513,12 +7515,12 @@ TEST_P(StoreTestSpecificAUSize, ZeroBlockDetectionBigAppend) {
 }
 
 TEST_P(StoreTestSpecificAUSize, ZeroBlockDetectionBigOverwrite) {
-  if (string(GetParam()) != "bluestore") {
-    return;
+  CephContext *cct = (new CephContext(CEPH_ENTITY_TYPE_CLIENT))->get();
+  if (string(GetParam()) != "bluestore" || !cct->_conf->bluestore_zero_block_detection) {
+    GTEST_SKIP() << "not bluestore or bluestore_zero_block_detection=false, skipping";
   }
   if (smr) {
-    cout << "SKIP" << std::endl;
-    return;
+    GTEST_SKIP() << "smr, skipping";
   }
 
   size_t block_size = 4096;