Merge pull request ceph#60803 from xxhdx1985126/wip-69027

crimson/osd/replicate_backend: add standalone delete to the push queue if the object is deleted while it's being backfilled

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

Author: Matan-B

src/crimson/osd/backfill_state.cc

@@ -677,6 +677,17 @@ void BackfillState::enqueue_standalone_push(
   backfill_machine.backfill_listener.enqueue_push(obj, v, peers);
 }
 
+void BackfillState::enqueue_standalone_delete(
+  const hobject_t &obj,
+  const eversion_t &v,
+  const std::vector<pg_shard_t> &peers)
+{
+  progress_tracker->enqueue_drop(obj);
+  for (auto bt : peers) {
+    backfill_machine.backfill_listener.enqueue_drop(bt, obj, v);
+  }
+}
+
 std::ostream &operator<<(std::ostream &out, const BackfillState::PGFacade &pg) {
   return pg.print(out);
 }

src/crimson/osd/backfill_state.h

@@ -292,6 +292,11 @@ struct BackfillState {
     const hobject_t &obj,
     const eversion_t &v,
     const std::vector<pg_shard_t> &peers);
+  void enqueue_standalone_delete(
+    const hobject_t &obj,
+    const eversion_t &v,
+    const std::vector<pg_shard_t> &peers);
+
 
   bool is_triggered() const {
     return backfill_machine.triggering_event() != nullptr;

src/crimson/osd/pg.cc

@@ -879,6 +879,17 @@ void PG::enqueue_push_for_backfill(
   backfill_state->enqueue_standalone_push(obj, v, peers);
 }
 
+void PG::enqueue_delete_for_backfill(
+  const hobject_t &obj,
+  const eversion_t &v,
+  const std::vector<pg_shard_t> &peers)
+{
+  assert(recovery_handler);
+  assert(recovery_handler->backfill_state);
+  auto backfill_state = recovery_handler->backfill_state.get();
+  backfill_state->enqueue_standalone_delete(obj, v, peers);
+}
+
 PG::interruptible_future<
   std::tuple<PG::interruptible_future<>,
              PG::interruptible_future<>>>

src/crimson/osd/pg.h

@@ -904,6 +904,11 @@ class PG : public boost::intrusive_ref_counter<
     const hobject_t &obj,
     const eversion_t &v,
     const std::vector<pg_shard_t> &peers);
+  void enqueue_delete_for_backfill(
+    const hobject_t &obj,
+    const eversion_t &v,
+    const std::vector<pg_shard_t> &peers);
+
   bool can_discard_replica_op(const Message& m, epoch_t m_map_epoch) const;
   bool can_discard_op(const MOSDOp& m) const;
   void context_registry_on_change();

src/crimson/osd/replicated_backend.cc

@@ -96,11 +96,18 @@ ReplicatedBackend::submit_transaction(
   bufferlist encoded_txn;
   encode(txn, encoded_txn);
 
+  bool is_delete = false;
   for (auto &le : log_entries) {
     le.mark_unrollbackable();
+    if (le.is_delete()) {
+      is_delete = true;
+    }
   }
 
+  co_await pg.update_snap_map(log_entries, txn);
+
   std::vector<pg_shard_t> to_push_clone;
+  std::vector<pg_shard_t> to_push_delete;
   auto sends = std::make_unique<std::vector<seastar::future<>>>();
   for (auto &pg_shard : pg_shards) {
     if (pg_shard == whoami) {
@@ -115,12 +122,17 @@ ReplicatedBackend::submit_transaction(
       m = new_repop_msg(
 	pg_shard, hoid, encoded_txn, osd_op_p,
 	min_epoch, map_epoch, log_entries, false, tid);
-      if (_new_clone && pg.is_missing_on_peer(pg_shard, hoid)) {
-	// The head is in the push queue but hasn't been pushed yet.
-	// We need to ensure that the newly created clone will be 
-	// pushed as well, otherwise we might skip it.
-	// See: https://tracker.ceph.com/issues/68808
-	to_push_clone.push_back(pg_shard);
+      if (pg.is_missing_on_peer(pg_shard, hoid)) {
+	if (_new_clone) {
+	  // The head is in the push queue but hasn't been pushed yet.
+	  // We need to ensure that the newly created clone will be
+	  // pushed as well, otherwise we might skip it.
+	  // See: https://tracker.ceph.com/issues/68808
+	  to_push_clone.push_back(pg_shard);
+	}
+	if (is_delete) {
+	  to_push_delete.push_back(pg_shard);
+	}
       }
     }
     pending_txn->second.acked_peers.push_back({pg_shard, eversion_t{}});
@@ -130,8 +142,6 @@ ReplicatedBackend::submit_transaction(
 	pg_shard.osd, std::move(m), map_epoch));
   }
 
-  co_await pg.update_snap_map(log_entries, txn);
-
   pg.log_operation(
     std::move(log_entries),
     osd_op_p.pg_trim_to,
@@ -157,7 +167,8 @@ ReplicatedBackend::submit_transaction(
       return seastar::now();
     }
     return peers->all_committed.get_shared_future();
-  }).then_interruptible([pending_txn, this, _new_clone,
+  }).then_interruptible([pending_txn, this, _new_clone, &hoid,
+			to_push_delete=std::move(to_push_delete),
 			to_push_clone=std::move(to_push_clone)] {
     auto acked_peers = std::move(pending_txn->second.acked_peers);
     pending_trans.erase(pending_txn);
@@ -167,6 +178,9 @@ ReplicatedBackend::submit_transaction(
 	_new_clone->obs.oi.version,
 	to_push_clone);
     }
+    if (!to_push_delete.empty()) {
+      pg.enqueue_delete_for_backfill(hoid, {}, to_push_delete);
+    }
     return seastar::make_ready_future<
       crimson::osd::acked_peers_t>(std::move(acked_peers));
   });

src/crimson/osd/replicated_recovery_backend.cc

@@ -35,6 +35,15 @@ ReplicatedRecoveryBackend::recover_object(
     logger().debug("recover_object: loading obc: {}", soid);
     return pg.obc_loader.with_obc<RWState::RWREAD>(soid,
       [this, soid, need](auto head, auto obc) {
+      if (!obc->obs.exists) {
+        // XXX: this recovery must be triggered by backfills and the corresponding
+        //      object must have been deleted by some client request after the object
+        //      is enqueued for push but before the lock is acquired by the recovery.
+        //
+        //      Abort the recovery in this case, a "recover_delete" must have been
+        //      added for this object by the client request that deleted it.
+        return interruptor::now();
+      }
       logger().debug("recover_object: loaded obc: {}", obc->obs.oi.soid);
       auto& recovery_waiter = get_recovering(soid);
       recovery_waiter.obc = obc;