crimson: fix mismatch between backfill's enqueue_push() and recovery.

The unittesting discovered an issue between `BackfillState` and
the low-layer recovery code: `RecoveryBackend::recover_object()`.
`BackfillState::enqueue_push()` was assuming it controlls also
to which backfill target the push is sent while `recover_object()`
calculated the set on its own.
Fixing this is the reason why `enqueue_push()` loses the `const
pg_shard_t& target` parameter.

Signed-off-by: Radoslaw Zarzynski <[email protected]>

src/crimson/osd/backfill_state.cc

@@ -258,20 +258,20 @@ BackfillState::Enqueuing::update_on_peers(const hobject_t& check)
     // Find all check peers that have the wrong version
     if (const eversion_t& obj_v = primary_bi.objects.begin()->second;
         check == primary_bi.begin && check == peer_bi.begin) {
-      if(peer_bi.objects.begin()->second != obj_v) {
-        backfill_state().progress_tracker->enqueue_push(primary_bi.begin);
-        backfill_listener().enqueue_push(bt, primary_bi.begin, obj_v);
+      if(peer_bi.objects.begin()->second != obj_v &&
+          backfill_state().progress_tracker->enqueue_push(primary_bi.begin)) {
+        backfill_listener().enqueue_push(primary_bi.begin, obj_v);
       } else {
-        // it's fine, keep it!
+        // it's fine, keep it! OR already recovering
       }
       result.pbi_targets.insert(bt);
     } else {
       // Only include peers that we've caught up to their backfill line
       // otherwise, they only appear to be missing this object
       // because their peer_bi.begin > backfill_info.begin.
-      if (primary_bi.begin > peering_state().get_peer_last_backfill(bt)) {
-        backfill_state().progress_tracker->enqueue_push(primary_bi.begin);
-        backfill_listener().enqueue_push(bt, primary_bi.begin, obj_v);
+      if (primary_bi.begin > peering_state().get_peer_last_backfill(bt) &&
+          backfill_state().progress_tracker->enqueue_push(primary_bi.begin)) {
+        backfill_listener().enqueue_push(primary_bi.begin, obj_v);
       }
     }
   }
@@ -496,16 +496,17 @@ bool BackfillState::ProgressTracker::tracked_objects_completed() const
   return registry.empty();
 }
 
-void BackfillState::ProgressTracker::enqueue_push(const hobject_t& obj)
+bool BackfillState::ProgressTracker::enqueue_push(const hobject_t& obj)
 {
-  ceph_assert(registry.count(obj) == 0);
-  registry[obj] = registry_item_t{ op_stage_t::enqueued_push, std::nullopt };
+  [[maybe_unused]] const auto [it, first_seen] = registry.try_emplace(
+    obj, registry_item_t{op_stage_t::enqueued_push, std::nullopt});
+  return first_seen;
 }
 
 void BackfillState::ProgressTracker::enqueue_drop(const hobject_t& obj)
 {
-  ceph_assert(registry.count(obj) == 0);
-  registry[obj] = registry_item_t{ op_stage_t::enqueued_drop, pg_stat_t{} };
+  registry.try_emplace(
+    obj, registry_item_t{op_stage_t::enqueued_drop, pg_stat_t{}});
 }
 
 void BackfillState::ProgressTracker::complete_to(

src/crimson/osd/backfill_state.h

@@ -287,7 +287,6 @@ struct BackfillState::BackfillListener {
     const hobject_t& begin) = 0;
 
   virtual void enqueue_push(
-    const pg_shard_t& target,
     const hobject_t& obj,
     const eversion_t& v) = 0;
 
@@ -339,7 +338,7 @@ class BackfillState::ProgressTracker {
 
   bool tracked_objects_completed() const;
 
-  void enqueue_push(const hobject_t&);
+  bool enqueue_push(const hobject_t&);
   void enqueue_drop(const hobject_t&);
   void complete_to(const hobject_t&, const pg_stat_t&);
 };

src/crimson/osd/pg_recovery.cc

@@ -430,12 +430,11 @@ void PGRecovery::request_primary_scan(
 }
 
 void PGRecovery::enqueue_push(
-  const pg_shard_t& target,
   const hobject_t& obj,
   const eversion_t& v)
 {
-  logger().debug("{}: target={} obj={} v={}",
-                 __func__, target, obj, v);
+  logger().debug("{}: obj={} v={}",
+                 __func__, obj, v);
   pg->get_recovery_backend()->add_recovering(obj);
   std::ignore = pg->get_recovery_backend()->recover_object(obj, v).\
   handle_exception([] (auto) {

src/crimson/osd/pg_recovery.h

@@ -93,7 +93,6 @@ class PGRecovery : public crimson::osd::BackfillState::BackfillListener {
   void request_primary_scan(
     const hobject_t& begin) final;
   void enqueue_push(
-    const pg_shard_t& target,
     const hobject_t& obj,
     const eversion_t& v) final;
   void enqueue_drop(

src/test/crimson/test_backfill.cc

@@ -124,7 +124,6 @@ class BackfillFixture : public crimson::osd::BackfillState::BackfillListener {
     const hobject_t& begin) override;
 
   void enqueue_push(
-    const pg_shard_t& target,
     const hobject_t& obj,
     const eversion_t& v) override;
 
@@ -281,11 +280,12 @@ void BackfillFixture::request_primary_scan(
 }
 
 void BackfillFixture::enqueue_push(
-  const pg_shard_t& target,
   const hobject_t& obj,
   const eversion_t& v)
 {
-  backfill_targets.at(target).store.push(obj, v);
+  for (auto& [ _, bt ] : backfill_targets) {
+    bt.store.push(obj, v);
+  }
   schedule_event(crimson::osd::BackfillState::ObjectPushed{ obj });
 }