Merge pull request ceph#54872 from amathuria/wip-amat-mclock-adjust-deletion-cost

osd: Change PG Deletion cost for mClock

Reviewed-by: Samuel Just <[email protected]>
Reviewed-by: Sridhar Seshasayee <[email protected]>

Author: yuriw

src/osd/OSD.cc

@@ -1879,14 +1879,21 @@ void OSDService::queue_scrub_next_chunk(PG *pg, Scrub::scrub_prio_t with_priorit
   queue_scrub_event_msg<PGScrubGetNextChunk>(pg, with_priority);
 }
 
-void OSDService::queue_for_pg_delete(spg_t pgid, epoch_t e)
+void OSDService::queue_for_pg_delete(spg_t pgid, epoch_t e, int64_t num_objects)
 {
   dout(10) << __func__ << " on " << pgid << " e " << e  << dendl;
+  uint64_t cost_for_queue = [this, num_objects] {
+    if (op_queue_type_t::mClockScheduler == osd->osd_op_queue_type()) {
+      return num_objects * cct->_conf->osd_pg_delete_cost;
+    } else {
+      return cct->_conf->osd_pg_delete_cost;
+    }
+  }();
   enqueue_back(
     OpSchedulerItem(
       unique_ptr<OpSchedulerItem::OpQueueable>(
 	new PGDelete(pgid, e)),
-      cct->_conf->osd_pg_delete_cost,
+      cost_for_queue,
       cct->_conf->osd_pg_delete_priority,
       ceph_clock_now(),
       0,
@@ -4068,7 +4075,6 @@ int OSD::init()
 
   // Override a few options if mclock scheduler is enabled.
   maybe_override_sleep_options_for_qos();
-  maybe_override_cost_for_qos();
   maybe_override_options_for_qos();
   maybe_override_max_osd_capacity_for_qos();
 
@@ -9886,9 +9892,6 @@ void OSD::handle_conf_change(const ConfigProxy& conf,
       changed.count("osd_recovery_sleep_hybrid")) {
     maybe_override_sleep_options_for_qos();
   }
-  if (changed.count("osd_pg_delete_cost")) {
-    maybe_override_cost_for_qos();
-  }
   if (changed.count("osd_min_recovery_priority")) {
     service.local_reserver.set_min_priority(cct->_conf->osd_min_recovery_priority);
     service.remote_reserver.set_min_priority(cct->_conf->osd_min_recovery_priority);
@@ -10225,15 +10228,6 @@ void OSD::maybe_override_sleep_options_for_qos()
   }
 }
 
-void OSD::maybe_override_cost_for_qos()
-{
-  // If the scheduler enabled is mclock, override the default PG deletion cost
-  // so that mclock can meet the QoS goals.
-  if (op_queue_type_t::mClockScheduler == osd_op_queue_type()) {
-    uint64_t pg_delete_cost = 15728640;
-    cct->_conf.set_val("osd_pg_delete_cost", std::to_string(pg_delete_cost));
-  }
-}
 
 /**
  * A context for receiving status from a background mon command to set

src/osd/OSD.h

@@ -563,7 +563,7 @@ class OSDService : public Scrub::ScrubSchedListener {
 				   unsigned int qu_priority,
 				   Scrub::act_token_t act_token);
 
-  void queue_for_pg_delete(spg_t pgid, epoch_t e);
+  void queue_for_pg_delete(spg_t pgid, epoch_t e, int64_t num_objects);
   bool try_finish_pg_delete(PG *pg, unsigned old_pg_num);
 
 private:
@@ -1994,7 +1994,6 @@ class OSD : public Dispatcher,
   void maybe_override_sleep_options_for_qos();
   bool maybe_override_options_for_qos(
     const std::set<std::string> *changed = nullptr);
-  void maybe_override_cost_for_qos();
   int run_osd_bench_test(int64_t count,
                          int64_t bsize,
                          int64_t osize,

src/osd/PG.cc

@@ -2665,7 +2665,8 @@ void PG::C_DeleteMore::complete(int r) {
   ceph_assert(r == 0);
   pg->lock();
   if (!pg->pg_has_reset_since(epoch)) {
-    pg->osd->queue_for_pg_delete(pg->get_pgid(), epoch);
+    pg->osd->queue_for_pg_delete(pg->get_pgid(), epoch,
+	                         num_objects);
   }
   pg->unlock();
   delete this;
@@ -2689,7 +2690,9 @@ std::pair<ghobject_t, bool> PG::do_delete_work(
         std::scoped_lock locker{*this};
         delete_needs_sleep = false;
         if (!pg_has_reset_since(e)) {
-          osd->queue_for_pg_delete(get_pgid(), e);
+	  // We pass 1 for num_objects here as only wpq uses this code path
+	  // and it will be ignored
+          osd->queue_for_pg_delete(get_pgid(), e, 1);
         }
       });
 
@@ -2762,7 +2765,7 @@ std::pair<ghobject_t, bool> PG::do_delete_work(
   bool running = true;
   if (num) {
     dout(20) << __func__ << " deleting " << num << " objects" << dendl;
-    Context *fin = new C_DeleteMore(this, get_osdmap_epoch());
+    Context *fin = new C_DeleteMore(this, get_osdmap_epoch(), num);
     t.register_on_commit(fin);
   } else {
     if (cct->_conf->osd_inject_failure_on_pg_removal) {

src/osd/PG.h

@@ -776,7 +776,9 @@ class PG : public DoutPrefixProvider,
   struct C_DeleteMore : public Context {
     PGRef pg;
     epoch_t epoch;
-    C_DeleteMore(PG *p, epoch_t e) : pg(p), epoch(e) {}
+    int64_t num_objects;
+    C_DeleteMore(PG *p, epoch_t e, int64_t num) : pg(p), epoch(e),
+	                                          num_objects(num){}
     void finish(int r) override {
       ceph_abort();
     }

src/osd/PrimaryLogPG.cc

@@ -12851,7 +12851,9 @@ void PrimaryLogPG::on_removal(ObjectStore::Transaction &t)
 
   on_shutdown();
 
-  t.register_on_commit(new C_DeleteMore(this, get_osdmap_epoch()));
+  // starting PG deletion, num_objects can be 1
+  // do_delete_work will update num_objects
+  t.register_on_commit(new C_DeleteMore(this, get_osdmap_epoch(), 1));
 }
 
 void PrimaryLogPG::clear_async_reads()