From: chunmei-liu <chunmei.liu@intel.com>
Date: Thu, 10 Jun 2021 03:37:07 +0000 (-0700)
Subject: crimson/seastore: fix cache::get_extent got retired extent
X-Git-Tag: v17.1.0~1649^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F41801%2Fhead;p=ceph.git

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 139dc104211a..0793c7deb296 100644
--- 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 01b59c91c467..0f3e260f3c14 100644
--- 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 ee0a52ceb69c..4c183589bd51 100644
--- 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 5dd80b7c4591..a14e835fb6f1 100644
--- 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 991b6004fba3..51d4b9d7e53e 100644
--- 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 45e941ea8d20..a5777bd4f1c7 100644
--- 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();
   }