From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Mon, 16 Dec 2024 17:24:03 +0000 (+0000)
Subject: Revert "crimson/.../object_context: drop recovery_read_marker"
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0f6abcd999324ed702b013aa20100bc2e165f94c;p=ceph-ci.git

Revert "crimson/.../object_context: drop recovery_read_marker"

This reverts commit c1b7435d6c3a598ee13dd57ba8b83014c4cca893.

EC support needs `wait_for_recovery()`.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
---

diff --git a/src/crimson/osd/object_context.h b/src/crimson/osd/object_context.h
index 415f4281035..3714913a725 100644
--- a/src/crimson/osd/object_context.h
+++ b/src/crimson/osd/object_context.h
@@ -64,6 +64,7 @@ class ObjectContext : public ceph::common::intrusive_lru_base<
 {
 private:
   tri_mutex lock;
+  bool recovery_read_marker = false;
 
 public:
   ObjectState obs;
@@ -138,6 +139,9 @@ public:
   template<typename Exception>
   void interrupt(Exception ex) {
     lock.abort(std::move(ex));
+    if (recovery_read_marker) {
+      drop_recovery_read();
+    }
   }
 
   bool is_loaded() const {
@@ -228,6 +232,23 @@ public:
   bool is_request_pending() const {
     return lock.is_acquired();
   }
+
+  bool get_recovery_read() {
+    if (lock.try_lock_for_read()) {
+      recovery_read_marker = true;
+      return true;
+    } else {
+      return false;
+    }
+  }
+  void wait_recovery_read() {
+    assert(lock.get_readers() > 0);
+    recovery_read_marker = true;
+  }
+  void drop_recovery_read() {
+    assert(recovery_read_marker);
+    recovery_read_marker = false;
+  }
 };
 using ObjectContextRef = ObjectContext::Ref;
 
diff --git a/src/crimson/osd/pg_recovery.cc b/src/crimson/osd/pg_recovery.cc
index 625c6abf86d..2e36d63a8f8 100644
--- a/src/crimson/osd/pg_recovery.cc
+++ b/src/crimson/osd/pg_recovery.cc
@@ -432,6 +432,8 @@ void PGRecovery::on_global_recover (
   pg->get_peering_state().object_recovered(soid, stat_diff);
   pg->publish_stats_to_osd();
   auto& recovery_waiter = pg->get_recovery_backend()->get_recovering(soid);
+  if (!is_delete)
+    recovery_waiter.obc->drop_recovery_read();
   recovery_waiter.set_recovered();
   pg->get_recovery_backend()->remove_recovering(soid);
   pg->get_recovery_backend()->found_and_remove(soid);
diff --git a/src/crimson/osd/replicated_recovery_backend.cc b/src/crimson/osd/replicated_recovery_backend.cc
index 2b02fac5bde..e35cde53363 100644
--- a/src/crimson/osd/replicated_recovery_backend.cc
+++ b/src/crimson/osd/replicated_recovery_backend.cc
@@ -31,34 +31,30 @@ ReplicatedRecoveryBackend::recover_object(
   // always add_recovering(soid) before recover_object(soid)
   assert(is_recovering(soid));
   // start tracking the recovery of soid
-  return maybe_pull_missing_obj(
-    soid, need
-  ).then_interruptible([FNAME, this, soid, need] {
+  return maybe_pull_missing_obj(soid, need).then_interruptible([FNAME, this, soid, need] {
     DEBUGDPP("loading obc: {}", pg, soid);
-    return pg.obc_loader.with_obc<RWState::RWREAD>(
-      soid,
+    return pg.obc_loader.with_obc<RWState::RWREAD>(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.
-	  return interruptor::now();
-	}
-	DEBUGDPP("loaded obc: {}", pg, obc->obs.oi.soid);
-	auto& recovery_waiter = get_recovering(soid);
-	recovery_waiter.obc = obc;
-	return maybe_push_shards(head, soid, need);
-      }, false).handle_error_interruptible(
-	crimson::osd::PG::load_obc_ertr::all_same_way(
-	  [FNAME, this, soid](auto& code) {
-	    // TODO: may need eio handling?
-	    ERRORDPP("saw error code {}, ignoring object {}",
-		     pg, code, soid);
-	    return seastar::now();
-	  }));
+      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();
+      }
+      DEBUGDPP("loaded obc: {}", pg, obc->obs.oi.soid);
+      auto& recovery_waiter = get_recovering(soid);
+      recovery_waiter.obc = obc;
+      recovery_waiter.obc->wait_recovery_read();
+      return maybe_push_shards(head, soid, need);
+    }, false).handle_error_interruptible(
+      crimson::osd::PG::load_obc_ertr::all_same_way([FNAME, this, soid](auto& code) {
+      // TODO: may need eio handling?
+      ERRORDPP("saw error code {}, ignoring object {}", pg, code, soid);
+      return seastar::now();
+    }));
   });
 }
 
@@ -111,6 +107,10 @@ ReplicatedRecoveryBackend::maybe_push_shards(
     }
     return seastar::make_ready_future<>();
   }).handle_exception_interruptible([this, soid](auto e) {
+    auto &recovery = get_recovering(soid);
+    if (recovery.obc) {
+      recovery.obc->drop_recovery_read();
+    }
     recovering.erase(soid);
     return seastar::make_exception_future<>(e);
   });