Merge pull request ceph#56916 from Matan-B/wip-matanb-crimson-recovery-misc

crimson/osd/replicated_backend: misc fixes

Reviewed-by: Samuel Just <[email protected]>

Author: Matan-B

src/crimson/osd/osd_operations/client_request.cc

@@ -251,7 +251,8 @@ ClientRequest::process_op(
   co_await ihref.enter_stage<interruptor>(
     client_pp(*pg).recover_missing, *this
   );
-  co_await recover_missings(pg, m->get_hobj(), snaps_need_to_recover());
+  co_await recover_missings(pg, m->get_hobj(),
+                            snaps_need_to_recover(), m->get_reqid());
 
   DEBUGDPP("{}.{}: checking already_complete",
 	   *pg, *this, this_instance_id);

src/crimson/osd/osd_operations/client_request_common.cc

@@ -19,7 +19,8 @@ InterruptibleOperation::template interruptible_future<>
 CommonClientRequest::recover_missings(
   Ref<PG> &pg,
   const hobject_t& soid,
-  std::set<snapid_t> &&snaps)
+  std::set<snapid_t> &&snaps,
+  const osd_reqid_t& reqid)
 {
   LOG_PREFIX(CommonClientRequest::recover_missings);
   if (!pg->is_primary()) {
@@ -28,17 +29,17 @@ CommonClientRequest::recover_missings(
     return seastar::now();
   }
   return do_recover_missing(
-    pg, soid.get_head()
-  ).then_interruptible([snaps=std::move(snaps), pg, soid]() mutable {
+    pg, soid.get_head(), reqid
+  ).then_interruptible([snaps=std::move(snaps), pg, soid, reqid]() mutable {
     return pg->obc_loader.with_obc<RWState::RWREAD>(
       soid.get_head(),
-      [snaps=std::move(snaps), pg, soid](auto head, auto) mutable {
+      [snaps=std::move(snaps), pg, soid, reqid](auto head, auto) mutable {
       return seastar::do_with(
 	std::move(snaps),
-	[pg, soid, head](auto &snaps) mutable {
+	[pg, soid, head, reqid](auto &snaps) mutable {
 	return InterruptibleOperation::interruptor::do_for_each(
 	  snaps,
-	  [pg, soid, head](auto &snap) mutable ->
+	  [pg, soid, head, reqid](auto &snap) mutable ->
 	  InterruptibleOperation::template interruptible_future<> {
 	  auto coid = head->obs.oi.soid;
 	  coid.snap = snap;
@@ -50,7 +51,7 @@ CommonClientRequest::recover_missings(
 	   * we skip the oid as there is no corresponding clone to recover.
 	   * See https://tracker.ceph.com/issues/63821 */
 	  if (oid) {
-	    return do_recover_missing(pg, *oid);
+	    return do_recover_missing(pg, *oid, reqid);
 	  } else {
 	    return seastar::now();
 	  }
@@ -64,19 +65,29 @@ CommonClientRequest::recover_missings(
 
 typename InterruptibleOperation::template interruptible_future<>
 CommonClientRequest::do_recover_missing(
-  Ref<PG>& pg, const hobject_t& soid)
+  Ref<PG>& pg,
+  const hobject_t& soid,
+  const osd_reqid_t& reqid)
 {
   eversion_t ver;
   assert(pg->is_primary());
-  logger().debug("{} check for recovery, {}", __func__, soid);
+  logger().debug("{} reqid {} check for recovery, {}",
+                 __func__, reqid, soid);
   if (!pg->is_unreadable_object(soid, &ver) &&
       !pg->is_degraded_or_backfilling_object(soid)) {
+    logger().debug("{} reqid {} nothing to recover {}",
+                   __func__, reqid, soid);
     return seastar::now();
   }
-  logger().debug("{} need to wait for recovery, {}", __func__, soid);
+  logger().debug("{} reqid {} need to wait for recovery, {} version {}",
+                 __func__, reqid, soid, ver);
   if (pg->get_recovery_backend()->is_recovering(soid)) {
+    logger().debug("{} reqid {} object {} version {}, already recovering",
+                   __func__, reqid, soid, ver);
     return pg->get_recovery_backend()->get_recovering(soid).wait_for_recovered();
   } else {
+    logger().debug("{} reqid {} object {} version {}, starting recovery",
+                   __func__, reqid, soid, ver);
     auto [op, fut] =
       pg->get_shard_services().start_operation<UrgentRecovery>(
         soid, ver, pg, pg->get_shard_services(), pg->get_osdmap_epoch());

src/crimson/osd/osd_operations/client_request_common.h

@@ -15,10 +15,14 @@ struct CommonClientRequest {
   recover_missings(
     Ref<PG> &pg,
     const hobject_t& soid,
-    std::set<snapid_t> &&snaps);
+    std::set<snapid_t> &&snaps,
+    const osd_reqid_t& reqid);
 
   static InterruptibleOperation::template interruptible_future<>
-  do_recover_missing(Ref<PG>& pg, const hobject_t& soid);
+  do_recover_missing(
+    Ref<PG>& pg,
+    const hobject_t& soid,
+    const osd_reqid_t& reqid);
 
   static bool should_abort_request(
     const crimson::Operation& op, std::exception_ptr eptr);

src/crimson/osd/osd_operations/internal_client_request.cc

@@ -69,7 +69,7 @@ seastar::future<> InternalClientRequest::start()
           return enter_stage<interruptor>(
             client_pp().recover_missing);
         }).then_interruptible([this] {
-          return do_recover_missing(pg, get_target_oid());
+          return do_recover_missing(pg, get_target_oid(), osd_reqid_t());
         }).then_interruptible([this] {
           return enter_stage<interruptor>(
             client_pp().get_obc);

src/crimson/osd/replicated_backend.cc

@@ -44,7 +44,7 @@ ReplicatedBackend::_submit_transaction(std::set<pg_shard_t>&& pg_shards,
 				       std::vector<pg_log_entry_t>&& log_entries)
 {
   LOG_PREFIX(ReplicatedBackend::_submit_transaction);
-  DEBUGDPP("object {}, {}", dpp, hoid);
+  DEBUGDPP("object {}", dpp, hoid);
 
   const ceph_tid_t tid = shard_services.get_tid();
   auto pending_txn =

src/crimson/osd/replicated_recovery_backend.cc

@@ -401,7 +401,7 @@ ReplicatedRecoveryBackend::prep_push(
   push_info.recovery_info.ss = push_info_ss;
   push_info.recovery_info.soid = soid;
   push_info.recovery_info.oi = obc->obs.oi;
-  push_info.recovery_info.version = obc->obs.oi.version;
+  push_info.recovery_info.version = need;
   push_info.recovery_info.object_exist =
     missing_iter->second.clean_regions.object_is_exist();
   push_info.recovery_progress.omap_complete =