crimson/seastore: fix cache::get_extent got retired extent

one transaction got an extent whose state is MUTATION_PENDINGat that time.
but another transaction do split and set the extent state to RETIRED.
when the first transaction resume and do continuation, the state of the extent
has been changed to RETIRED. So need eagain to try again.

Signed-off-by: chunmei-liu <chunmei.liu@intel.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 139dc104211aa7ade2f42dd45c58595f23ce0dc9..0793c7deb2963b88617e69a5629fbcd9ad538dff 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -123,7 +123,7 @@ void Cache::retire_extent(CachedExtentRef ref)
   remove_from_dirty(ref);
   ref->dirty_from_or_retired_at = JOURNAL_SEQ_MAX;
   retired_extent_gate.add_extent(*ref);
-  ref->state = CachedExtent::extent_state_t::RETIRED;
+  ref->state = CachedExtent::extent_state_t::INVALID;
 }
 
 void Cache::replace_extent(CachedExtentRef next, CachedExtentRef prev)

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 01b59c91c467b95f19fb2ed28a9459b82c49dcc1..0f3e260f3c143e64884049f230cf2eac3fdb7af9 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -185,8 +185,6 @@ public:
           return get_extent_ret<T>(
             get_extent_ertr::ready_future_marker{},
             std::move(ret));
-        } else if (ret->is_retired()) {
-          ceph_abort_msg("impossible retired extent");
         } else {
           return crimson::ct_error::eagain::make();
        }

diff --git a/src/crimson/os/seastore/cached_extent.cc b/src/crimson/os/seastore/cached_extent.cc
index ee0a52ceb69c4fedf7f19f0c22ef5d65d8213ddc..4c183589bd51682db2f8bc3cb57349328d2dc8e7 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.cc
+++ b/src/crimson/os/seastore/cached_extent.cc
@@ -46,8 +46,6 @@ std::ostream &operator<<(std::ostream &out, CachedExtent::extent_state_t state)
     return out << "CLEAN";
   case CachedExtent::extent_state_t::DIRTY:
     return out << "DIRTY";
-  case CachedExtent::extent_state_t::RETIRED:
-    return out << "RETIRED";
   case CachedExtent::extent_state_t::INVALID:
     return out << "INVALID";
   default:

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 5dd80b7c45918e8a196981fa589404cc981f7bc1..a14e835fb6f1674f79a926e1170722edcf11ea34 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -48,7 +48,6 @@ class CachedExtent : public boost::intrusive_ref_counter<
                            //  during write, contents match disk, version == 0
     DIRTY,                 // Same as CLEAN, but contents do not match disk,
                            //  version > 0
-    RETIRED,               // In ExtentIndex while in retired_extent_gate
     INVALID                // Part of no ExtentIndex set
   } state = extent_state_t::INVALID;
   friend std::ostream &operator<<(std::ostream &, extent_state_t);
@@ -225,12 +224,7 @@ public:
 
   /// Returns true if extent has not been superceded or retired
   bool is_valid() const {
-    return state != extent_state_t::INVALID && state != extent_state_t::RETIRED;
-  }
-
-  /// True iff extent is in state RETIRED
-  bool is_retired() const {
-    return state == extent_state_t::RETIRED;
+    return state != extent_state_t::INVALID;
   }
 
   /// Returns true if extent or prior_instance has been invalidated
@@ -246,7 +240,7 @@ public:
 
   /// Return journal location of oldest relevant delta, only valid while RETIRED
   auto get_retired_at() const {
-    ceph_assert(is_retired());
+    ceph_assert(!is_valid());
     return dirty_from_or_retired_at;
   }
 
@@ -376,7 +370,7 @@ protected:
     poffset(other.poffset) {}
 
   struct retired_placeholder_t{};
-  CachedExtent(retired_placeholder_t) : state(extent_state_t::RETIRED) {}
+  CachedExtent(retired_placeholder_t) : state(extent_state_t::INVALID) {}
 
   friend class Cache;
   template <typename T, typename... Args>

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h
index 991b6004fba368c2a5ea7d763bcc316d7d663839..51d4b9d7e53e6da96a45b1c9a22495c26ec4a354 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h
@@ -339,7 +339,6 @@ class NodeExtentAccessorT {
 
   bool is_retired() const {
     if (extent) {
-      assert(!extent->is_retired());
       return false;
     } else {
       return true;

diff --git a/src/test/crimson/seastore/test_object_data_handler.cc b/src/test/crimson/seastore/test_object_data_handler.cc
index 45e941ea8d2005bdfaf65dd05f8260dbea6d4a12..a5777bd4f1c7902e2d26f814918ab7968b6bdf59 100644 (file)
--- a/src/test/crimson/seastore/test_object_data_handler.cc
+++ b/src/test/crimson/seastore/test_object_data_handler.cc
@@ -134,6 +134,7 @@ struct object_data_handler_test_t:
   seastar::future<> set_up_fut() final {
     onode = new TestOnode{};
     known_contents = buffer::create(4<<20 /* 4MB */);
+    memset(known_contents.c_str(), 0, known_contents.length());
     size = 0;
     return tm_setup();
   }