crimson/osd: errororize PG::with_*_obc methods()

they all use `seastar::with_lock()` to ensure the obc lock is released
in the error handling path. but we cannot assume that
`seastar::with_lock()` always return `seastar::future<>`, so propogate
the error returned by the function passed to `with_lock()` using
errorator.

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 7598326379c77cd8dd6a4553a084a32e3c67d9e5..5f8129c62ca1cbfac98921b929104d3aa62ec42d 100644 (file)
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -795,7 +795,7 @@ std::optional<hobject_t> PG::resolve_oid(
 }
 
 template<RWState::State State>
-seastar::future<>
+PG::load_obc_ertr::future<>
 PG::with_head_obc(hobject_t oid, with_obc_func_t&& func)
 {
   assert(oid.is_head());
@@ -803,7 +803,7 @@ PG::with_head_obc(hobject_t oid, with_obc_func_t&& func)
   return obc->with_lock<State>(
     [oid=std::move(oid), existed=existed, obc=std::move(obc),
      func=std::move(func), this] {
-    auto loaded = seastar::make_ready_future<ObjectContextRef>(obc);
+    auto loaded = load_obc_ertr::make_ready_future<ObjectContextRef>(obc);
     if (existed) {
       logger().debug("with_head_obc: found {} in cache", oid);
     } else {
@@ -812,30 +812,31 @@ PG::with_head_obc(hobject_t oid, with_obc_func_t&& func)
         return load_head_obc(obc);
       });
     }
-    return loaded.then([func = std::move(func)](auto obc) {
+    return loaded.safe_then([func=std::move(func)](auto obc) {
       return func(std::move(obc));
     });
   });
 }
 
 template<RWState::State State>
-seastar::future<>
+PG::load_obc_ertr::future<>
 PG::with_clone_obc(hobject_t oid, with_obc_func_t&& func)
 {
   assert(!oid.is_head());
   return with_head_obc<RWState::RWREAD>(oid.get_head(),
-    [oid, func=std::move(func), this](auto head) {
+    [oid, func=std::move(func), this](auto head) -> load_obc_ertr::future<> {
     auto coid = resolve_oid(head->get_ro_ss(), oid);
     if (!coid) {
       // TODO: return crimson::ct_error::enoent::make();
       logger().error("with_clone_obc: {} clone not found", coid);
-      return seastar::make_ready_future<>();
+      return load_obc_ertr::make_ready_future<>();
     }
     auto [clone, existed] = shard_services.obc_registry.get_cached_obc(*coid);
     return clone->template with_lock<State>(
       [coid=*coid, existed=existed,
-      head=std::move(head), clone=std::move(clone), func=std::move(func), this] {
-      auto loaded = seastar::make_ready_future<ObjectContextRef>(clone);
+       head=std::move(head), clone=std::move(clone),
+       func=std::move(func), this]() -> load_obc_ertr::future<> {
+      auto loaded = load_obc_ertr::make_ready_future<ObjectContextRef>(clone);
       if (existed) {
         logger().debug("with_clone_obc: found {} in cache", coid);
       } else {
@@ -849,7 +850,7 @@ PG::with_clone_obc(hobject_t oid, with_obc_func_t&& func)
           });
         });
       }
-      return loaded.then([func = std::move(func)](auto clone) {
+      return loaded.safe_then([func=std::move(func)](auto clone) {
         return func(std::move(clone));
       });
     });
@@ -857,7 +858,7 @@ PG::with_clone_obc(hobject_t oid, with_obc_func_t&& func)
 }
 
 // explicitly instantiate the used instantiations
-template seastar::future<>
+template PG::load_obc_ertr::future<>
 PG::with_head_obc<RWState::RWNONE>(hobject_t, with_obc_func_t&&);
 
 PG::load_obc_ertr::future<crimson::osd::ObjectContextRef>

diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
index bccb74b198a09c9b5255de445ee4ffb1526ed4fa..d81d81f8f7939d08fa186dc1369c7df889e67712 100644 (file)
--- a/src/crimson/osd/pg.h
+++ b/src/crimson/osd/pg.h
@@ -501,10 +501,11 @@ public:
   load_head_obc(ObjectContextRef obc);
 
 public:
-  using with_obc_func_t = std::function<seastar::future<> (ObjectContextRef)>;
+  using with_obc_func_t =
+    std::function<load_obc_ertr::future<> (ObjectContextRef)>;
 
   template<RWState::State State>
-  seastar::future<> with_head_obc(hobject_t oid, with_obc_func_t&& func);
+  load_obc_ertr::future<> with_head_obc(hobject_t oid, with_obc_func_t&& func);
 
   load_obc_ertr::future<> with_locked_obc(
     Ref<MOSDOp> &m,
@@ -521,7 +522,7 @@ public:
 
 private:
   template<RWState::State State>
-  seastar::future<> with_clone_obc(hobject_t oid, with_obc_func_t&& func);
+  load_obc_ertr::future<> with_clone_obc(hobject_t oid, with_obc_func_t&& func);
 
   load_obc_ertr::future<ObjectContextRef> get_locked_obc(
     Operation *op,

diff --git a/src/crimson/osd/replicated_recovery_backend.cc b/src/crimson/osd/replicated_recovery_backend.cc
index edd2513e97ae04aad5e50e4f5040e84f065918e7..42fc442ec435bbc51bdfb96538a0da011cb65c9a 100644 (file)
--- a/src/crimson/osd/replicated_recovery_backend.cc
+++ b/src/crimson/osd/replicated_recovery_backend.cc
@@ -30,20 +30,20 @@ seastar::future<> ReplicatedRecoveryBackend::recover_object(
     [this, soid, need](auto& pops, auto& shards) {
     return maybe_pull_missing_obj(soid, need).then([this, soid](bool pulled) {
       return load_obc_for_recovery(soid, pulled);
-    }).then([this, soid, need, &pops, &shards] {
+    }).safe_then([this, soid, need, &pops, &shards] {
       if (!shards.empty()) {
        return prep_push(soid, need, &pops, shards);
       } else {
        return seastar::now();
       }
-    }).handle_exception([this, soid](auto e) {
+    }, crimson::ct_error::all_same_way([this, soid](const std::error_code& e) {
       auto recovery_waiter = recovering.find(soid);
       if (auto obc = recovery_waiter->second.obc; obc) {
         obc->drop_recovery_read();
       }
       recovering.erase(recovery_waiter);
       return seastar::make_exception_future<>(e);
-    }).then([this, &pops, &shards, soid] {
+    })).then([this, &pops, &shards, soid] {
       return seastar::parallel_for_each(shards,
        [this, &pops, soid](auto shard) {
        auto msg = make_message<MOSDPGPush>();
@@ -114,13 +114,14 @@ ReplicatedRecoveryBackend::maybe_pull_missing_obj(
   });
 }
 
-seastar::future<> ReplicatedRecoveryBackend::load_obc_for_recovery(
+auto ReplicatedRecoveryBackend::load_obc_for_recovery(
   const hobject_t& soid,
-  bool pulled)
+  bool pulled) ->
+  load_obc_ertr::future<>
 {
   auto& recovery_waiter = recovering.at(soid);
   if (recovery_waiter.obc) {
-    return seastar::now();
+    return load_obc_ertr::now();
   }
   return pg.with_head_obc<RWState::RWREAD>(soid, [&recovery_waiter](auto obc) {
     logger().debug("load_obc_for_recovery: loaded obc: {}", obc->obs.oi.soid);
@@ -649,8 +650,8 @@ seastar::future<bool> ReplicatedRecoveryBackend::_handle_pull_response(
         recovery_waiter.obc = obc;
         obc->obs.oi.decode(pop.attrset[OI_ATTR]);
         pi.recovery_info.oi = obc->obs.oi;
-        return seastar::make_ready_future<>();
-      });
+        return crimson::osd::PG::load_obc_ertr::now();
+      }).handle_error(crimson::ct_error::assert_all{});
   };
   return prepare_waiter.then([this, first=pi.recovery_progress.first,
                              &pi, &pop, t, response]() mutable {

diff --git a/src/crimson/osd/replicated_recovery_backend.h b/src/crimson/osd/replicated_recovery_backend.h
index 4bb353c5aa0dc32570683f4517cccebc9265e9d4..919aa3a703bae110f95a6fe00c8733e243f47967 100644 (file)
--- a/src/crimson/osd/replicated_recovery_backend.h
+++ b/src/crimson/osd/replicated_recovery_backend.h
@@ -117,7 +117,9 @@ private:
     eversion_t need);
 
   /// load object context for recovery if it is not ready yet
-  seastar::future<> load_obc_for_recovery(
+  using load_obc_ertr = crimson::errorator<
+    crimson::ct_error::object_corrupted>;
+  load_obc_ertr::future<> load_obc_for_recovery(
     const hobject_t& soid,
     bool pulled);