From: Sridhar Seshasayee <sseshasa@redhat.com>
Date: Fri, 3 Feb 2023 05:36:06 +0000 (+0530)
Subject: osd/osd_types: use appropriate cost value for PullOp
X-Git-Tag: v18.1.0~122^2~28
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=da78c375432fe626b33d05a6063a0ef144819b15;p=ceph.git

osd/osd_types: use appropriate cost value for PullOp

See included comments -- previous values did not account for object
size.  This causes problems for mclock which is much more strict
in how it interprets costs.

Fixes: https://tracker.ceph.com/issues/58607
Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Sridhar Seshasayee <sseshasa@redhat.com>
---

diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc
index e78500ba37080..14694de195b62 100644
--- a/src/osd/osd_types.cc
+++ b/src/osd/osd_types.cc
@@ -15,6 +15,7 @@
  *
  */
 
+#include <algorithm>
 #include <list>
 #include <map>
 #include <ostream>
@@ -6807,8 +6808,20 @@ ostream& operator<<(ostream& out, const PullOp &op)
 
 uint64_t PullOp::cost(CephContext *cct) const
 {
-  return cct->_conf->osd_push_per_object_cost +
-    cct->_conf->osd_recovery_max_chunk;
+  if (cct->_conf->osd_op_queue == "mclock_scheduler") {
+    return std::clamp<uint64_t>(
+      recovery_progress.estimate_remaining_data_to_recover(recovery_info),
+      1,
+      cct->_conf->osd_recovery_max_chunk);
+  } else {
+    /* We retain this legacy behavior for WeightedPriorityQueue. It seems to
+     * require very large costs for several messages in order to do any
+     * meaningful amount of throttling.  This branch should be removed after
+     * Reef.
+     */
+    return cct->_conf->osd_push_per_object_cost +
+      cct->_conf->osd_recovery_max_chunk;
+  }
 }
 
 // -- PushOp --
diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index ce370c0b7dcb8..afed5fa835103 100644
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -6029,6 +6029,11 @@ struct ObjectRecoveryProgress {
       omap_complete;
   }
 
+  uint64_t estimate_remaining_data_to_recover(const ObjectRecoveryInfo& info) const {
+    // Overestimates in case of clones, but avoids traversing copy_subset
+    return info.size - data_recovered_to;
+  }
+
   static void generate_test_instances(std::list<ObjectRecoveryProgress*>& o);
   void encode(ceph::buffer::list &bl) const;
   void decode(ceph::buffer::list::const_iterator &bl);