src/messages, osd: Calculate and set cost for subOpReads for mClock scheduler

Previously, sub-op reads returned a hardcoded cost of 0, bypassing
mClock's background bandwidth and tag calculation mechanisms. This
allowed backfill operations to proceed un-metered, occasionally causing
backend resource contention and driving up client tail latencies.

Cost is calculated based on whether the complete chunk/shard or a subchunk
needs to be read. The possible cases are:
1. Read the complete chunk aligned length:
   - Cost is set to the length of the chunk aligned extent size.
2. Fragmented reads:
   - Consider the subchunk length and count to calculate the cost.
   - compute_cost evaluates the exact layout of fragmented shard bytes on
     disk by summing up the active subchunk allocations exactly once
     (`fragmented_shard_bytes += k.second * subchunk_size`).
   - Linear Extent Scaling: Scale the baseline footprint cleanly by
     multiplying it against the true count of read extents (`tl.size()`),
     achieving a highly efficient O(N) time complexity.

This linear cost model is compatible with pools running with
'allow_ec_optimizations' set to true. Under the FastEC optimized
pipeline, most operations are unified and bypass fragment slicing,
meaning requests will primarily match the Case 1 chunk-aligned path.
In Case 2 where applicable, the O(N) loop ensures that cost will
scale proportionally according to the layout.

It is important to note that the amount of data to read was set to an upper
bound defined by osd_recovery_max_chunk (8 MiB) and was rounded up to the
stripe width. The reason for setting a higher than actual upper bound is that
there may be cases where the object doesn't have the xattrs yet to determine
its size. Therefore, the amount to read was ultimatly set to ~(8 MiB / k)
where k is the number of data shards. This can cause mClock to prolong
the recovery times as items stay longer in the queue. To address this, the
amount to read is set to the remaining length of the object to recover
if the object size is known. Otherwise, the amount to read is set to the
recovery chunk size as before. Therefore, in some cases, only the first
recovery read could be costly if the object context is not known.

The MOSDECSubOpRead class introduces the following:
 - cost member. This necessitates an increment to the HEAD_VERSION and
   appropriate handling within the encode and decode methods.
 - compute_cost() that is called when creating the message by
   ECCommonL::ReadPipeline::do_read_op(). This calls into ECSubRead::cost()
   that performs the actual calculations to set the cost based on the cases
   mentioned above.
 - The same sequence applies to the EC optimized path in
   ECCommon::ReadPipeline::do_read_op().

Fixes: https://tracker.ceph.com/issues/71655
Signed-off-by: Sridhar Seshasayee <sridhar.seshasayee@ibm.com>
(cherry picked from commit e79befea328e58366c43e490c129e2a467e6db84)

Conflicts:
        src/osd/ECMsgTypes.h
- Removed a couple of variable declarations related to recovery of OMAP
  header and entries in EC pools which is yet to be backported to
  tentacle. See commit: a5f4a4902075e343df154da61e3d205d2bd2a5d5.

diff --git a/src/messages/MOSDECSubOpRead.h b/src/messages/MOSDECSubOpRead.h
index 254357f137f1192c6152cd4b65b199f26eee75f0..bb4974e6b3da32fd8e1e462958ea8f63187b844c 100644 (file)
--- a/src/messages/MOSDECSubOpRead.h
+++ b/src/messages/MOSDECSubOpRead.h
@@ -20,16 +20,26 @@
 
 class MOSDECSubOpRead : public MOSDFastDispatchOp {
 private:
-  static constexpr int HEAD_VERSION = 3;
+  static constexpr int HEAD_VERSION = 4;
   static constexpr int COMPAT_VERSION = 1;
 
 public:
   spg_t pgid;
   epoch_t map_epoch = 0, min_epoch = 0;
   ECSubRead op;
+  uint64_t cost = 0;
 
+  /**
+    * Calculate the cost of the SubOp read operation for mClock scheduler.
+    *
+    * @param *cct: *CephContext
+    * @param pair<int, int>&: subchunk_count and subchunk_size
+    */
+  void compute_cost(CephContext *cct, std::pair<int, int> &subchunk_info) {
+    cost = op.cost(cct, subchunk_info);
+  }
   int get_cost() const override {
-    return 0;
+    return cost;
   }
   epoch_t get_map_epoch() const override {
     return map_epoch;
@@ -57,6 +67,9 @@ public:
     } else {
       min_epoch = map_epoch;
     }
+    if (header.version >= 4) {
+      decode(cost, p);
+    }
   }
 
   void encode_payload(uint64_t features) override {
@@ -66,6 +79,9 @@ public:
     encode(op, payload, features);
     encode(min_epoch, payload);
     encode_trace(payload, features);
+    if (header.version >= 4) {
+      encode(cost, payload);
+    }
   }
 
   std::string_view get_type_name() const override { return "MOSDECSubOpRead"; }

diff --git a/src/osd/ECBackendL.cc b/src/osd/ECBackendL.cc
index 79666f9370c3c8da0191074d36d54d0aa99daf7c..d9b9ac694ab834ba90b1756cba8036350b01043f 100644 (file)
--- a/src/osd/ECBackendL.cc
+++ b/src/osd/ECBackendL.cc
@@ -572,7 +572,18 @@ void ECBackendL::RecoveryBackend::continue_recovery_op(
       ceph_assert(!op.recovery_progress.data_complete);
       set<int> want(op.missing_on_shards.begin(), op.missing_on_shards.end());
       uint64_t from = op.recovery_progress.data_recovered_to;
+      /* When beginning recovery, the OI may not be known. As such the object
+       * size is not known. For the first read, attempt to read the default
+       * size.  If this is larger than the object sizes, then the OSD will
+       * return truncated reads.  If the object size is known, then attempt
+       * correctly sized reads, capped at recovery chunk size.
+       * (Ref: ECCommon.cc -> ECCommon::RecoveryBackend::continue_recovery_op())
+       */
       uint64_t amount = get_recovery_chunk_size();
+      if (op.obc) {
+        uint64_t remaining = op.obc->obs.oi.size - from;
+        amount = std::min(remaining, amount);
+      }
 
       if (op.recovery_progress.first && op.obc) {
         if (auto [r, attrs, size] = ecbackend->get_attrs_n_size_from_disk(op.hoid);

diff --git a/src/osd/ECCommon.cc b/src/osd/ECCommon.cc
index 1125d0655a3d1493703d7c08c51d966c46d19acc..6b597e4129f84228f13685cf854d129a9c1577a7 100644 (file)
--- a/src/osd/ECCommon.cc
+++ b/src/osd/ECCommon.cc
@@ -499,6 +499,9 @@ void ECCommon::ReadPipeline::do_read_op(ReadOp &rop) {
   std::optional<ECSubRead> local_read_op;
   std::vector<std::pair<int, Message*>> m;
   m.reserve(messages.size());
+  std::pair<int, int> subchunk_info =
+    std::make_pair(ec_impl->get_sub_chunk_count(),
+      sinfo.get_chunk_size() / ec_impl->get_sub_chunk_count());
   for (auto &&[pg_shard, read]: messages) {
     rop.in_progress.insert(pg_shard);
     shard_to_read_map[pg_shard].insert(rop.tid);
@@ -522,6 +525,7 @@ void ECCommon::ReadPipeline::do_read_op(ReadOp &rop) {
       msg->trace.init("ec sub read", nullptr, &rop.trace);
       msg->trace.keyval("shard", pg_shard.shard.id);
     }
+    msg->compute_cost(cct, subchunk_info);
     m.push_back(std::make_pair(pg_shard.osd, msg));
     dout(10) << __func__ << ": will send msg " << *msg
              << " to osd." << pg_shard << dendl;

diff --git a/src/osd/ECCommonL.cc b/src/osd/ECCommonL.cc
index da4cd6b35c7df95322ec2e085509b95bb78af59b..16c2ceb5bbc49015d0f5d4ecf0f975578f2fe32a 100644 (file)
--- a/src/osd/ECCommonL.cc
+++ b/src/osd/ECCommonL.cc
@@ -467,6 +467,9 @@ void ECCommonL::ReadPipeline::do_read_op(ReadOp &op)
 
   std::vector<std::pair<int, Message*>> m;
   m.reserve(messages.size());
+  std::pair<int, int> subchunk_info =
+    std::make_pair(ec_impl->get_sub_chunk_count(),
+      sinfo.get_chunk_size() / ec_impl->get_sub_chunk_count());
   for (map<pg_shard_t, ECSubRead>::iterator i = messages.begin();
        i != messages.end();
        ++i) {
@@ -488,6 +491,7 @@ void ECCommonL::ReadPipeline::do_read_op(ReadOp &op)
       msg->trace.init("ec sub read", nullptr, &op.trace);
       msg->trace.keyval("shard", i->first.shard.id);
     }
+    msg->compute_cost(cct, subchunk_info);
     m.push_back(std::make_pair(i->first.osd, msg));
   }
   if (!m.empty()) {
@@ -1106,4 +1110,4 @@ ECUtilL::HashInfoRef ECCommonL::UnstableHashInfoRegistry::get_hash_info(
 }
 }
 
-END_IGNORE_DEPRECATED
\ No newline at end of file
+END_IGNORE_DEPRECATED

diff --git a/src/osd/ECMsgTypes.cc b/src/osd/ECMsgTypes.cc
index 1ccdc188d0ee977f463a3ab35c57f7ad915264de..698947738d2fa190bdb89f120426b557626b3b4a 100644 (file)
--- a/src/osd/ECMsgTypes.cc
+++ b/src/osd/ECMsgTypes.cc
@@ -14,6 +14,8 @@
 
 #include "ECMsgTypes.h"
 
+#include "common/ceph_context.h"
+
 using std::list;
 using std::make_pair;
 using std::map;
@@ -252,6 +254,66 @@ void ECSubRead::decode(bufferlist::const_iterator &bl)
   DECODE_FINISH(bl);
 }
 
+/**
+ * Calculate the cost of the SubOp read operation for mClock scheduler.
+ * Cost is calculated based on whether the complete chunk/shard
+ * or a subchunk needs to be read:
+ * Case 1. Read the complete chunk aligned length:
+ *  - Cost is set to the length of the chunk aligned extent size.
+ * Case 2. Fragmented reads:
+ *  - Cost is set by considering the subchunk length and count.
+ *
+ * Note: To retain the legacy behavior, a cost of '0' is returned as
+ *       before for WeightedPriorityQueue scheduler.
+ */
+uint64_t ECSubRead::cost(CephContext *cct, std::pair<int, int>& subchunk_info)
+{
+  uint64_t total_cost = 0;
+  /**
+   * While the cost is calculated by the primary shard with mClock
+   * scheduler being active, the replica shard could still be
+   * running on the legacy WPQ scheduler. In such a case, the
+   * replica shard's decoding logic would set the cost to 0, which
+   * is consistent with what the legacy WPQ scheduler expects.
+   *
+   * In the converse case, the primary shard running with the
+   * legacy WPQ scheduler sends a cost of 0. The replica shard
+   * running with mClock scheduler will interpret this and set
+   * the cost to 1 accordingly.
+   */
+  if (cct->_conf->osd_op_queue != "mclock_scheduler") {
+    return total_cost; // Legacy behavior for WPQ scheduler
+  }
+
+  uint64_t subchunk_size = subchunk_info.second;
+
+  for (auto &&[hoid, tl] : to_read) {
+    auto it = subchunks.find(hoid);
+    if (it == subchunks.end()) continue;
+
+    auto &sc = it->second;
+    if (sc.empty()) continue;
+
+    // Case 1: Optimized / Complete chunk aligned read
+    if (sc.size() == 1 && sc.front().second == subchunk_info.first) {
+      for ([[maybe_unused]] auto &&[offset, len, flags] : tl) {
+        total_cost += len;
+      }
+      continue;
+    }
+
+    // Case 2: Fragmented / Subchunk Reads
+    uint64_t fragmented_shard_bytes = 0;
+    for (auto &&k : sc) {
+      fragmented_shard_bytes += (uint64_t)k.second * subchunk_size;
+    }
+    total_cost += fragmented_shard_bytes * tl.size();
+  }
+
+  // Safety Boundary: mClock requires non-zero costs for tracking active ops
+  return std::max<uint64_t>(total_cost, 1ULL);
+}
+
 std::ostream &operator<<(
   std::ostream &lhs, const ECSubRead &rhs)
 {

diff --git a/src/osd/ECMsgTypes.h b/src/osd/ECMsgTypes.h
index 147b56c0f0a7cf0fb201b627177a35cf445d6a46..86a33d372936a90a1e9e79b9da306f38fe734c15 100644 (file)
--- a/src/osd/ECMsgTypes.h
+++ b/src/osd/ECMsgTypes.h
@@ -116,6 +116,14 @@ struct ECSubRead {
   std::map<hobject_t, std::list<boost::tuple<uint64_t, uint64_t, uint32_t> >> to_read;
   std::set<hobject_t> attrs_to_read;
   std::map<hobject_t, std::vector<std::pair<int, int>>> subchunks;
+  /**
+    * Calculate the cost of the SubOp read operation for mClock scheduler.
+    *
+    * @param *cct: *CephContext
+    * @param pair<int, int>&: subchunk_count and subchunk_size
+    * @return uint64_t - Cost of EC SubOpRead (size in Bytes)
+    */
+  uint64_t cost(CephContext *cct, std::pair<int, int>& subchunk_info);
   void encode(ceph::buffer::list &bl, uint64_t features) const;
   void decode(ceph::buffer::list::const_iterator &bl);
   void dump(ceph::Formatter *f) const;