From afcb617dc9791aa6551a1856c89b3e8e2648eabc Mon Sep 17 00:00:00 2001
From: xie xingguo <xie.xingguo@zte.com.cn>
Date: Tue, 5 Sep 2017 20:56:32 +0800
Subject: [PATCH] osd/PrimaryLogPG: do not generate data digest for BlueStore
 by default

BlueStore enables CRC by default, so this is a dup and gains
no more benefits.

Turn this off by default, which is good for performance.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
---
 qa/standalone/scrub/osd-scrub-repair.sh     | 27 +++++++++++----------
 qa/suites/rados/basic/tasks/scrub_test.yaml |  1 +
 src/common/options.cc                       |  4 +++
 src/os/ObjectStore.h                        |  3 +++
 src/os/bluestore/BlueStore.h                |  5 +++-
 src/os/filestore/FileStore.h                |  4 +++
 src/osd/ECBackend.cc                        | 12 ++++++---
 src/osd/PrimaryLogPG.cc                     | 20 +++++++++++----
 src/osd/ReplicatedBackend.cc                | 15 +++++++++---
 9 files changed, 65 insertions(+), 26 deletions(-)

diff --git a/qa/standalone/scrub/osd-scrub-repair.sh b/qa/standalone/scrub/osd-scrub-repair.sh
index 3b94dc4769e..3e1287c962d 100755
--- a/qa/standalone/scrub/osd-scrub-repair.sh
+++ b/qa/standalone/scrub/osd-scrub-repair.sh
@@ -49,6 +49,7 @@ function run() {
     export CEPH_ARGS
     CEPH_ARGS+="--fsid=$(uuidgen) --auth-supported=none "
     CEPH_ARGS+="--mon-host=$CEPH_MON "
+    CEPH_ARGS+="--osd-skip-data-digest=false "
 
     local funcs=${@:-$(set | sed -n -e 's/^\(TEST_[0-9a-z_]*\) .*/\1/p')}
     for func in $funcs ; do
@@ -1858,7 +1859,7 @@ function corrupt_scrub_erasure() {
           "primary": false
         }
       ],
-      "selected_object_info": "3:9175b684:::EOBJ1:head(21'1 client.4179.0:1 dirty|data_digest|omap_digest s 7 uv 1 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:9175b684:::EOBJ1:head(21'1 client.4179.0:1 dirty|data_digest s 7 uv 1 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "size_mismatch_oi",
         "obj_size_oi_mismatch"
@@ -1899,7 +1900,7 @@ function corrupt_scrub_erasure() {
           "primary": false
         }
       ],
-      "selected_object_info": "3:b197b25d:::EOBJ3:head(37'3 client.4251.0:1 dirty|data_digest|omap_digest s 7 uv 3 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:b197b25d:::EOBJ3:head(37'3 client.4251.0:1 dirty|data_digest s 7 uv 3 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "missing"
       ],
@@ -2017,7 +2018,7 @@ function corrupt_scrub_erasure() {
           ]
         }
       ],
-      "selected_object_info": "3:5e723e06:::EOBJ4:head(45'6 client.4289.0:1 dirty|data_digest|omap_digest s 7 uv 6 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:5e723e06:::EOBJ4:head(45'6 client.4289.0:1 dirty|data_digest s 7 uv 6 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [],
       "errors": [
         "attr_value_mismatch",
@@ -2058,7 +2059,7 @@ function corrupt_scrub_erasure() {
           "primary": false
         }
       ],
-      "selected_object_info": "3:8549dfb5:::EOBJ5:head(65'7 client.4441.0:1 dirty|data_digest|omap_digest s 7 uv 7 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:8549dfb5:::EOBJ5:head(65'7 client.4441.0:1 dirty|data_digest s 7 uv 7 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "size_mismatch_oi",
         "obj_size_oi_mismatch"
@@ -2140,7 +2141,7 @@ EOF
           "primary": false
         }
       ],
-      "selected_object_info": "3:9175b684:::EOBJ1:head(27'1 client.4155.0:1 dirty|data_digest|omap_digest s 7 uv 1 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:9175b684:::EOBJ1:head(27'1 client.4155.0:1 dirty|data_digest s 7 uv 1 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "read_error",
         "size_mismatch_oi",
@@ -2186,7 +2187,7 @@ EOF
           "primary": false
         }
       ],
-      "selected_object_info": "3:b197b25d:::EOBJ3:head(41'3 client.4199.0:1 dirty|data_digest|omap_digest s 7 uv 3 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:b197b25d:::EOBJ3:head(41'3 client.4199.0:1 dirty|data_digest s 7 uv 3 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "missing"
       ],
@@ -2310,7 +2311,7 @@ EOF
           "primary": false
         }
       ],
-      "selected_object_info": "3:5e723e06:::EOBJ4:head(48'6 client.4223.0:1 dirty|data_digest|omap_digest s 7 uv 6 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:5e723e06:::EOBJ4:head(48'6 client.4223.0:1 dirty|data_digest s 7 uv 6 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [],
       "errors": [
         "attr_value_mismatch",
@@ -2357,7 +2358,7 @@ EOF
           "primary": false
         }
       ],
-      "selected_object_info": "3:8549dfb5:::EOBJ5:head(65'7 client.4288.0:1 dirty|data_digest|omap_digest s 7 uv 7 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:8549dfb5:::EOBJ5:head(65'7 client.4288.0:1 dirty|data_digest s 7 uv 7 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "size_mismatch_oi",
         "obj_size_oi_mismatch"
@@ -2415,7 +2416,7 @@ EOF
           "primary": false
         }
       ],
-      "selected_object_info": "3:9175b684:::EOBJ1:head(21'1 client.4179.0:1 dirty|data_digest|omap_digest s 7 uv 1 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:9175b684:::EOBJ1:head(21'1 client.4179.0:1 dirty|data_digest s 7 uv 1 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "read_error",
         "size_mismatch_oi",
@@ -2462,7 +2463,7 @@ EOF
           "primary": false
         }
       ],
-      "selected_object_info": "3:9babd184:::EOBJ2:head(29'2 client.4217.0:1 dirty|data_digest|omap_digest s 7 uv 2 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:9babd184:::EOBJ2:head(29'2 client.4217.0:1 dirty|data_digest s 7 uv 2 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "ec_hash_error"
       ],
@@ -2504,7 +2505,7 @@ EOF
           "primary": false
         }
       ],
-      "selected_object_info": "3:b197b25d:::EOBJ3:head(37'3 client.4251.0:1 dirty|data_digest|omap_digest s 7 uv 3 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:b197b25d:::EOBJ3:head(37'3 client.4251.0:1 dirty|data_digest s 7 uv 3 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "missing"
       ],
@@ -2628,7 +2629,7 @@ EOF
           ]
         }
       ],
-      "selected_object_info": "3:5e723e06:::EOBJ4:head(45'6 client.4289.0:1 dirty|data_digest|omap_digest s 7 uv 6 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:5e723e06:::EOBJ4:head(45'6 client.4289.0:1 dirty|data_digest s 7 uv 6 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [],
       "errors": [
         "attr_value_mismatch",
@@ -2674,7 +2675,7 @@ EOF
           "primary": false
         }
       ],
-      "selected_object_info": "3:8549dfb5:::EOBJ5:head(65'7 client.4441.0:1 dirty|data_digest|omap_digest s 7 uv 7 dd 2ddbf8f5 od ffffffff alloc_hint [0 0 0])",
+      "selected_object_info": "3:8549dfb5:::EOBJ5:head(65'7 client.4441.0:1 dirty|data_digest s 7 uv 7 dd 2ddbf8f5 alloc_hint [0 0 0])",
       "union_shard_errors": [
         "size_mismatch_oi",
         "ec_size_error",
diff --git a/qa/suites/rados/basic/tasks/scrub_test.yaml b/qa/suites/rados/basic/tasks/scrub_test.yaml
index d87f5bfdd35..268fd01ea43 100644
--- a/qa/suites/rados/basic/tasks/scrub_test.yaml
+++ b/qa/suites/rados/basic/tasks/scrub_test.yaml
@@ -24,5 +24,6 @@ overrides:
     conf:
       osd:
         osd deep scrub update digest min age: 0
+        osd skip data digest: false
 tasks:
 - scrub_test:
diff --git a/src/common/options.cc b/src/common/options.cc
index d8f2744e8b6..6a126227e88 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -1911,6 +1911,10 @@ std::vector<Option> get_global_options() {
     .set_default(8)
     .set_description(""),
 
+    Option("osd_skip_data_digest", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
+    .set_default(true)
+    .set_description(""),
+
     Option("osd_op_queue", Option::TYPE_STR, Option::LEVEL_ADVANCED)
     .set_default("wpq")
     .set_enum_allowed( { "wpq", "prioritized", "mclock_opclass", "mclock_client", "debug_random" } )
diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h
index 16221168ccb..cde284b6fbc 100644
--- a/src/os/ObjectStore.h
+++ b/src/os/ObjectStore.h
@@ -2030,6 +2030,9 @@ public:
   virtual void inject_mdata_error(const ghobject_t &oid) {}
 
   virtual void compact() {}
+  virtual bool has_builtin_csum() const {
+    return false;
+  }
 };
 WRITE_CLASS_ENCODER(ObjectStore::Transaction)
 WRITE_CLASS_ENCODER(ObjectStore::Transaction::TransactionData)
diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index d0de456d422..583fa58628c 100644
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -2392,7 +2392,10 @@ public:
     assert(db);
     db->compact();
   }
-  
+  bool has_builtin_csum() const override {
+    return true;
+  }
+
 private:
   bool _debug_data_eio(const ghobject_t& o) {
     if (!cct->_conf->bluestore_debug_inject_read_err) {
diff --git a/src/os/filestore/FileStore.h b/src/os/filestore/FileStore.h
index 617796c533f..9c2410f1412 100644
--- a/src/os/filestore/FileStore.h
+++ b/src/os/filestore/FileStore.h
@@ -645,6 +645,10 @@ public:
     object_map->compact();
   }
 
+  bool has_builtin_csum() const override {
+    return false;
+  }
+
   void debug_obj_on_delete(const ghobject_t &oid);
   bool debug_data_eio(const ghobject_t &oid);
   bool debug_mdata_eio(const ghobject_t &oid);
diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc
index 983f0f578eb..34b2f0f3e76 100644
--- a/src/osd/ECBackend.cc
+++ b/src/osd/ECBackend.cc
@@ -2392,8 +2392,11 @@ void ECBackend::be_deep_scrub(
   if (stride % sinfo.get_chunk_size())
     stride += sinfo.get_chunk_size() - (stride % sinfo.get_chunk_size());
   uint64_t pos = 0;
+  bool skip_data_digest = store->has_builtin_csum() &&
+    g_conf->get_val<bool>("osd_skip_data_digest");
 
-  uint32_t fadvise_flags = CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL | CEPH_OSD_OP_FLAG_FADVISE_DONTNEED;
+  uint32_t fadvise_flags = CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL |
+                           CEPH_OSD_OP_FLAG_FADVISE_DONTNEED;
 
   while (true) {
     bufferlist bl;
@@ -2412,7 +2415,9 @@ void ECBackend::be_deep_scrub(
       break;
     }
     pos += r;
-    h << bl;
+    if (!skip_data_digest) {
+      h << bl;
+    }
     if ((unsigned)r < stride)
       break;
   }
@@ -2439,7 +2444,8 @@ void ECBackend::be_deep_scrub(
 	return;
       }
 
-      if (hinfo->get_chunk_hash(get_parent()->whoami_shard().shard) != h.digest()) {
+      if (!skip_data_digest &&
+          hinfo->get_chunk_hash(get_parent()->whoami_shard().shard) != h.digest()) {
 	dout(0) << "_scan_list  " << poid << " got incorrect hash on read" << dendl;
 	o.ec_hash_mismatch = true;
 	return;
diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index 3eb6b8ff026..0f308c2b5a9 100644
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -4936,6 +4936,8 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
   ObjectState& obs = ctx->new_obs;
   object_info_t& oi = obs.oi;
   const hobject_t& soid = oi.soid;
+  bool skip_data_digest = osd->store->has_builtin_csum() &&
+    g_conf->get_val<bool>("osd_skip_data_digest");
 
   PGTransaction* t = ctx->op_t.get();
 
@@ -5749,12 +5751,18 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	    soid, op.extent.offset, op.extent.length, osd_op.indata, op.flags);
 	}
 
-	if (op.extent.offset == 0 && op.extent.length >= oi.size)
+	if (op.extent.offset == 0 && op.extent.length >= oi.size
+            && !skip_data_digest) {
 	  obs.oi.set_data_digest(osd_op.indata.crc32c(-1));
-	else if (op.extent.offset == oi.size && obs.oi.is_data_digest())
-	  obs.oi.set_data_digest(osd_op.indata.crc32c(obs.oi.data_digest));
-	else
+	} else if (op.extent.offset == oi.size && obs.oi.is_data_digest()) {
+          if (skip_data_digest) {
+            obs.oi.clear_data_digest();
+          } else {
+	    obs.oi.set_data_digest(osd_op.indata.crc32c(obs.oi.data_digest));
+          }
+	} else {
 	  obs.oi.clear_data_digest();
+        }
 	write_update_size_and_usage(ctx->delta_stats, oi, ctx->modified_ranges,
 				    op.extent.offset, op.extent.length);
 
@@ -5786,7 +5794,9 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	if (op.extent.length) {
 	  t->write(soid, 0, op.extent.length, osd_op.indata, op.flags);
 	}
-	obs.oi.set_data_digest(osd_op.indata.crc32c(-1));
+        if (!skip_data_digest) {
+	  obs.oi.set_data_digest(osd_op.indata.crc32c(-1));
+        }
 
 	write_update_size_and_usage(ctx->delta_stats, oi, ctx->modified_ranges,
 	    0, op.extent.length, true);
diff --git a/src/osd/ReplicatedBackend.cc b/src/osd/ReplicatedBackend.cc
index 45807a861ba..f14bd12459c 100644
--- a/src/osd/ReplicatedBackend.cc
+++ b/src/osd/ReplicatedBackend.cc
@@ -703,8 +703,11 @@ void ReplicatedBackend::be_deep_scrub(
   bufferlist bl, hdrbl;
   int r;
   __u64 pos = 0;
+  bool skip_data_digest = store->has_builtin_csum() &&
+    g_conf->get_val<bool>("osd_skip_data_digest");
 
-  uint32_t fadvise_flags = CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL | CEPH_OSD_OP_FLAG_FADVISE_DONTNEED;
+  uint32_t fadvise_flags = CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL |
+                           CEPH_OSD_OP_FLAG_FADVISE_DONTNEED;
 
   while (true) {
     handle.reset_tp_timeout();
@@ -718,7 +721,9 @@ void ReplicatedBackend::be_deep_scrub(
     if (r <= 0)
       break;
 
-    h << bl;
+    if (!skip_data_digest) {
+      h << bl;
+    }
     pos += bl.length();
     bl.clear();
   }
@@ -728,8 +733,10 @@ void ReplicatedBackend::be_deep_scrub(
     o.read_error = true;
     return;
   }
-  o.digest = h.digest();
-  o.digest_present = true;
+  if (!skip_data_digest) {
+    o.digest = h.digest();
+    o.digest_present = true;
+  }
 
   bl.clear();
   r = store->omap_get_header(
-- 
2.39.5