From: myoungwon oh Date: Thu, 9 May 2024 06:25:18 +0000 (+0000) Subject: crimson/os/seastore: add is_data_stable() to allow delta-overwrite on EXIST_CLEAN X-Git-Tag: testing/wip-jcollin-testing-20240625.102731-squid~64^2~2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=5c6662175f4878985634eef135af0a7435a1ec83;p=ceph-ci.git crimson/os/seastore: add is_data_stable() to allow delta-overwrite on EXIST_CLEAN Signed-off-by: Myoungwon Oh (cherry picked from commit c20a5d8b53789fd53fb98bb784a180f4d349035a) --- diff --git a/src/crimson/os/seastore/btree/btree_range_pin.cc b/src/crimson/os/seastore/btree/btree_range_pin.cc index 1c34e995860..1e72f3da75f 100644 --- a/src/crimson/os/seastore/btree/btree_range_pin.cc +++ b/src/crimson/os/seastore/btree/btree_range_pin.cc @@ -36,6 +36,16 @@ bool BtreeNodeMapping::is_stable() const return p.is_child_stable(ctx, pos); } +template +bool BtreeNodeMapping::is_data_stable() const +{ + assert(parent); + assert(parent->is_valid()); + assert(pos != std::numeric_limits::max()); + auto &p = (FixedKVNode&)*parent; + return p.is_child_data_stable(ctx, pos); +} + template class BtreeNodeMapping; template class BtreeNodeMapping; } // namespace crimson::os::seastore diff --git a/src/crimson/os/seastore/btree/btree_range_pin.h b/src/crimson/os/seastore/btree/btree_range_pin.h index b23a50bf4ba..a2d74558733 100644 --- a/src/crimson/os/seastore/btree/btree_range_pin.h +++ b/src/crimson/os/seastore/btree/btree_range_pin.h @@ -196,6 +196,7 @@ public: get_child_ret_t get_logical_extent(Transaction&) final; bool is_stable() const final; + bool is_data_stable() const final; }; } diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h index f4e7f6239a2..14a1703abbc 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_node.h +++ b/src/crimson/os/seastore/btree/fixed_kv_node.h @@ -227,6 +227,7 @@ struct FixedKVNode : ChildableCachedExtent { } virtual bool is_child_stable(op_context_t, uint16_t pos) const = 0; + virtual bool is_child_data_stable(op_context_t, uint16_t pos) const = 0; template get_child_ret_t get_child( @@ -597,6 +598,10 @@ struct FixedKVInternalNode ceph_abort("impossible"); return false; } + bool is_child_data_stable(op_context_t, uint16_t pos) const final { + ceph_abort("impossible"); + return false; + } bool validate_stable_children() final { LOG_PREFIX(FixedKVInternalNode::validate_stable_children); @@ -986,6 +991,13 @@ struct FixedKVLeafNode // // For reserved mappings, the return values are undefined. bool is_child_stable(op_context_t c, uint16_t pos) const final { + return _is_child_stable(c, pos); + } + bool is_child_data_stable(op_context_t c, uint16_t pos) const final { + return _is_child_stable(c, pos, true); + } + + bool _is_child_stable(op_context_t c, uint16_t pos, bool data_only = false) const { auto child = this->children[pos]; if (is_reserved_ptr(child)) { return true; @@ -994,7 +1006,11 @@ struct FixedKVLeafNode ceph_assert( child->is_pending_in_trans(c.trans.get_trans_id()) || this->is_stable_written()); - return c.cache.is_viewable_extent_stable(c.trans, child); + if (data_only) { + return c.cache.is_viewable_extent_data_stable(c.trans, child); + } else { + return c.cache.is_viewable_extent_stable(c.trans, child); + } } else if (this->is_pending()) { auto key = this->iter_idx(pos).get_key(); auto &sparent = this->get_stable_for_key(key); @@ -1002,7 +1018,11 @@ struct FixedKVLeafNode auto child = sparent.children[spos]; if (is_valid_child_ptr(child)) { ceph_assert(child->is_logical()); - return c.cache.is_viewable_extent_stable(c.trans, child); + if (data_only) { + return c.cache.is_viewable_extent_data_stable(c.trans, child); + } else { + return c.cache.is_viewable_extent_stable(c.trans, child); + } } else { return true; } diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index a3becb78ee9..0bf03abf6a9 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -452,6 +452,15 @@ public: return view->is_stable(); } + bool is_viewable_extent_data_stable( + Transaction &t, + CachedExtentRef extent) + { + assert(extent); + auto view = extent->get_transactional_view(t); + return view->is_data_stable(); + } + using get_extent_ertr = base_ertr; get_extent_ertr::future get_extent_viewable_by_trans( diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index f264b3a707c..942be723231 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -436,6 +436,10 @@ public: is_pending_io()); } + bool is_data_stable() const { + return is_stable() || is_exist_clean(); + } + /// Returns true if extent has a pending delta bool is_mutation_pending() const { return state == extent_state_t::MUTATION_PENDING; @@ -1087,6 +1091,7 @@ public: // For reserved mappings, the return values are // undefined although it won't crash virtual bool is_stable() const = 0; + virtual bool is_data_stable() const = 0; virtual bool is_clone() const = 0; bool is_zero_reserved() const { return !get_val().is_real(); diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc index b6d5a86dd6d..4af71c27ae3 100644 --- a/src/crimson/os/seastore/object_data_handler.cc +++ b/src/crimson/os/seastore/object_data_handler.cc @@ -297,14 +297,13 @@ overwrite_ops_t prepare_ops_list( interval_set pre_alloc_addr_removed, pre_alloc_addr_remapped; if (delta_based_overwrite_max_extent_size) { for (auto &r : ops.to_remove) { - // TODO: Introduce LBAMapping::is_data_stable() to include EXIST_CLEAN extents - if (r->is_stable() && !r->is_zero_reserved()) { + if (r->is_data_stable() && !r->is_zero_reserved()) { pre_alloc_addr_removed.insert(r->get_key(), r->get_length()); } } for (auto &r : ops.to_remap) { - if (r.pin && r.pin->is_stable() && !r.pin->is_zero_reserved()) { + if (r.pin && r.pin->is_data_stable() && !r.pin->is_zero_reserved()) { pre_alloc_addr_remapped.insert(r.pin->get_key(), r.pin->get_length()); } } diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index c7e4121263a..783fea1724e 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -365,7 +365,7 @@ public: read_extent_ret get_mutable_extent_by_laddr(Transaction &t, laddr_t laddr, extent_len_t len) { return get_pin(t, laddr ).si_then([this, &t, len](auto pin) { - ceph_assert(pin->is_stable() && !pin->is_zero_reserved()); + ceph_assert(pin->is_data_stable() && !pin->is_zero_reserved()); ceph_assert(!pin->is_clone()); ceph_assert(pin->get_length() == len); return this->read_pin(t, std::move(pin)); diff --git a/src/test/crimson/seastore/test_object_data_handler.cc b/src/test/crimson/seastore/test_object_data_handler.cc index 2200be8a4e7..0e258b9a36c 100644 --- a/src/test/crimson/seastore/test_object_data_handler.cc +++ b/src/test/crimson/seastore/test_object_data_handler.cc @@ -800,22 +800,9 @@ TEST_P(object_data_handler_test_t, overwrite_then_read_within_transaction) { auto pin1 = remap_pin(*t, std::move(pins.front()), 4096, 8192); auto ext = get_extent(*t, base + 4096, 4096 * 2); ASSERT_TRUE(ext->is_exist_clean()); - ext = tm->get_mutable_extent(*t, ext)->cast(); - - auto l = 4096; - memset( - known_contents.c_str() + base + 4096, - 'z', - l); - bufferlist bl; - bl.append( - bufferptr( - known_contents, - base + 4096, - l)); - - ext->overwrite(0, bl); + write(*t, base + 4096, 4096, 'y'); ASSERT_TRUE(ext->is_exist_mutation_pending()); + write(*t, base + 8092, 4096, 'z'); } submit_transaction(std::move(t)); read(base + 4096, 4096);