crimson/osd/pg_recovery: avoiding duplicated object recovering

UrgentRecovery and other recoveries may collide when doing
`PGRecovery::add_recovering`, this is not an error. We should allow this
to happen

Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>

diff --git a/src/crimson/osd/pg_recovery.cc b/src/crimson/osd/pg_recovery.cc
index 09b45779ec879fda799bf41db2b8152b312de7a3..efbbf7e4f3ee19a5a3d12209f5ffba04547e659a 100644 (file)
--- a/src/crimson/osd/pg_recovery.cc
+++ b/src/crimson/osd/pg_recovery.cc
@@ -266,20 +266,27 @@ PGRecovery::recover_missing(
   RecoveryBackend::RecoveryBlockingEvent::TriggerI& trigger,
   const hobject_t &soid, eversion_t need)
 {
-  if (pg->get_peering_state().get_missing_loc().is_deleted(soid)) {
-    return pg->get_recovery_backend()->add_recovering(soid).wait_track_blocking(
-      trigger,
-      pg->get_recovery_backend()->recover_delete(soid, need));
+  logger().info("{} {} v {}", __func__, soid, need);
+  auto [recovering, added] = pg->get_recovery_backend()->add_recovering(soid);
+  if (added) {
+    logger().info("{} {} v {}, new recovery", __func__, soid, need);
+    if (pg->get_peering_state().get_missing_loc().is_deleted(soid)) {
+      return recovering.wait_track_blocking(
+       trigger,
+       pg->get_recovery_backend()->recover_delete(soid, need));
+    } else {
+      return recovering.wait_track_blocking(
+       trigger,
+       pg->get_recovery_backend()->recover_object(soid, need)
+       .handle_exception_interruptible(
+         [=, this, soid = std::move(soid)] (auto e) {
+         on_failed_recover({ pg->get_pg_whoami() }, soid, need);
+         return seastar::make_ready_future<>();
+       })
+      );
+    }
   } else {
-    return pg->get_recovery_backend()->add_recovering(soid).wait_track_blocking(
-      trigger,
-      pg->get_recovery_backend()->recover_object(soid, need)
-      .handle_exception_interruptible(
-       [=, this, soid = std::move(soid)] (auto e) {
-       on_failed_recover({ pg->get_pg_whoami() }, soid, need);
-       return seastar::make_ready_future<>();
-      })
-    );
+    return recovering.wait_for_recovered();
   }
 }
 
@@ -288,16 +295,23 @@ RecoveryBackend::interruptible_future<> PGRecovery::prep_object_replica_deletes(
   const hobject_t& soid,
   eversion_t need)
 {
-  return pg->get_recovery_backend()->add_recovering(soid).wait_track_blocking(
-    trigger,
-    pg->get_recovery_backend()->push_delete(soid, need).then_interruptible(
-      [=, this] {
-      object_stat_sum_t stat_diff;
-      stat_diff.num_objects_recovered = 1;
-      on_global_recover(soid, stat_diff, true);
-      return seastar::make_ready_future<>();
-    })
-  );
+  logger().info("{} {} v {}", __func__, soid, need);
+  auto [recovering, added] = pg->get_recovery_backend()->add_recovering(soid);
+  if (added) {
+    logger().info("{} {} v {}, new recovery", __func__, soid, need);
+    return recovering.wait_track_blocking(
+      trigger,
+      pg->get_recovery_backend()->push_delete(soid, need).then_interruptible(
+       [=, this] {
+       object_stat_sum_t stat_diff;
+       stat_diff.num_objects_recovered = 1;
+       on_global_recover(soid, stat_diff, true);
+       return seastar::make_ready_future<>();
+      })
+    );
+  } else {
+    return recovering.wait_for_recovered();
+  }
 }
 
 RecoveryBackend::interruptible_future<> PGRecovery::prep_object_replica_pushes(
@@ -305,15 +319,22 @@ RecoveryBackend::interruptible_future<> PGRecovery::prep_object_replica_pushes(
   const hobject_t& soid,
   eversion_t need)
 {
-  return pg->get_recovery_backend()->add_recovering(soid).wait_track_blocking(
-    trigger,
-    pg->get_recovery_backend()->recover_object(soid, need)
-    .handle_exception_interruptible(
-      [=, this, soid = std::move(soid)] (auto e) {
-      on_failed_recover({ pg->get_pg_whoami() }, soid, need);
-      return seastar::make_ready_future<>();
-    })
-  );
+  logger().info("{} {} v {}", __func__, soid, need);
+  auto [recovering, added] = pg->get_recovery_backend()->add_recovering(soid);
+  if (added) {
+    logger().info("{} {} v {}, new recovery", __func__, soid, need);
+    return recovering.wait_track_blocking(
+      trigger,
+      pg->get_recovery_backend()->recover_object(soid, need)
+      .handle_exception_interruptible(
+       [=, this, soid = std::move(soid)] (auto e) {
+       on_failed_recover({ pg->get_pg_whoami() }, soid, need);
+       return seastar::make_ready_future<>();
+      })
+    );
+  } else {
+    return recovering.wait_for_recovered();
+  }
 }
 
 void PGRecovery::on_local_recover(
@@ -449,9 +470,11 @@ void PGRecovery::enqueue_push(
   const hobject_t& obj,
   const eversion_t& v)
 {
-  logger().debug("{}: obj={} v={}",
+  logger().info("{}: obj={} v={}",
                  __func__, obj, v);
-  pg->get_recovery_backend()->add_recovering(obj);
+  auto [recovering, added] = pg->get_recovery_backend()->add_recovering(obj);
+  if (!added)
+    return;
   std::ignore = pg->get_recovery_backend()->recover_object(obj, v).\
   handle_exception_interruptible([] (auto) {
     ceph_abort_msg("got exception on backfill's push");

diff --git a/src/crimson/osd/recovery_backend.h b/src/crimson/osd/recovery_backend.h
index 65e9bb01fbda54eac956f96a566d964aef34d0a6..abf6958915966d51ece04ceee3e3726f3a09eded 100644 (file)
--- a/src/crimson/osd/recovery_backend.h
+++ b/src/crimson/osd/recovery_backend.h
@@ -45,10 +45,10 @@ public:
       coll{coll},
       backend{backend} {}
   virtual ~RecoveryBackend() {}
-  WaitForObjectRecovery& add_recovering(const hobject_t& soid) {
+  std::pair<WaitForObjectRecovery&, bool> add_recovering(const hobject_t& soid) {
     auto [it, added] = recovering.emplace(soid, new WaitForObjectRecovery{});
-    assert(added);
-    return *(it->second);
+    assert(it->second);
+    return {*(it->second), added};
   }
   WaitForObjectRecovery& get_recovering(const hobject_t& soid) {
     assert(is_recovering(soid));