Merge pull request ceph#62513 from xxhdx1985126/wip-seastore-transaction-manager-iterator

crimson/os/seastore: LBACursor based LBAManager/TM interfaces and the related ObjectDataHandler refactor

Reviewed-by: Samuel Just <[email protected]>

Author: Matan-B

src/common/options/crimson.yaml.in

@@ -173,13 +173,6 @@ options:
         Note this size ratio does not reflect actual memory usage, as it represents the size of evicted
         pages from A1_in queue.
   default: 0.5
-- name: seastore_obj_data_write_amplification
-  type: float
-  level: advanced
-  desc: split extent if ratio of total extent size to write size exceeds this value
-  default: 1.25
-# TODO: seastore_obj_data_write_amplification is no longer correct if 
-#       seastore_data_delta_based_overwrite is enabled. So, this should be reconsidered.
 - name: seastore_max_concurrent_transactions
   type: uint
   level: advanced

src/crimson/os/seastore/btree/btree_types.cc

@@ -4,9 +4,85 @@
 #include "crimson/os/seastore/btree/btree_types.h"
 #include "crimson/os/seastore/lba/lba_btree_node.h"
 #include "crimson/os/seastore/backref/backref_tree_node.h"
+#include "crimson/os/seastore/lba/btree_lba_manager.h"
 
 namespace crimson::os::seastore {
 
+LBACursor::base_iertr::future<> LBACursor::refresh()
+{
+  LOG_PREFIX(LBACursor::refresh);
+  return with_btree<lba::LBABtree>(
+    ctx.cache,
+    ctx,
+    [this, FNAME, c=ctx](auto &btree) {
+    c.trans.cursor_stats.num_refresh_parent_total++;
+
+    if (!parent->is_valid()) {
+      c.trans.cursor_stats.num_refresh_invalid_parent++;
+      SUBTRACET(
+	seastore_lba,
+	"cursor {} parent is invalid, re-search from scratch",
+	 c.trans, *this);
+      return btree.lower_bound(c, this->get_laddr()
+      ).si_then([this](lba::LBABtree::iterator iter) {
+	auto leaf = iter.get_leaf_node();
+	parent = leaf;
+	modifications = leaf->modifications;
+	pos = iter.get_leaf_pos();
+	if (!is_end()) {
+	  ceph_assert(!iter.is_end());
+	  ceph_assert(iter.get_key() == get_laddr());
+	  val = iter.get_val();
+	  assert(is_viewable());
+	}
+      });
+    }
+    assert(parent->is_stable() ||
+      parent->is_pending_in_trans(c.trans.get_trans_id()));
+    auto leaf = parent->cast<lba::LBALeafNode>();
+    if (leaf->is_pending_in_trans(c.trans.get_trans_id())) {
+      if (leaf->modified_since(modifications)) {
+	c.trans.cursor_stats.num_refresh_modified_viewable_parent++;
+      } else {
+	// no need to refresh
+	return base_iertr::now();
+      }
+    } else {
+      auto [viewable, l] = leaf->resolve_transaction(c.trans, key);
+      SUBTRACET(
+	seastore_lba,
+	"cursor: {} viewable: {}",
+	c.trans, *this, viewable);
+      if (!viewable) {
+	leaf = l;
+	c.trans.cursor_stats.num_refresh_unviewable_parent++;
+	parent = leaf;
+      } else {
+	assert(leaf.get() == l.get());
+	assert(leaf->is_stable());
+	return base_iertr::now();
+      }
+    }
+
+    modifications = leaf->modifications;
+    if (is_end()) {
+      pos = leaf->get_size();
+      assert(!val);
+    } else {
+      auto i = leaf->lower_bound(get_laddr());
+      pos = i.get_offset();
+      val = i.get_val();
+
+      auto iter = lba::LBALeafNode::iterator(leaf.get(), pos);
+      ceph_assert(iter.get_key() == key);
+      ceph_assert(iter.get_val() == val);
+      assert(is_viewable());
+    }
+
+    return base_iertr::now();
+  });
+}
+
 namespace lba {
 
 std::ostream& operator<<(std::ostream& out, const lba_map_val_t& v)
@@ -58,7 +134,6 @@ bool BtreeCursor<key_t, val_t>::is_viewable() const {
   }
 
   auto [viewable, state] = parent->is_viewable_by_trans(ctx.trans);
-  assert(state != CachedExtent::viewable_state_t::invalid);
   SUBTRACET(seastore_cache, "{} with viewable state {}",
             ctx.trans, *parent, state);
   return viewable;

src/crimson/os/seastore/btree/btree_types.h

@@ -198,7 +198,9 @@ struct __attribute__((packed)) backref_map_val_le_t {
  * time.
  */
 template <typename key_t, typename val_t>
-struct BtreeCursor {
+struct BtreeCursor
+  : public boost::intrusive_ref_counter<
+      BtreeCursor<key_t, val_t>, boost::thread_unsafe_counter> {
   BtreeCursor(
     op_context_t &ctx,
     CachedExtentRef parent,
@@ -253,8 +255,7 @@ struct LBACursor : BtreeCursor<laddr_t, lba::lba_map_val_t> {
   using Base = BtreeCursor<laddr_t, lba::lba_map_val_t>;
   using Base::BtreeCursor;
   bool is_indirect() const {
-    assert(!is_end());
-    return val->pladdr.is_laddr();
+    return !is_end() && val->pladdr.is_laddr();
   }
   laddr_t get_laddr() const {
     return key;
@@ -274,21 +275,27 @@ struct LBACursor : BtreeCursor<laddr_t, lba::lba_map_val_t> {
     assert(!is_indirect());
     return val->checksum;
   }
+  bool contains(laddr_t laddr) const {
+    return get_laddr() <= laddr && get_laddr() + get_length() > laddr;
+  }
   extent_ref_count_t get_refcount() const {
     assert(!is_end());
     assert(!is_indirect());
     return val->refcount;
   }
-  std::unique_ptr<LBACursor> duplicate() const {
-    return std::make_unique<LBACursor>(*this);
-  }
+
+  using base_ertr = crimson::errorator<
+    crimson::ct_error::input_output_error>;
+  using base_iertr = trans_iertr<base_ertr>;
+  base_iertr::future<> refresh();
 };
-using LBACursorRef = std::unique_ptr<LBACursor>;
+using LBACursorRef = boost::intrusive_ptr<LBACursor>;
 
 struct BackrefCursor : BtreeCursor<paddr_t, backref::backref_map_val_t> {
   using Base = BtreeCursor<paddr_t, backref::backref_map_val_t>;
   using Base::BtreeCursor;
   paddr_t get_paddr() const {
+    assert(key.is_absolute());
     return key;
   }
   laddr_t get_laddr() const {
@@ -300,7 +307,7 @@ struct BackrefCursor : BtreeCursor<paddr_t, backref::backref_map_val_t> {
     return val->type;
   }
 };
-using BackrefCursorRef = std::unique_ptr<BackrefCursor>;
+using BackrefCursorRef = boost::intrusive_ptr<BackrefCursor>;
 
 template <typename key_t, typename val_t>
 std::ostream &operator<<(

src/crimson/os/seastore/btree/fixed_kv_btree.h

@@ -286,14 +286,13 @@ class FixedKVBtree {
     }
 
     // handle_boundary() must be called before get_cursor
-    std::unique_ptr<cursor_t> get_cursor(op_context_t ctx) const {
-      assert(!is_end());
-      return std::make_unique<cursor_t>(
+    boost::intrusive_ptr<cursor_t> get_cursor(op_context_t ctx) const {
+      return new cursor_t(
         ctx,
 	leaf.node,
         leaf.node->modifications,
-        get_key(),
-        std::make_optional(get_val()),
+        is_end() ? min_max_t<node_key_t>::max : get_key(),
+        is_end() ? std::nullopt : std::make_optional(get_val()),
         leaf.pos);
     }
 
@@ -490,32 +489,38 @@ class FixedKVBtree {
   }
 
   iterator make_partial_iter(
+    op_context_t c,
+    cursor_t &cursor)
+  {
+    return make_partial_iter(
+      c,
+      cursor.parent->template cast<leaf_node_t>(),
+      cursor.key,
+      cursor.pos);
+  }
+
+  boost::intrusive_ptr<cursor_t> get_cursor(
+    op_context_t c,
+    TCachedExtentRef<leaf_node_t> leaf,
+    node_key_t key)
+  {
+    auto it = leaf->lower_bound(key);
+    assert(it != leaf->end());
+    return new cursor_t(
+      c, leaf, leaf->modifications,
+      key, it.get_val(), it.get_offset());
+  }
+
+  boost::intrusive_ptr<cursor_t> get_cursor(
     op_context_t c,
     TCachedExtentRef<leaf_node_t> leaf,
     node_key_t key,
     uint16_t pos)
   {
-    assert(leaf->is_valid());
-    assert(leaf->is_viewable_by_trans(c.trans).first);
-
-    auto depth = get_root().get_depth();
-#ifndef NDEBUG
-    auto ret = iterator(
-      depth,
-      depth == 1
-        ? iterator::state_t::FULL
-        : iterator::state_t::PARTIAL);
-#else
-    auto ret = iterator(depth);
-#endif
-    ret.leaf.node = leaf;
-    ret.leaf.pos = pos;
-    if (ret.is_end()) {
-      ceph_assert(key == min_max_t<node_key_t>::max);
-    } else {
-      ceph_assert(key == ret.get_key());
-    }
-    return ret;
+    assert(leaf->get_size() != pos);
+    auto it = leaf->iter_idx(pos);
+    assert(it.get_key() == key);
+    return new cursor_t(c, leaf, leaf->modifications, key, it.get_val(), pos);
   }
 
   /**
@@ -1358,6 +1363,35 @@ class FixedKVBtree {
 private:
   RootBlockRef root_block;
 
+  iterator make_partial_iter(
+    op_context_t c,
+    TCachedExtentRef<leaf_node_t> leaf,
+    node_key_t key,
+    uint16_t pos)
+  {
+    assert(leaf->is_valid());
+    assert(leaf->is_viewable_by_trans(c.trans).first);
+
+    auto depth = get_root().get_depth();
+#ifndef NDEBUG
+    auto ret = iterator(
+      depth,
+      depth == 1
+        ? iterator::state_t::FULL
+        : iterator::state_t::PARTIAL);
+#else
+    auto ret = iterator(depth);
+#endif
+    ret.leaf.node = leaf;
+    ret.leaf.pos = pos;
+    if (ret.is_end()) {
+      ceph_assert(key == min_max_t<node_key_t>::max);
+    } else {
+      ceph_assert(key == ret.get_key());
+    }
+    return ret;
+  }
+
   template <typename T>
   using node_position_t = typename iterator::template node_position_t<T>;
 

src/crimson/os/seastore/cache.cc

@@ -213,6 +213,26 @@ void Cache::register_metrics()
         },
         sm::description("total number of cache hits")
       ),
+      sm::make_counter(
+        "refresh_parent_total",
+        cursor_stats.num_refresh_parent_total,
+        sm::description("total number of refreshed cursors")
+      ),
+      sm::make_counter(
+        "refresh_invalid_parent",
+        cursor_stats.num_refresh_invalid_parent,
+        sm::description("total number of refreshed cursors with invalid parents")
+      ),
+      sm::make_counter(
+        "refresh_unviewable_parent",
+        cursor_stats.num_refresh_unviewable_parent,
+        sm::description("total number of refreshed cursors with unviewable parents")
+      ),
+      sm::make_counter(
+        "refresh_modified_viewable_parent",
+        cursor_stats.num_refresh_modified_viewable_parent,
+        sm::description("total number of refreshed cursors with viewable but modified parents")
+      ),
     }
   );
 
@@ -1743,6 +1763,7 @@ record_t Cache::prepare_record(
     assert(rewrite_stats.is_clear());
   }
 
+  cursor_stats.apply(t.cursor_stats);
   return record;
 }
 

src/crimson/os/seastore/cache.h

@@ -449,22 +449,12 @@ class Cache : public ExtentTransViewRetriever {
       std::forward<Func>(extent_init_func));
   }
 
-  bool is_viewable_extent_stable(
+  CachedExtentRef peek_extent_viewable_by_trans(
     Transaction &t,
     CachedExtentRef extent) final
   {
     assert(extent);
-    auto view = extent->get_transactional_view(t);
-    return view->is_stable();
-  }
-
-  bool is_viewable_extent_data_stable(
-    Transaction &t,
-    CachedExtentRef extent) final
-  {
-    assert(extent);
-    auto view = extent->get_transactional_view(t);
-    return view->is_data_stable();
+    return extent->get_transactional_view(t);
   }
 
   get_extent_iertr::future<> maybe_wait_accessible(
@@ -1670,6 +1660,7 @@ class Cache : public ExtentTransViewRetriever {
     uint64_t hit = 0;
   };
 
+  btree_cursor_stats_t cursor_stats;
   struct invalid_trans_efforts_t {
     io_stat_t read;
     io_stat_t mutate;

src/crimson/os/seastore/cached_extent.cc

@@ -104,9 +104,7 @@ void CachedExtent::set_invalid(Transaction &t) {
 
 std::pair<bool, CachedExtent::viewable_state_t>
 CachedExtent::is_viewable_by_trans(Transaction &t) {
-  if (!is_valid()) {
-    return std::make_pair(false, viewable_state_t::invalid);
-  }
+  ceph_assert(is_valid());
 
   auto trans_id = t.get_trans_id();
   if (is_pending()) {
@@ -142,8 +140,6 @@ std::ostream &operator<<(
     return out << "stable";
   case CachedExtent::viewable_state_t::pending:
     return out << "pending";
-  case CachedExtent::viewable_state_t::invalid:
-    return out << "invalid";
   case CachedExtent::viewable_state_t::stable_become_retired:
     return out << "stable_become_retired";
   case CachedExtent::viewable_state_t::stable_become_pending:

src/crimson/os/seastore/cached_extent.h

@@ -806,7 +806,6 @@ class CachedExtent
   enum class viewable_state_t {
     stable,                // viewable
     pending,               // viewable
-    invalid,               // unviewable
     stable_become_retired, // unviewable
     stable_become_pending, // unviewable
   };