crimson/osd: clear peer_missing entries for deleted objects

This commit fixes the abort in Recovered::Recovered.

There is a race to acquire the OBC lock between backfill and
client delete for the same object.

When the lock is acquired first by the backfill, the object is
recovered first, and then deleted by the client delete request.
When recovering the object, the corresponding peer_missing entry
is cleared and we are able to transition to Recovered state
successfully.

When the lock is acquired first by client delete request, the
object is deleted. Then backfill tries to recover the object,
finds it deleted and exists early. The stale peer_missing
entry is not cleared. In Recovered::Recovered, needs_recovery()
sees this stale peer_missing entry and calls abort.

This issue is fixed by clearing out the stale peer_missing
entries for the deleted object when object is being recovered
by recover_object().

Earlier, an alternative fix was to clear the stale
peer_missing entries in enqueue_delete_for_backfill called by
the client delete. This approach was abandoned as this can also
lead to a race condition which might result in this same error,
if the bckfill completes before the entries are cleared in
enqueue_delete_for_backfill().

Fixes: https://tracker.ceph.com/issues/70501
Signed-off-by: Shraddha Agrawal <shraddha.agrawal000@gmail.com>

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 59f5ff7c764ec49d31921fea03f234c394bda909..b2715cdde926f2fecc0711a18e37c5e519d56057 100644 (file)
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -951,9 +951,11 @@ void PG::enqueue_delete_for_backfill(
   const eversion_t &v,
   const std::vector<pg_shard_t> &peers)
 {
+  LOG_PREFIX(PG::enqueue_delete_for_backfill);
   assert(recovery_handler);
   assert(recovery_handler->backfill_state);
   auto backfill_state = recovery_handler->backfill_state.get();
+  DEBUGDPP("enqueue standalone delete {} v {} for peers {}", *this, obj, v, peers);
   backfill_state->enqueue_standalone_delete(obj, v, peers);
 }
 

diff --git a/src/crimson/osd/pg_recovery.cc b/src/crimson/osd/pg_recovery.cc
index 48d12d6c92e824ed952999e15b3d9c038af70e56..19961769bb236bf5c61a92763e1e298149286cfe 100644 (file)
--- a/src/crimson/osd/pg_recovery.cc
+++ b/src/crimson/osd/pg_recovery.cc
@@ -544,7 +544,7 @@ void PGRecovery::enqueue_push(
     return seastar::make_ready_future<>();
   }).then_interruptible([this, FNAME, obj] {
 
-    DEBUGDPP("enqueue_push", pg->get_dpp());
+    DEBUGDPP("complete obj={}", pg->get_dpp(), obj);
     using BackfillState = crimson::osd::BackfillState;
     if (backfill_state->is_triggered()) {
       backfill_state->post_event(

diff --git a/src/crimson/osd/replicated_recovery_backend.cc b/src/crimson/osd/replicated_recovery_backend.cc
index 5503063cbc9d810d06fb6235755d73034f34a1e9..d3ecafa6c4450788063260a729d04ad774bfdc9f 100644 (file)
--- a/src/crimson/osd/replicated_recovery_backend.cc
+++ b/src/crimson/osd/replicated_recovery_backend.cc
@@ -39,12 +39,22 @@ ReplicatedRecoveryBackend::recover_object(
       soid,
       [FNAME, 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.
+         // This recovery was triggered by backfill and the object was deleted by
+         // a client request after it was enqueued for push but before recovery
+         // acquired the OBC lock. Clear stale peer_missing entries that were added
+         // by prepare_backfill_for_missing() so that needs_recovery() does not
+         // fail the assertion in Recovered::Recovered.
+         DEBUGDPP("object does not exist, clearing peer_missing: {}", pg, soid);
+         for (const auto& peer : pg.get_acting_recovery_backfill()) {
+           if (peer == pg.get_pg_whoami())
+             continue;
+           pg_missing_item item;
+           if (pg.get_peering_state().get_peer_missing(peer).is_missing(soid, &item)) {
+             DEBUGDPP("clear stale peer_missing entry {} v {} for peer {}",
+                      pg, soid, item.need, peer);
+             pg.get_peering_state().on_peer_recover(peer, soid, item.need);
+           }
+         }
          return interruptor::now();
        }
        DEBUGDPP("loaded obc: {}", pg, obc->obs.oi.soid);