crimson/osd/pg: unify the current should_send_op method implementation

Fixes: https://tracker.ceph.com/issues/76196
Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 979cd00a31cd0c35c747c73333500f61a640fb48..f1b890eb2bea57400a3fbfecb5136831787f349b 100644 (file)
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -346,34 +346,6 @@ void PG::recheck_readable()
   }
 }
 
-bool PG::should_send_op(pg_shard_t peer, const hobject_t &hoid)
-{
-  if (peer == get_primary()) {
-    return true;
-  }
-  bool should_send =
-    hoid.pool != (int64_t)get_pgid().pool() ||
-    hoid <= peering_state.get_peer_info(peer).last_backfill ||
-    (recovery_handler->backfill_state &&
-      hoid <= recovery_handler->backfill_state->get_last_backfill_started());
-  if (!should_send) {
-    ceph_assert(is_backfill_target(peer));
-    logger().debug("{}: {} shipping empty opt to osd.{}, object {}"
-                  " beyond std::max(last_backfill_started,"
-                  " peer_info[peer].last_backfill {})",
-                  *this, __func__, peer, hoid,
-                  peering_state.get_peer_info(peer).last_backfill);
-    return should_send;
-  }
-  if (peering_state.is_async_recovery_target(peer) &&
-      peering_state.get_peer_missing(peer).is_missing(hoid)) {
-    should_send = false;
-    logger().info("{}: {} shipping empty opt to osd.{}, object {}"
-                 " which is pending recovery in async_recovery_targets",
-                *this, __func__, peer, hoid);
-  }
-  return should_send;
-}
 unsigned PG::get_target_pg_log_entries() const
 {
   const unsigned local_num_pgs = shard_services.get_num_local_pgs();
@@ -1801,31 +1773,35 @@ bool PG::is_degraded_or_backfilling_object(const hobject_t& soid) const {
 
 bool PG::should_send_op(
   pg_shard_t peer,
-  const hobject_t &hoid) const
+  const hobject_t &hoid)
 {
   if (peer == get_primary())
     return true;
   bool should_send =
     (hoid.pool != (int64_t)get_info().pgid.pool() ||
-    // An object has been fully pushed to the backfill target if and only if
-    // either of the following conditions is met:
-    // 1. peer_info.last_backfill has passed "hoid"
-    // 2. last_backfill_started has passed "hoid" and "hoid" is not in the peer
-    //    missing set
-    hoid <= peering_state.get_peer_info(peer).last_backfill ||
-    (has_backfill_state() && hoid <= get_last_backfill_started() &&
-     !is_missing_on_peer(peer, hoid)));
-  if (!should_send) {
-    ceph_assert(is_backfill_target(peer));
-    logger().debug("{} issue_repop shipping empty opt to osd."
-                   "{}, object {} beyond std::max(last_backfill_started, "
-                   "peer_info[peer].last_backfill {})",
-                   __func__, peer, hoid,
-                   peering_state.get_peer_info(peer).last_backfill);
+     // An object has been fully pushed to the backfill target if and only if
+     // hoid is not in the peer's missing set and either of the following
+     // conditions is met:
+     // 1. peer_info.last_backfill has passed "hoid"
+     // 2. last_backfill_started has passed "hoid"
+     hoid <= peering_state.get_peer_info(peer).last_backfill ||
+     (has_backfill_state() && hoid <= get_last_backfill_started())) &&
+    !is_missing_on_peer(peer, hoid);
+  if (unlikely(!should_send)) {
+    if (peering_state.is_async_recovery_target(peer)) {
+      logger().info("{}: {} shipping empty opt to osd.{}, object {}"
+                    " which is pending recovery in async_recovery_targets",
+                   *this, __func__, peer, hoid);
+    } else {
+      ceph_assert(is_backfill_target(peer));
+      logger().debug("{} issue_repop shipping empty opt to osd."
+                     "{}, object {} beyond std::max(last_backfill_started, "
+                     "peer_info[peer].last_backfill {})",
+                     __func__, peer, hoid,
+                     peering_state.get_peer_info(peer).last_backfill);
+    }
   }
   return should_send;
-  // TODO: should consider async recovery cases in the future which are not supported
-  //       by crimson yet
 }
 
 void PG::op_applied(const eversion_t &applied_version)

diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
index 808491abf219b1cfebb873ceb8709ca8459f5f56..b68fa886ec1620a6c0e6f58e1c421b89871c2e93 100644 (file)
--- a/src/crimson/osd/pg.h
+++ b/src/crimson/osd/pg.h
@@ -171,8 +171,6 @@ public:
     return get_acting_recovery_backfill();
   }
 
-  bool should_send_op(pg_shard_t peer, const hobject_t &hoid) override;
-
   spg_t primary_spg_t() const override {
     return spg_t(get_info().pgid.pgid, get_primary().shard);
   }
@@ -696,7 +694,7 @@ public:
   bool get_need_up_thru() const {
     return peering_state.get_need_up_thru();
   }
-  bool should_send_op(pg_shard_t peer, const hobject_t &hoid) const;
+  bool should_send_op(pg_shard_t peer, const hobject_t &hoid) final;
   epoch_t get_same_interval_since() const {
     return get_info().history.same_interval_since;
   }