From: Xuehan Xu <xuxuehan@qianxin.com>
Date: Sat, 20 Jul 2024 08:17:47 +0000 (+0800)
Subject: crimson/osd/recovery_backend: fix RecoveryBackend::temp_contents usage
X-Git-Tag: v19.1.1~37^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=5742ebf78d6e299235b9f71dfffe1e05703a6c7d;p=ceph.git

crimson/osd/recovery_backend: fix RecoveryBackend::temp_contents usage

All temp objects are added *only* to PGBackend::temp_content.
cleaning RecoveryBackend::temp_contents (which is always empty) instead
of PGBackend::temp_contents is wrong.

Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>
(cherry picked from commit 71ba42693e7334f3905fb7a45a29caa16e5e8a58)
---

diff --git a/src/crimson/osd/pg_backend.h b/src/crimson/osd/pg_backend.h
index 6d1a1271a44cc..579e2bdee79c1 100644
--- a/src/crimson/osd/pg_backend.h
+++ b/src/crimson/osd/pg_backend.h
@@ -486,8 +486,6 @@ private:
   friend class ::crimson::osd::PG;
 
 protected:
-  boost::container::flat_set<hobject_t> temp_contents;
-
   template <class... Args>
   void add_temp_obj(Args&&... args) {
     temp_contents.insert(std::forward<Args>(args)...);
@@ -501,5 +499,15 @@ protected:
       clear_temp_obj(oid);
     }
   }
+  template <typename Func>
+  void for_each_temp_obj(Func &&f) {
+    std::for_each(temp_contents.begin(), temp_contents.end(), f);
+  }
+  void clear_temp_objs() {
+    temp_contents.clear();
+  }
+private:
+  boost::container::flat_set<hobject_t> temp_contents;
+
   friend class RecoveryBackend;
 };
diff --git a/src/crimson/osd/recovery_backend.cc b/src/crimson/osd/recovery_backend.cc
index e6b232c35613e..c55d315085033 100644
--- a/src/crimson/osd/recovery_backend.cc
+++ b/src/crimson/osd/recovery_backend.cc
@@ -45,11 +45,11 @@ void RecoveryBackend::clear_temp_obj(const hobject_t &oid)
 void RecoveryBackend::clean_up(ceph::os::Transaction& t,
 			       std::string_view why)
 {
-  for (auto& soid : temp_contents) {
+  for_each_temp_obj([&](auto &soid) {
     t.remove(pg.get_collection_ref()->get_cid(),
 	      ghobject_t(soid, ghobject_t::NO_GEN, pg.get_pg_whoami().shard));
-  }
-  temp_contents.clear();
+  });
+  clear_temp_objs();
 
   for (auto& [soid, recovery_waiter] : recovering) {
     if ((recovery_waiter->pull_info
diff --git a/src/crimson/osd/recovery_backend.h b/src/crimson/osd/recovery_backend.h
index f5a365c155883..179cfbabd084b 100644
--- a/src/crimson/osd/recovery_backend.h
+++ b/src/crimson/osd/recovery_backend.h
@@ -10,6 +10,7 @@
 #include "crimson/os/futurized_collection.h"
 #include "crimson/osd/pg_interval_interrupt_condition.h"
 #include "crimson/osd/object_context.h"
+#include "crimson/osd/pg_backend.h"
 #include "crimson/osd/shard_services.h"
 
 #include "messages/MOSDPGBackfill.h"
@@ -22,8 +23,6 @@ namespace crimson::osd{
   class PG;
 }
 
-class PGBackend;
-
 class RecoveryBackend {
 public:
   class WaitForObjectRecovery;
@@ -240,10 +239,15 @@ protected:
     const hobject_t& target,
     eversion_t version) const;
 
-  boost::container::flat_set<hobject_t> temp_contents;
-
   void add_temp_obj(const hobject_t &oid);
   void clear_temp_obj(const hobject_t &oid);
+  template <typename Func>
+  void for_each_temp_obj(Func &&f) {
+    backend->for_each_temp_obj(std::forward<Func>(f));
+  }
+  void clear_temp_objs() {
+    backend->clear_temp_objs();
+  }
 
   void clean_up(ceph::os::Transaction& t, std::string_view why);
   virtual seastar::future<> on_stop() = 0;