crimson/seastore: skip LBA-leaf crc check for in-place delta-overwrite extents

on a cold read, check_full_extent_integrity() compares the crc of the read
content against the crc stored in the LBA leaf (pin_crc). this aborts on a
RANDOM_BLOCK data extent that was overwritten via delta-based overwrite:

  abort(extent checksum inconsistent)
   4# Cache::check_full_extent_integrity()
   5# Cache::_read_extent()::{lambda}
   7# Cache::replay_delta()  (or _read_pin on the live IO path)

the leaf crc is not a valid reference for these extents. delta overwrite
mutates the extent in place to avoid touching the LBA tree, so
update_lba_mappings() skips the leaf for is_exist_mutation_pending() extents
and the pre-mutation crc stays there. the new content lands on disk later and
out-of-band, when the cleaner rewrites the modified region in place
(RandomBlockOolWriter::do_write). that write is not atomic with the leaf, so a
cold read can pick up new content against an old leaf crc.

updating the leaf crc on overwrite is not the fix. it brings back the LBA-tree
mutation (and the txn conflicts and hot-path crc) that delta overwrite exists
to avoid, and it still does not make the leaf reliable: the data write and the
leaf update happen separately, so publishing leaf=new while disk still holds
old just moves the mismatch.

so stop trusting the leaf for these extents. their content is verified at
replay, where replay_delta() applies the deltas and
CircularBoundedJournal::replay() asserts last_committed_crc == delta.final_crc.
and the content on disk is always current: modified_region accumulates every
change via union_insert and is cleared only when the in-place rewrite commits,
so do_write writes the latest content; only the leaf crc lags.

drop the leaf-crc check on read for RANDOM_BLOCK in-place-rewritable (data)
extents, but only when delta-based overwrite is enabled. with it off (the
default), overwrites go through remapping, the leaf crc stays valid, and the
check still runs for every other extent. last_committed_crc is computed
regardless, so the delta-chain check is unaffected.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index be90decca48a62f62c9d39fc9e4726f6aef16d85..9500ffdbc61d048ea68926ae6c8996be19e190b4 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -33,6 +33,9 @@ Cache::Cache(
   ExtentPlacementManager &epm,
   store_index_t store_index)
   : epm(epm),
+    delta_based_overwrite_enabled(
+      crimson::common::get_conf<Option::size_t>(
+        "seastore_data_delta_based_overwrite") > 0),
     pinboard(create_extent_pinboard(
       crimson::common::get_conf<Option::size_t>(
        "seastore_cachepin_size_pershard")))

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 24aebba12f229ae87d538b8034b2da188077e3fc..c878ad6d51f1b3be7e5b2eade0001483161fec34 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -1731,6 +1731,9 @@ private:
   }
 
   ExtentPlacementManager& epm;
+  // when delta-based overwrite is enabled, RANDOM_BLOCK data extents may be
+  // mutated in place, leaving their LBA-leaf crc stale; see _read_extent().
+  const bool delta_based_overwrite_enabled;
   RootBlockRef root;               ///< ref to current root
   ExtentIndex extents_index;             ///< set of live extents
 
@@ -2062,14 +2065,26 @@ void stage_visibility_handoff(Transaction& t,
               offset, length, *extent);
 
             if (pin_crc != CRC_NULL) {
-              SUBDEBUG(seastore_cache, "read extent 0x{:x}~0x{:x} veryfing integrity -- {}",
-                offset, length, *extent);
-              // We must check the integrity here prior to complete_io.
-              // Previously, concurrent transaction could have checked
-              // crc of non matching extent data.
-              // See: https://tracker.ceph.com/issues/73790
-              assert(extent->is_fully_loaded());
-              check_full_extent_integrity(extent->last_committed_crc, pin_crc);
+              // a RANDOM_BLOCK data extent that delta-based overwrite may have
+              // mutated in place has a stale LBA-leaf crc: its content and its
+              // leaf crc are updated through separate, non-atomic paths, so a
+              // cold read can see the new content against the old leaf crc.
+              // such extents are verified via the delta chain instead (see
+              // replay_delta()), so skip the leaf-crc check for them.
+              bool inplace_delta_overwrite =
+                delta_based_overwrite_enabled &&
+                extent->get_paddr().is_absolute_random_block() &&
+                can_inplace_rewrite(extent->get_type());
+              if (!inplace_delta_overwrite) {
+                SUBDEBUG(seastore_cache, "read extent 0x{:x}~0x{:x} verifying integrity -- {}",
+                  offset, length, *extent);
+                // We must check the integrity here prior to complete_io.
+                // Previously, concurrent transaction could have checked
+                // crc of non matching extent data.
+                // See: https://tracker.ceph.com/issues/73790
+                assert(extent->is_fully_loaded());
+                check_full_extent_integrity(extent->last_committed_crc, pin_crc);
+              }
             }
           } else {
             extent->last_committed_crc = CRC_NULL;