crimson/os/seastore/transaction_manager: fully load extents when

remapping extents with full extent integrity check enabled

Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>

3 files changed, 59 insertions, 16 deletions

diff --git a/src/common/options/crimson.yaml.in b/src/common/options/crimson.yaml.in
index 4a0eb5627b8..7030780d9a9 100644
--- a/src/common/options/crimson.yaml.in
+++ b/src/common/options/crimson.yaml.in
@@ -83,6 +83,16 @@ options:
   level: dev
   desc: default logical address space reservation for seastore objects' metadata
   default: 16777216
+# TODO: implement sub-extent checksum and deprecate this configuration.
+- name: seastore_full_integrity_check
+  type: bool
+  level: dev
+  desc: Whether seastore need to fully check the integrity of each extent,
+        non-full integrity check means the integrity check might be skipped
+        during extent remapping for better performance, disable with caution
+  default: false
+# TODO: seastore_max_data_allocation_size should be dropped once the sub-extent
+#       read/checksum is implemented.
 - name: seastore_max_data_allocation_size
   type: size
   level: advanced
diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index f2f180f2718..581777e68f1 100644
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -34,7 +34,10 @@ TransactionManager::TransactionManager(
     lba_manager(std::move(_lba_manager)),
     journal(std::move(_journal)),
     epm(std::move(_epm)),
-    backref_manager(std::move(_backref_manager))
+    backref_manager(std::move(_backref_manager)),
+    full_extent_integrity_check(
+      crimson::common::get_conf<bool>(
+        "seastore_full_integrity_check"))
 {
   epm->set_extent_callback(this);
   journal->set_write_pipeline(&write_pipeline);
diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index bec1189e8c7..6d99bd77ec2 100644
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -426,9 +426,21 @@ public:
 #endif
 
     // The according extent might be stable or pending.
-    return cache->get_extent_if_cached(
-      t, pin->get_val(), T::TYPE
-    ).si_then([this, &t, remaps,
+    auto fut = base_iertr::make_ready_future<TCachedExtentRef<T>>();
+    if (full_extent_integrity_check) {
+      fut = read_pin<T>(t, pin->duplicate());
+    } else {
+      fut = cache->get_extent_if_cached(
+	t, pin->get_val(), T::TYPE
+      ).si_then([](auto extent) {
+	if (extent) {
+	  return extent->template cast<T>();
+	} else {
+	  return TCachedExtentRef<T>();
+	}
+      });
+    }
+    return fut.si_then([this, &t, remaps,
               original_laddr = pin->get_key(),
 	      intermediate_base = pin->is_indirect()
 				  ? pin->get_intermediate_base()
@@ -807,6 +819,8 @@ private:
 
   WritePipeline write_pipeline;
 
+  bool full_extent_integrity_check = true;
+
   rewrite_extent_ret rewrite_logical_extent(
     Transaction& t,
     LogicalCachedExtentRef extent);
@@ -860,16 +874,24 @@ private:
 	pref.link_child(&extent);
 	extent.maybe_set_intermediate_laddr(pref);
       }
-    ).si_then([FNAME, &t, pin=std::move(pin)](auto ref) mutable -> ret {
-      SUBTRACET(seastore_tm, "got extent -- {}", t, *ref);
+    ).si_then([FNAME, &t, pin=std::move(pin), this](auto ref) mutable -> ret {
+      auto crc = ref->calc_crc32c();
+      SUBTRACET(
+	seastore_tm,
+	"got extent -- {}, chksum in the lba tree: {}, actual chksum: {}",
+	t,
+	*ref,
+	pin->get_checksum(),
+	crc);
       assert(ref->is_fully_loaded());
       bool inconsistent = false;
       if (pin->is_indirect()) {
 	inconsistent = (pin->get_checksum() != 0);
-      } else {
-	inconsistent = !(pin->get_checksum() == 0 || // TODO: remapped extents may
-						     // not have recorded chksums
-			 pin->get_checksum() == ref->calc_crc32c());
+      } else if (full_extent_integrity_check) {
+	inconsistent = (pin->get_checksum() != crc);
+      } else { // !full_extent_integrity_check: remapped extent may be skipped
+	inconsistent = !(pin->get_checksum() == 0 ||
+			 pin->get_checksum() == crc);
       }
       if (unlikely(inconsistent)) {
 	SUBERRORT(seastore_tm,
@@ -920,16 +942,24 @@ private:
 	pref.link_child(&lextent);
 	lextent.maybe_set_intermediate_laddr(pref);
       }
-    ).si_then([FNAME, &t, pin=std::move(pin)](auto ref) {
-      SUBTRACET(seastore_tm, "got extent -- {}", t, *ref);
+    ).si_then([FNAME, &t, pin=std::move(pin), this](auto ref) {
+      auto crc = ref->calc_crc32c();
+      SUBTRACET(
+	seastore_tm,
+	"got extent -- {}, chksum in the lba tree: {}, actual chksum: {}",
+	t,
+	*ref,
+	pin->get_checksum(),
+	crc);
       assert(ref->is_fully_loaded());
       bool inconsistent = false;
       if (pin->is_indirect()) {
 	inconsistent = (pin->get_checksum() != 0);
-      } else {
-	inconsistent = !(pin->get_checksum() == 0 || // TODO: remapped extents may
-						     // not have recorded chksums
-			 pin->get_checksum() == ref->calc_crc32c());
+      } else if (full_extent_integrity_check) {
+	inconsistent = (pin->get_checksum() != crc);
+      } else { // !full_extent_integrity_check: remapped extent may be skipped
+	inconsistent = !(pin->get_checksum() == 0 ||
+			 pin->get_checksum() == crc);
       }
       if (unlikely(inconsistent)) {
 	SUBERRORT(seastore_tm,