crimson/os/seastore: don't set INVALID extents to CLEAN when reading

extents

CLEAN_PENDING extents may be invalidated before read completes:

1. transaction A retired an laddr, which lead to a RetirePlaceHolder in
   Cache
2. transaction B try to read that extent, and replace A's
   RetirePlaceHolder with it;
3. transaction A commits and invalidate that extent;
4. transaction B complete reading that extent;

In this case, we shouldn't set the extent's state to CLEAN

Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>
(cherry picked from commit 729dabd8ff20a747e02c15113e005a6d61acc8d7)

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 4fbdfdaa637ee66f8289589ad1fc3fae158cce3c..9652f27a4313758461a6b3a868f3e7bb096f6f67 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -1311,11 +1311,15 @@ private:
     ).safe_then(
       [extent=std::move(extent)]() mutable {
         LOG_PREFIX(Cache::read_extent);
-        extent->state = CachedExtent::extent_state_t::CLEAN;
-        /* TODO: crc should be checked against LBA manager */
-        extent->last_committed_crc = extent->get_crc32c();
-
-        extent->on_clean_read();
+       if (likely(extent->state == CachedExtent::extent_state_t::CLEAN_PENDING)) {
+         extent->state = CachedExtent::extent_state_t::CLEAN;
+         /* TODO: crc should be checked against LBA manager */
+         extent->last_committed_crc = extent->get_crc32c();
+
+         extent->on_clean_read();
+       } else {
+         ceph_assert(!extent->is_valid());
+       }
         extent->complete_io();
         SUBDEBUG(seastore_cache, "read extent done -- {}", *extent);
         return get_extent_ertr::make_ready_future<TCachedExtentRef<T>>(