From: Yingxin Cheng Date: Fri, 13 Jun 2025 09:03:24 +0000 (+0800) Subject: crimson/os/seastore: update extent states upon prepare_record() X-Git-Tag: v21.0.0~256^2~372^2~6 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=756702b011d95f7414a73f4550300bd013d46391;p=ceph.git crimson/os/seastore: update extent states upon prepare_record() So that the transactional pending states won't be leaked/shared and interfere with unrelated transactions. Signed-off-by: Yingxin Cheng --- diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index d6c8c499594..8552ecbaeea 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1306,14 +1306,13 @@ record_t Cache::prepare_record( i->prepare_commit(); if (i->is_mutation_pending()) { - i->set_io_wait(); + i->set_io_wait(CachedExtent::extent_state_t::DIRTY); // extent with EXIST_MUTATION_PENDING doesn't have // prior_instance field so skip these extents. // the existing extents should be added into Cache // during complete_commit to sync with gc transaction. commit_replace_extent(t, i, i->prior_instance); } // Note, else(is_exist_mutation_pending), add_extent() atomically - // Note, i->state become DIRTY in complete_commit() assert(i->get_version() > 0); auto final_crc = i->calc_crc32c(); @@ -1493,7 +1492,7 @@ record_t Cache::prepare_record( i->get_length(), i->get_type())); } - i->set_io_wait(); + i->set_io_wait(CachedExtent::extent_state_t::CLEAN); // Note, paddr is known until complete_commit(), // so add_extent() later. } @@ -1519,7 +1518,7 @@ record_t Cache::prepare_record( i->get_length(), i->get_type())); } - i->set_io_wait(); + i->set_io_wait(CachedExtent::extent_state_t::CLEAN); // Note, paddr is (can be) known until complete_commit(), // so add_extent() later. } @@ -1568,8 +1567,7 @@ record_t Cache::prepare_record( i->state = CachedExtent::extent_state_t::CLEAN; } else { assert(i->is_exist_mutation_pending()); - i->set_io_wait(); - // Note, i->state become DIRTY in complete_commit() + i->set_io_wait(CachedExtent::extent_state_t::DIRTY); } // exist mutation pending extents must be in t.mutated_block_list @@ -1815,8 +1813,6 @@ void Cache::complete_commit( #endif i->pending_for_transaction = TRANS_ID_NULL; i->on_initial_write(); - - i->state = CachedExtent::extent_state_t::CLEAN; i->reset_prior_instance(); DEBUGT("add extent as fresh, inline={} -- {}", t, is_inline, *i); @@ -1865,12 +1861,14 @@ void Cache::complete_commit( if (!i->is_valid()) { continue; } - assert(i->is_exist_mutation_pending() || - i->prior_instance); + assert(i->has_delta()); + assert(i->is_pending_io()); + assert(i->io_wait->from_state == CachedExtent::extent_state_t::EXIST_MUTATION_PENDING + || (i->io_wait->from_state == CachedExtent::extent_state_t::MUTATION_PENDING + && i->prior_instance)); i->on_delta_write(final_block_start); i->pending_for_transaction = TRANS_ID_NULL; i->reset_prior_instance(); - i->state = CachedExtent::extent_state_t::DIRTY; assert(i->version > 0); if (i->version == 1 || is_root_type(i->get_type())) { i->dirty_from = start_seq; diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 9d3193b9e9c..d2a03a0b9aa 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -1941,7 +1941,7 @@ private: assert(is_aligned(offset, get_block_size())); assert(is_aligned(length, get_block_size())); assert(extent->get_paddr().is_absolute()); - extent->set_io_wait(); + extent->set_io_wait(extent->state); auto old_length = extent->get_loaded_length(); load_ranges_t to_read = extent->load_ranges(offset, length); auto new_length = extent->get_loaded_length(); diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 6e284fb7fda..e3fb588f97a 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -469,7 +469,6 @@ public: out << "CachedExtent(addr=" << this << ", type=" << get_type() << ", trans=" << pending_for_transaction - << ", pending_io=" << is_pending_io() << ", version=" << version << ", dirty_from=" << dirty_from << ", modify_time=" << sea_time_point_printer_t{modify_time} @@ -481,7 +480,13 @@ public: << ", last_committed_crc=" << last_committed_crc << ", refcount=" << use_count() << ", user_hint=" << user_hint - << ", rewrite_gen=" << rewrite_gen_printer_t{rewrite_generation}; + << ", rewrite_gen=" << rewrite_gen_printer_t{rewrite_generation} + << ", pending_io="; + if (is_pending_io()) { + out << io_wait->from_state; + } else { + out << "N/A"; + } if (is_valid() && is_fully_loaded() && !is_stable_clean_pending()) { print_detail(out); } @@ -655,7 +660,7 @@ public: } bool is_pending_io() const { - return !!io_wait_promise; + return io_wait.has_value(); } /// Return journal location of oldest relevant delta, only valid while DIRTY @@ -897,25 +902,30 @@ private: /// relative address before ool write, used to update mapping std::optional prior_poffset = std::nullopt; - /// used to wait while in-progress commit completes - std::optional> io_wait_promise; + struct io_wait_t { + seastar::shared_promise<> pr; + extent_state_t from_state; + }; + std::optional io_wait; - void set_io_wait() { - ceph_assert(!io_wait_promise); - io_wait_promise = seastar::shared_promise<>(); + void set_io_wait(extent_state_t new_state) { + ceph_assert(!io_wait); + io_wait.emplace(seastar::shared_promise<>(), state); + state = new_state; + assert(is_data_stable()); } void complete_io() { - ceph_assert(io_wait_promise); - io_wait_promise->set_value(); - io_wait_promise = std::nullopt; + ceph_assert(io_wait.has_value()); + io_wait->pr.set_value(); + io_wait = std::nullopt; } seastar::future<> wait_io() { - if (!io_wait_promise) { + if (!io_wait) { return seastar::now(); } else { - return io_wait_promise->get_shared_future(); + return io_wait->pr.get_shared_future(); } } @@ -1479,7 +1489,6 @@ protected: virtual void logical_on_delta_write() {} void on_delta_write(paddr_t record_block_offset) final { - assert(is_exist_mutation_pending() || get_prior_instance()); logical_on_delta_write(); }