crimson/os/seastore: distinguish between is_mutation_pending() and has_mutation()

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 03cc55781383f8c342bf8109a427af21bf638b1e..db2ff1ff0a615de3e51871aa942d56c6a733e6c9 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -1273,7 +1273,7 @@ record_t Cache::prepare_record(
     i->set_modify_time(commit_time);
     DEBUGT("mutated extent with {}B delta -- {}",
           t, delta_length, *i);
-    if (!i->is_exist_mutation_pending()) {
+    if (i->is_mutation_pending()) {
       DEBUGT("commit replace extent ... -- {}, prior={}",
             t, *i, *i->prior_instance);
 
@@ -1298,6 +1298,8 @@ record_t Cache::prepare_record(
       // the existing extents should be added into Cache
       // during complete_commit to sync with gc transaction.
       commit_replace_extent(t, i, i->prior_instance);
+    } else {
+      assert(i->is_exist_mutation_pending());
     }
 
     i->prepare_write();

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 22082a48360a069b3199ca40b9a7401ff30d9732..8775a69444a5d7d1def78d8af5a4182ae2affa14 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -439,7 +439,7 @@ public:
     if (!e.is_pending()) {
       set_prior_instance(&e);
     } else {
-      assert(e.is_mutation_pending());
+      assert(e.has_mutation());
       set_prior_instance(e.prior_instance);
     }
     e.get_bptr().copy_out(
@@ -565,12 +565,12 @@ public:
   }
 
   bool is_stable_writting() const {
-    // MUTATION_PENDING/INITIAL_WRITE_PENDING and under-io extents are already
+    // mutated/INITIAL_WRITE_PENDING and under-io extents are already
     // stable and visible, see prepare_record().
     //
     // XXX: It might be good to mark this case as DIRTY/CLEAN from the definition,
     // which probably can make things simpler.
-    return (is_mutation_pending() || is_initial_pending()) && is_pending_io();
+    return (has_mutation() || is_initial_pending()) && is_pending_io();
   }
 
   /// Returns true if extent is stable and shared among transactions
@@ -583,11 +583,15 @@ public:
   }
 
   /// Returns true if extent has a pending delta
-  bool is_mutation_pending() const {
+  bool has_mutation() const {
     return state == extent_state_t::MUTATION_PENDING
       || state == extent_state_t::EXIST_MUTATION_PENDING;
   }
 
+  bool is_mutation_pending() const {
+    return state == extent_state_t::MUTATION_PENDING;
+  }
+
   /// Returns true if extent is a fresh extent
   bool is_initial_pending() const {
     return state == extent_state_t::INITIAL_WRITE_PENDING;
@@ -636,10 +640,10 @@ public:
 
   /// Returns true if extent or prior_instance has been invalidated
   bool has_been_invalidated() const {
-    return !is_valid() || (is_mutation_pending() && !prior_instance->is_valid());
+    return !is_valid() || (prior_instance && !prior_instance->is_valid());
   }
 
-  /// Returns true if extent is a plcaeholder
+  /// Returns true if extent is a placeholder
   bool is_placeholder() const {
     return is_retired_placeholder_type(get_type());
   }
@@ -1077,7 +1081,7 @@ protected:
     if (is_initial_pending() && addr.is_record_relative()) {
       return addr.block_relative_to(get_paddr());
     } else {
-      ceph_assert(!addr.is_record_relative() || is_mutation_pending());
+      ceph_assert(!addr.is_record_relative() || has_mutation());
       return addr;
     }
   }

diff --git a/src/crimson/os/seastore/object_data_handler.h b/src/crimson/os/seastore/object_data_handler.h
index 123a7889b9157f9f672a39abf072ebae0e2bd13c..2c3e41bf4d1d825a7d76744db55caddf1154825b 100644 (file)
--- a/src/crimson/os/seastore/object_data_handler.h
+++ b/src/crimson/os/seastore/object_data_handler.h
@@ -128,7 +128,7 @@ struct ObjectDataBlock : crimson::os::seastore::LogicalChildNode {
   }
 
   void prepare_commit() final {
-    if (is_mutation_pending() || is_exist_mutation_pending()) {
+    if (has_mutation()) {
       ceph_assert(!cached_overwrites.is_empty());
       if (cached_overwrites.has_cached_bptr()) {
         set_bptr(cached_overwrites.move_cached_bptr());

diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index 3a1899b41f935d3f1e96f226abb9984413a66fe8..e902afd998419fd71cb6f7d6ebf8e0875df70408 100644 (file)
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -151,6 +151,7 @@ public:
       assert(read_set.count(ref->prior_instance->get_paddr(), extent_cmp_t{}));
       ref->reset_prior_instance();
     } else {
+      assert(ref->is_stable_written());
       // && retired_set.count(ref->get_paddr()) == 0
       // If it's already in the set, insert here will be a noop,
       // which is what we want.
@@ -285,9 +286,10 @@ public:
     assert(ref->is_exist_mutation_pending() ||
           read_set.count(ref->prior_instance->get_paddr(), extent_cmp_t{}));
     mutated_block_list.push_back(ref);
-    if (!ref->is_exist_mutation_pending()) {
+    if (ref->is_mutation_pending()) {
       write_set.insert(*ref);
     } else {
+      assert(ref->is_exist_mutation_pending());
       // already added as fresh extent in write_set
       assert(write_set.exists(*ref));
     }

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 8756feaf8e9158880c4d3ec687ac729e8a7e7320..6b93c3bb4384ea049e1131a8329da2c26b04293f 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -633,9 +633,10 @@ TransactionManager::rewrite_extent_ret TransactionManager::rewrite_extent(
     assert(extent->get_version() > 0);
     if (is_root_type(extent->get_type())) {
       // pass
-    } else if (extent->get_version() == 1 && extent->is_mutation_pending()) {
+    } else if (extent->get_version() == 1 && extent->has_mutation()) {
       t.get_rewrite_stats().account_n_dirty();
     } else {
+      // extent->get_version() > 1 or DIRTY
       t.get_rewrite_stats().account_dirty(extent->get_version());
     }
     if (epm->can_inplace_rewrite(t, extent)) {