crimson/osd: accurately judge whether to do drop_recovery_read on recovering objs

Fixes: https://tracker.ceph.com/issues/47313
Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>

diff --git a/src/crimson/osd/recovery_backend.cc b/src/crimson/osd/recovery_backend.cc
index cd8173fd519fa71b328059a83643ae34ad1927b4..deb0ca4a07b91bf6e2fc2b198e9b199caf174fc5 100644 (file)
--- a/src/crimson/osd/recovery_backend.cc
+++ b/src/crimson/osd/recovery_backend.cc
@@ -39,7 +39,9 @@ void RecoveryBackend::clean_up(ceph::os::Transaction& t,
   temp_contents.clear();
 
   for (auto& [soid, recovery_waiter] : recovering) {
-    if (recovery_waiter.obc && recovery_waiter.obc->obs.exists) {
+    if ((recovery_waiter.pi && recovery_waiter.pi->is_complete())
+       || (!recovery_waiter.pi
+         && recovery_waiter.obc && recovery_waiter.obc->obs.exists)) {
       recovery_waiter.obc->drop_recovery_read();
       recovery_waiter.interrupt(why);
     }

diff --git a/src/crimson/osd/recovery_backend.h b/src/crimson/osd/recovery_backend.h
index 49396ad8ed1001e7e4641d893216c842db49885d..20c469e0c0d6f6f720f033454e2dd53fd7b9e825 100644 (file)
--- a/src/crimson/osd/recovery_backend.h
+++ b/src/crimson/osd/recovery_backend.h
@@ -125,7 +125,7 @@ protected:
     static constexpr const char* type_name = "WaitForObjectRecovery";
 
     crimson::osd::ObjectContextRef obc;
-    PullInfo pi;
+    std::optional<PullInfo> pi;
     std::map<pg_shard_t, PushInfo> pushing;
 
     seastar::future<> wait_for_readable() {

diff --git a/src/crimson/osd/replicated_recovery_backend.cc b/src/crimson/osd/replicated_recovery_backend.cc
index a45ff1be88f89a866102894b68e855c5c5791161..0590c4336b227eb122aa722342c1a925c9c44c42 100644 (file)
--- a/src/crimson/osd/replicated_recovery_backend.cc
+++ b/src/crimson/osd/replicated_recovery_backend.cc
@@ -58,7 +58,7 @@ seastar::future<> ReplicatedRecoveryBackend::recover_object(
       });
     }).then([this, soid] {
       auto& recovery = recovering.at(soid);
-      bool error = recovery.pi.recovery_progress.error;
+      bool error = recovery.pi->recovery_progress.error;
       if (!error) {
         auto push_info = recovery.pushing.begin();
         object_stat_sum_t stat = {};
@@ -66,7 +66,7 @@ seastar::future<> ReplicatedRecoveryBackend::recover_object(
           stat = push_info->second.stat;
         } else {
           // no push happened, take pull_info's stat
-          stat = recovery.pi.stat;
+          stat = recovery.pi->stat;
         }
         pg.get_recovery_handler()->on_global_recover(soid, stat, false);
         return seastar::make_ready_future<>();
@@ -92,7 +92,8 @@ ReplicatedRecoveryBackend::maybe_pull_missing_obj(
   }
   PullOp po;
   auto& recovery_waiter = recovering.at(soid);
-  auto& pi = recovery_waiter.pi;
+  recovery_waiter.pi = std::make_optional<RecoveryBackend::PullInfo>();
+  auto& pi = *recovery_waiter.pi;
   prepare_pull(po, pi, soid, need);
   auto msg = make_message<MOSDPGPull>();
   msg->from = pg.get_pg_whoami();
@@ -242,7 +243,7 @@ seastar::future<> ReplicatedRecoveryBackend::local_recover_delete(
     return seastar::make_ready_future<>();
   }).safe_then([this, soid, epoch_to_freeze, need] {
     auto& recovery_waiter = recovering[soid];
-    auto& pi = recovery_waiter.pi;
+    auto& pi = *recovery_waiter.pi;
     pi.recovery_info.soid = soid;
     pi.recovery_info.version = need;
     return on_local_recover_persist(soid, pi.recovery_info,
@@ -250,7 +251,7 @@ seastar::future<> ReplicatedRecoveryBackend::local_recover_delete(
   }, PGBackend::load_metadata_ertr::all_same_way(
       [this, soid, epoch_to_freeze, need] (auto e) {
       auto& recovery_waiter = recovering[soid];
-      auto& pi = recovery_waiter.pi;
+      auto& pi = *recovery_waiter.pi;
       pi.recovery_info.soid = soid;
       pi.recovery_info.version = need;
       return on_local_recover_persist(soid, pi.recovery_info,
@@ -633,7 +634,7 @@ seastar::future<bool> ReplicatedRecoveryBackend::_handle_pull_response(
 
   const hobject_t &hoid = pop.soid;
   auto& recovery_waiter = recovering[hoid];
-  auto& pi = recovery_waiter.pi;
+  auto& pi = *recovery_waiter.pi;
   if (pi.recovery_info.size == (uint64_t(-1))) {
     pi.recovery_info.size = pop.recovery_info.size;
     pi.recovery_info.copy_subset.intersection_of(
@@ -698,7 +699,7 @@ seastar::future<bool> ReplicatedRecoveryBackend::_handle_pull_response(
 
        if (complete) {
          pi.stat.num_objects_recovered++;
-         pg.get_recovery_handler()->on_local_recover(pop.soid, recovering[pop.soid].pi.recovery_info,
+         pg.get_recovery_handler()->on_local_recover(pop.soid, recovering[pop.soid].pi->recovery_info,
                              false, *t);
          return seastar::make_ready_future<bool>(true);
        } else {
@@ -719,7 +720,7 @@ seastar::future<> ReplicatedRecoveryBackend::handle_pull_response(
   if (pop.version == eversion_t()) {
     // replica doesn't have it!
     pg.get_recovery_handler()->on_failed_recover({ m->from }, pop.soid,
-       get_recovering(pop.soid).pi.recovery_info.version);
+       get_recovering(pop.soid).pi->recovery_info.version);
     return seastar::make_exception_future<>(
        std::runtime_error(fmt::format(
            "Error on pushing side {} when pulling obj {}",