crimson/so/seastore: add crcs to deltas

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index e479a5ca310404c66b885c4414da82a0026d1883..a977db30db78c669f0ada486b03d433939712864 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -68,6 +68,7 @@ CachedExtentRef Cache::duplicate_for_write(
 
   auto ret = i->duplicate_for_write();
   ret->version++;
+  ret->last_committed_crc = i->last_committed_crc;
   ret->state = CachedExtent::extent_state_t::MUTATION_PENDING;
 
   if (ret->get_type() == extent_types_t::ROOT) {
@@ -109,14 +110,19 @@ std::optional<record_t> Cache::try_construct_record(Transaction &t)
     add_extent(i);
     i->prepare_write();
     i->set_io_wait();
+    assert(i->get_version() > 0);
+    auto final_crc = i->get_crc32c();
     record.deltas.push_back(
       delta_info_t{
        i->get_type(),
        i->get_paddr(),
+       i->last_committed_crc,
+       final_crc,
        (segment_off_t)i->get_length(),
-       i->get_version(),
+       i->get_version() - 1,
        i->get_delta()
       });
+    i->last_committed_crc = final_crc;
   }
 
   record.extents.reserve(t.fresh_block_list.size());
@@ -132,6 +138,8 @@ std::optional<record_t> Cache::try_construct_record(Transaction &t)
          i->get_paddr(),
          0,
          0,
+         0,
+         0,
          bufferlist()
        });
     }
@@ -154,6 +162,7 @@ void Cache::complete_commit(
     i->set_paddr(cur);
     cur.offset += i->get_length();
     i->state = CachedExtent::extent_state_t::CLEAN;
+    i->last_committed_crc = i->get_crc32c();
     logger().debug("complete_commit: fresh {}", *i);
     i->on_initial_write();
     add_extent(i);
@@ -190,6 +199,7 @@ Cache::replay_delta(paddr_t record_base, const delta_info_t &delta)
       ? record_base.add_record_relative(delta.paddr)
       : delta.paddr;
     logger().debug("replay_delta: found root addr {}", root_location);
+    root->apply_delta_and_adjust_crc(record_base, delta.bl);
     return get_extent<RootBlock>(
       root_location,
       RootBlock::SIZE

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index bd9a992804e7ab92251d1ec64ca9420d32cc3c3b..7fa91a307b0bd9d2f221790d7637ed91f2a59429 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -191,6 +191,10 @@ public:
       ref->set_io_wait();
       ref->set_paddr(offset);
       ref->state = CachedExtent::extent_state_t::CLEAN;
+
+      /* TODO: crc should be checked against LBA manager */
+      ref->last_committed_crc = ref->get_crc32c();
+
       return segment_manager.read(
        offset,
        length,

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 680f9970cbf2d73fa9c05ffc44d6ae01ff8ab41f..6e4bd650a758cecd28ed6eae40be592b1130c082 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -48,6 +48,7 @@ class CachedExtent : public boost::intrusive_ref_counter<
   } state = extent_state_t::INVALID;
   friend std::ostream &operator<<(std::ostream &, extent_state_t);
 
+  uint32_t last_committed_crc = 0;
 public:
   /**
    *  duplicate_for_write
@@ -129,11 +130,24 @@ public:
   virtual ceph::bufferlist get_delta() = 0;
 
   /**
+   * apply_delta
+   *
    * bl is a delta obtained previously from get_delta.  The versions will
    * match.  Implementation should mutate buffer based on bl.  base matches
    * the address passed on_delta_write.
+   *
+   * Implementation *must* use set_last_committed_crc to update the crc to
+   * what the crc of the buffer would have been at submission.  For physical
+   * extents that use base to adjust internal record-relative deltas, this
+   * means that the crc should be of the buffer after applying the delta,
+   * but before that adjustment.  We do it this way because the crc in the
+   * commit path does not yet know the record base address.
+   *
+   * LogicalCachedExtent overrides this method and provides a simpler
+   * apply_delta override for LogicalCachedExtent implementers.
    */
-  virtual void apply_delta(paddr_t base, const ceph::bufferlist &bl) = 0;
+  virtual void apply_delta_and_adjust_crc(
+    paddr_t base, const ceph::bufferlist &bl) = 0;
 
   /**
    * Called on dirty CachedExtent implementation after replay.
@@ -209,7 +223,7 @@ public:
   }
 
   /// Returns crc32c of buffer
-  uint32_t get_crc32c(uint32_t crc) {
+  uint32_t get_crc32c(uint32_t crc=1) {
     return ceph_crc32c(
       crc,
       reinterpret_cast<const unsigned char *>(get_bptr().c_str()),
@@ -312,6 +326,11 @@ protected:
     return new T(std::move(ptr));
   }
 
+  /// Sets last_committed_crc
+  void set_last_committed_crc(uint32_t crc) {
+    last_committed_crc = crc;
+  }
+
   void set_paddr(paddr_t offset) { poffset = offset; }
 
   /**

diff --git a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node_impl.h b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node_impl.h
index 66d85220326e25925e1dc42707376bf3e1d6edc7..1184f64527725fa3c25490c5f5b699d893d0b943 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node_impl.h
+++ b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node_impl.h
@@ -169,7 +169,8 @@ struct LBAInternalNode
     return ceph::bufferlist();
   }
 
-  void apply_delta(paddr_t delta_base, const ceph::bufferlist &bl) final {
+  void apply_delta_and_adjust_crc(
+    paddr_t base, const ceph::bufferlist &_bl) final {
     ceph_assert(0 == "TODO");
   }
 
@@ -371,7 +372,8 @@ struct LBALeafNode
     return ceph::bufferlist();
   }
 
-  void apply_delta(paddr_t delta_base, const ceph::bufferlist &bl) final {
+  void apply_delta_and_adjust_crc(
+    paddr_t base, const ceph::bufferlist &_bl) final {
     ceph_assert(0 == "TODO");
   }
 

diff --git a/src/crimson/os/seastore/onode_manager/simple-fltree/onode_block.cc b/src/crimson/os/seastore/onode_manager/simple-fltree/onode_block.cc
index d0d10747c3af969459072b7841f38f5d734c8460..1835ce895ddf2f8b25538d3e59e19689f6d2ca51 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/simple-fltree/onode_block.cc
+++ b/src/crimson/os/seastore/onode_manager/simple-fltree/onode_block.cc
@@ -26,7 +26,7 @@ void OnodeBlock::on_delta_write(paddr_t)
   apply_pending_changes(true);
 }
 
-void OnodeBlock::apply_delta(paddr_t, const ceph::bufferlist &bl)
+void OnodeBlock::apply_delta(const ceph::bufferlist &bl)
 {
   assert(deltas.empty());
 

diff --git a/src/crimson/os/seastore/onode_manager/simple-fltree/onode_block.h b/src/crimson/os/seastore/onode_manager/simple-fltree/onode_block.h
index 38a8424ebe0a39d4396893fd072a67c8cbd8bc30..892388be0c48ab927467227ae83d22498a22f570 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/simple-fltree/onode_block.h
+++ b/src/crimson/os/seastore/onode_manager/simple-fltree/onode_block.h
@@ -46,7 +46,7 @@ struct OnodeBlock final : LogicalCachedExtent {
   ceph::bufferlist get_delta() final;
   void on_initial_write() final;
   void on_delta_write(paddr_t record_block_offset) final;
-  void apply_delta(paddr_t base, const ceph::bufferlist &bl) final;
+  void apply_delta(const ceph::bufferlist &bl) final;
 
   void sync() {
     apply_pending_changes(false);

diff --git a/src/crimson/os/seastore/root_block.h b/src/crimson/os/seastore/root_block.h
index 72c0ab969138f7d7fd88833a89f633198a44a4da..5a74f10d0f86cc219fcda34e0dcd3c7a2b1ebb44 100644 (file)
--- a/src/crimson/os/seastore/root_block.h
+++ b/src/crimson/os/seastore/root_block.h
@@ -88,7 +88,7 @@ struct RootBlock : CachedExtent {
     return ceph::bufferlist();
   }
 
-  void apply_delta(paddr_t base, const ceph::bufferlist &bl) final {
+  void apply_delta_and_adjust_crc(paddr_t base, const ceph::bufferlist &_bl) final {
     ceph_assert(0 == "TODO");
   }
 

diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h
index 3a0d9922169ade8742ca6eff6d7bfde62f801067..0fcb7e4fdca842b69fb50c66be1f3591cf0fd3c9 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.h
+++ b/src/crimson/os/seastore/seastore_types.h
@@ -237,6 +237,8 @@ struct delta_info_t {
   paddr_t paddr;                               ///< physical address
   /* logical address -- needed for repopulating cache -- TODO don't actually need */
   // laddr_t laddr = L_ADDR_NULL;
+  uint32_t prev_crc;
+  uint32_t final_crc;
   segment_off_t length = NULL_SEG_OFF;         ///< extent length
   extent_version_t pversion;                   ///< prior version
   ceph::bufferlist bl;                         ///< payload
@@ -246,6 +248,8 @@ struct delta_info_t {
     denc(v.type, p);
     denc(v.paddr, p);
     //denc(v.laddr, p);
+    denc(v.prev_crc, p);
+    denc(v.final_crc, p);
     denc(v.length, p);
     denc(v.pversion, p);
     denc(v.bl, p);
@@ -256,6 +260,8 @@ struct delta_info_t {
     return (
       type == rhs.type &&
       paddr == rhs.paddr &&
+      prev_crc == rhs.prev_crc &&
+      final_crc == rhs.final_crc &&
       length == rhs.length &&
       pversion == rhs.pversion &&
       bl == rhs.bl

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 4e10f272df7c8436d38812a447f989859f34732f..814cbcf359130f6d8ec5b0f97ab6c9095254e39e 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -53,6 +53,14 @@ public:
     return pin->get_laddr();
   }
 
+  void apply_delta_and_adjust_crc(
+    paddr_t base, const ceph::bufferlist &bl) final {
+    apply_delta(bl);
+    set_last_committed_crc(get_crc32c());
+  }
+protected:
+  virtual void apply_delta(const ceph::bufferlist &bl) = 0;
+
 private:
   LBAPinRef pin;
 };

diff --git a/src/test/crimson/seastore/test_block.h b/src/test/crimson/seastore/test_block.h
index 2e9a91e4c314b956859b3a47322712189a7d332e..4d96c6fe6c314629cb539e5e4a31b3af30674090 100644 (file)
--- a/src/test/crimson/seastore/test_block.h
+++ b/src/test/crimson/seastore/test_block.h
@@ -61,7 +61,7 @@ struct TestBlock : crimson::os::seastore::LogicalCachedExtent {
     return { get_length(), get_crc32c(1) };
   }
 
-  void apply_delta(paddr_t delta_base, const ceph::bufferlist &bl) final {
+  void apply_delta(const ceph::bufferlist &bl) final {
     ceph_assert(0 == "TODO");
   }
 };

diff --git a/src/test/crimson/seastore/test_seastore_journal.cc b/src/test/crimson/seastore/test_seastore_journal.cc
index 4a901d86080df0f17e39f9536961cfe56bd8100e..ec85768f82c2fba230b141b4bbdf6196c64d6abf 100644 (file)
--- a/src/test/crimson/seastore/test_seastore_journal.cc
+++ b/src/test/crimson/seastore/test_seastore_journal.cc
@@ -182,6 +182,7 @@ struct journal_test_t : seastar_test_suite_t, JournalSegmentProvider {
     return delta_info_t{
       extent_types_t::TEST_BLOCK,
       paddr_t{},
+      0, 0,
       block_size,
       1,
       bl