From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Thu, 9 May 2024 19:09:50 +0000 (+0000)
Subject: osd: replace get_obc() with maybe_load_obc() in EC recovery
X-Git-Tag: v20.1.1~11^2~29
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2aa95b22a11676eb0d12b549db009c2686efd6d5;p=ceph.git

osd: replace get_obc() with maybe_load_obc() in EC recovery

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
(cherry picked from commit 266773625f997ff6a1fda82b201e023948a5c081)
---

diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc
index 51f785af7e4..3dfaf62bca3 100644
--- a/src/osd/ECBackend.cc
+++ b/src/osd/ECBackend.cc
@@ -353,28 +353,12 @@ void ECCommon::RecoveryBackend::handle_recovery_read_complete(
 
   if (res.attrs) {
     op.xattrs.swap(*(res.attrs));
-
-    if (!op.obc) {
-      // attrs only reference the origin bufferlist (decode from
-      // ECSubReadReply message) whose size is much greater than attrs
-      // in recovery. If obc cache it (get_obc maybe cache the attr),
-      // this causes the whole origin bufferlist would not be free
-      // until obc is evicted from obc cache. So rebuild the
-      // bufferlist before cache it.
-      for (map<string, bufferlist>::iterator it = op.xattrs.begin();
-           it != op.xattrs.end();
-           ++it) {
-        it->second.rebuild();
-      }
-      // Need to remove ECUtil::get_hinfo_key() since it should not leak out
-      // of the backend (see bug #12983)
-      map<string, bufferlist, less<>> sanitized_attrs(op.xattrs);
-      sanitized_attrs.erase(ECUtil::get_hinfo_key());
-      op.obc = get_parent()->get_obc(hoid, sanitized_attrs);
-      ceph_assert(op.obc);
-      op.recovery_info.size = op.obc->obs.oi.size;
-      op.recovery_info.oi = op.obc->obs.oi;
-
+    const auto empty_obc = !op.obc;
+    maybe_load_obc(op.xattrs, op);
+#ifdef WITH_CRIMSON
+    ceph_assert(hoid == op.hoid);
+#endif
+    if (empty_obc) {
       update_object_size_after_read(op.recovery_info.size, res, req);
     }
   }
@@ -404,6 +388,33 @@ void ECCommon::RecoveryBackend::handle_recovery_read_complete(
   continue_recovery_op(op, m);
 }
 
+void ECBackend::ECRecoveryBackend::maybe_load_obc(
+  const std::map<std::string, ceph::bufferlist, std::less<>>& raw_attrs,
+  RecoveryOp &op)
+{
+  if (!op.obc) {
+    // attrs only reference the origin bufferlist (decode from
+    // ECSubReadReply message) whose size is much greater than attrs
+    // in recovery. If obc cache it (get_obc maybe cache the attr),
+    // this causes the whole origin bufferlist would not be free
+    // until obc is evicted from obc cache. So rebuild the
+    // bufferlist before cache it.
+    for (map<string, bufferlist>::iterator it = op.xattrs.begin();
+         it != op.xattrs.end();
+         ++it) {
+      it->second.rebuild();
+    }
+    // Need to remove ECUtil::get_hinfo_key() since it should not leak out
+    // of the backend (see bug #12983)
+    map<string, bufferlist, less<>> sanitized_attrs(op.xattrs);
+    sanitized_attrs.erase(ECUtil::get_hinfo_key());
+    op.obc = get_parent()->get_obc(op.hoid, sanitized_attrs);
+    ceph_assert(op.obc);
+    op.recovery_info.size = op.obc->obs.oi.size;
+    op.recovery_info.oi = op.obc->obs.oi;
+  }
+}
+
 struct SendPushReplies : public Context {
   PGBackend::Listener *l;
   epoch_t epoch;
diff --git a/src/osd/ECBackend.h b/src/osd/ECBackend.h
index 4881043dcb2..0f15094a56d 100644
--- a/src/osd/ECBackend.h
+++ b/src/osd/ECBackend.h
@@ -204,6 +204,10 @@ class ECBackend : public ECCommon {
         ceph::os::Transaction &&txn,
         std::map<int, MOSDPGPushReply*> replies) override;
 
+    void maybe_load_obc(
+      const std::map<std::string, ceph::bufferlist, std::less<>>& raw_attrs,
+      RecoveryOp &op) final;
+
     PGBackend::Listener *get_parent() const { return parent; }
 
    private:
diff --git a/src/osd/ECCommon.h b/src/osd/ECCommon.h
index 66fbfde7398..cc84944a1d4 100644
--- a/src/osd/ECCommon.h
+++ b/src/osd/ECCommon.h
@@ -761,6 +761,9 @@ struct ECCommon {
     virtual void commit_txn_send_replies(
         ceph::os::Transaction &&txn,
         std::map<int, MOSDPGPushReply*> replies) = 0;
+    virtual void maybe_load_obc(
+      const std::map<std::string, ceph::bufferlist, std::less<>> &raw_attrs,
+      RecoveryOp &op) = 0;
     void dispatch_recovery_messages(RecoveryMessages &m, int priority);
 
     RecoveryBackend::RecoveryOp recover_object(
diff --git a/src/osd/ECListener.h b/src/osd/ECListener.h
index fd9e37a682f..a8f02e30af0 100644
--- a/src/osd/ECListener.h
+++ b/src/osd/ECListener.h
@@ -87,9 +87,11 @@ struct ECListener {
 
   virtual bool pg_is_repair() const = 0;
 
-     virtual ObjectContextRef get_obc(
-       const hobject_t &hoid,
-       const std::map<std::string, ceph::buffer::list, std::less<>> &attrs) = 0;
+#ifndef WITH_CRIMSON
+  virtual ObjectContextRef get_obc(
+    const hobject_t &hoid,
+    const std::map<std::string, ceph::buffer::list, std::less<>> &attrs) = 0;
+#endif
 
      virtual bool check_failsafe_full() = 0;
      virtual hobject_t get_temp_recovery_object(const hobject_t& target,