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 <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index d6c8c4995946a4714903aea8554f35dba19689f0..8552ecbaeead116fe8c9508bca1770922fde16e0 100644 (file)
--- 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 9d3193b9e9cbf169cc45b8de689c212320edde09..d2a03a0b9aa6fb213eb408f37e6f24136be892a3 100644 (file)
--- 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 6e284fb7fdaab5a8d22b3b0cffef73a3ca65c748..e3fb588f97adea58b735fc739cff12c483230a77 100644 (file)
--- 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<paddr_t> prior_poffset = std::nullopt;
 
-  /// used to wait while in-progress commit completes
-  std::optional<seastar::shared_promise<>> io_wait_promise;
+  struct io_wait_t {
+    seastar::shared_promise<> pr;
+    extent_state_t from_state;
+  };
+  std::optional<io_wait_t> 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();
   }