Merge pull request ceph#57865 from ronen-fr/wip-rf-at-once

osd/scrub: allow new scrubs while reserving

Reviewed-by: Samuel Just <[email protected]>
Reviewed-by: Neha Ojha <[email protected]>

src/osd/scrubber/osd_scrub.cc

@@ -171,12 +171,6 @@ Scrub::OSDRestrictions OsdScrub::restrictions_on_scrubbing(
 	       << dendl;
       env_conditions.high_priority_only = true;
 
-  } else if (m_queue.is_reserving_now()) {
-    // if there is a PG that is just now trying to reserve scrub replica
-    // resources - we should wait and not initiate a new scrub
-    dout(10) << "scrub resources reservation in progress" << dendl;
-    env_conditions.high_priority_only = true;
-
   } else if (is_recovery_active && !conf->osd_scrub_during_recovery) {
     if (conf->osd_repair_during_recovery) {
       dout(15)
@@ -482,13 +476,3 @@ int OsdScrub::get_blocked_pgs_count() const
 {
   return m_queue.get_blocked_pgs_count();
 }
-
-bool OsdScrub::set_reserving_now(spg_t reserving_id, utime_t now_is)
-{
-  return m_queue.set_reserving_now(reserving_id, now_is);
-}
-
-void OsdScrub::clear_reserving_now(spg_t reserving_id)
-{
-  m_queue.clear_reserving_now(reserving_id);
-}

src/osd/scrubber/osd_scrub.h

@@ -134,15 +134,6 @@ class OsdScrub {
       utime_t t,
       bool high_priority_scrub) const;
 
-  /**
-   * No new scrub session will start while a scrub was initiated on a PG,
-   * and that PG is trying to acquire replica resources.
-   * \retval false if the flag was already set (due to a race)
-   */
-  bool set_reserving_now(spg_t reserving_id, utime_t now_is);
-
-  void clear_reserving_now(spg_t reserving_id);
-
   /**
    * push the 'not_before' time out by 'delay' seconds, so that this scrub target
    * would not be retried before 'delay' seconds have passed.

src/osd/scrubber/osd_scrub_sched.cc

@@ -361,34 +361,3 @@ int ScrubQueue::get_blocked_pgs_count() const
 {
   return blocked_scrubs_cnt;
 }
-
-// ////////////////////////////////////////////////////////////////////////// //
-// ScrubQueue - maintaining the 'some PG is reserving' flag
-
-bool ScrubQueue::set_reserving_now(spg_t reserving_id, utime_t now_is)
-{
-  std::unique_lock l{reserving_lock};
-
-  if (!reserving_pg.has_value()) {
-    reserving_pg = reserving_id;
-    reserving_since = now_is;
-    return true;
-  }
-  ceph_assert(reserving_id != *reserving_pg);
-  return false;
-}
-
-void ScrubQueue::clear_reserving_now(spg_t was_reserving_id)
-{
-  std::unique_lock l{reserving_lock};
-  if (reserving_pg && (*reserving_pg == was_reserving_id)) {
-    reserving_pg.reset();
-  }
-  // otherwise - ignore silently
-}
-
-bool ScrubQueue::is_reserving_now() const
-{
-  // no lock needed, as set_reserving_now() will recheck
-  return reserving_pg.has_value();
-}

src/osd/scrubber/osd_scrub_sched.h

@@ -84,7 +84,6 @@ ScrubQueue interfaces (main functions):
   - can_inc_scrubs()
   - {inc/dec}_scrubs_{local/remote}()
   - dump_scrub_reservations()
-  - {set/clear/is}_reserving_now()
 
 <2> - environment conditions:
 
@@ -238,30 +237,6 @@ class ScrubQueue {
  public:
   void dump_scrubs(ceph::Formatter* f) const;
 
-  /**
-   * No new scrub session will start while a scrub was initiated on a PG,
-   * and that PG is trying to acquire replica resources.
-   *
-   * \todo replace the atomic bool with a regular bool protected by a
-   * common OSD-service lock. Or better still - once PR#53263 is merged,
-   * remove this flag altogether.
-   */
-
-  /**
-   * set_reserving_now()
-   * \returns 'false' if the flag was already set
-   * (which is a possible result of a race between the check in OsdScrub and
-   * the initiation of a scrub by some other PG)
-   */
-  bool set_reserving_now(spg_t reserving_id, utime_t now_is);
-
-  /**
-   * silently ignore attempts to clear the flag if it was not set by
-   * the named pg.
-   */
-  void clear_reserving_now(spg_t reserving_id);
-  bool is_reserving_now() const;
-
   /// counting the number of PGs stuck while scrubbing, waiting for objects
   void mark_pg_scrub_blocked(spg_t blocked_pg);
   void clear_pg_scrub_blocked(spg_t blocked_pg);
@@ -331,17 +306,6 @@ class ScrubQueue {
    */
   std::atomic_int_fast16_t blocked_scrubs_cnt{0};
 
-  /**
-   * One of the OSD's primary PGs is in the initial phase of a scrub,
-   * trying to secure its replicas' resources. We will refrain from initiating
-   * any other scrub sessions until this one is done.
-   *
-   * \todo replace the local lock with regular osd-service locking
-   */
-  ceph::mutex reserving_lock = ceph::make_mutex("ScrubQueue::reserving_lock");
-  std::optional<spg_t> reserving_pg;
-  utime_t reserving_since;
-
   /**
    * If the scrub job was not explicitly requested, we postpone it by some
    * random length of time.

src/osd/scrubber/pg_scrubber.cc

@@ -1773,17 +1773,6 @@ void PgScrubber::handle_scrub_reserve_msgs(OpRequestRef op)
   }
 }
 
-
-bool PgScrubber::set_reserving_now() {
-  return m_osds->get_scrub_services().set_reserving_now(m_pg_id,
-                                                        ceph_clock_now());
-}
-
-void PgScrubber::clear_reserving_now()
-{
-  m_osds->get_scrub_services().clear_reserving_now(m_pg_id);
-}
-
 void PgScrubber::set_queued_or_active()
 {
   m_queued_or_active = true;

src/osd/scrubber/pg_scrubber.h

@@ -456,9 +456,6 @@ class PgScrubber : public ScrubPgIF,
 
   int build_replica_map_chunk() final;
 
-  bool set_reserving_now() final;
-  void clear_reserving_now() final;
-
   [[nodiscard]] bool was_epoch_changed() const final;
 
   void set_queued_or_active() final;

src/osd/scrubber/scrub_machine.cc

@@ -183,15 +183,6 @@ Session::Session(my_context ctx)
   dout(10) << "-- state -->> PrimaryActive/Session" << dendl;
   DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases
 
-  // while we've checked the 'someone is reserving' flag before queueing
-  // the start-scrub event, it's possible that the flag was set in the meantime.
-  // Handling this case here requires adding a new sub-state, and the
-  // complication of reporting a failure to the caller in a new failure
-  // path. On the other hand - ignoring an ongoing reservation on rare
-  // occasions will cause no harm.
-  // We choose ignorance.
-  std::ignore = scrbr->set_reserving_now();
-
   m_perf_set = &scrbr->get_counters_set();
   m_perf_set->inc(scrbcnt_started);
 }
@@ -243,14 +234,6 @@ ReservingReplicas::ReservingReplicas(my_context ctx)
   }
 }
 
-ReservingReplicas::~ReservingReplicas()
-{
-  DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases
-  // it's OK to try and clear the flag even if we don't hold it
-  // (the flag remembers the actual holder)
-  scrbr->clear_reserving_now();
-}
-
 sc::result ReservingReplicas::react(const ReplicaGrant& ev)
 {
   DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases

src/osd/scrubber/scrub_machine.h

@@ -564,7 +564,7 @@ struct Session : sc::state<Session, PrimaryActive, ReservingReplicas>,
 
 struct ReservingReplicas : sc::state<ReservingReplicas, Session>, NamedSimply {
   explicit ReservingReplicas(my_context ctx);
-  ~ReservingReplicas();
+  ~ReservingReplicas() = default;
   using reactions = mpl::list<
       sc::custom_reaction<ReplicaGrant>,
       sc::custom_reaction<ReplicaReject>,

src/osd/scrubber/scrub_machine_lstnr.h

@@ -205,18 +205,6 @@ struct ScrubMachineListener {
 
   virtual void set_scrub_duration(std::chrono::milliseconds duration) = 0;
 
-  /**
-   * No new scrub session will start while a scrub was initiate on a PG,
-   * and that PG is trying to acquire replica resources.
-   * set_reserving_now()/clear_reserving_now() let's the OSD scrub-queue know
-   * we are busy reserving.
-   *
-   * set_reserving_now() returns 'false' if there already is a PG in the
-   * reserving stage of the scrub session.
-   */
-  virtual bool set_reserving_now() = 0;
-  virtual void clear_reserving_now() = 0;
-
   /**
    * Manipulate the 'I am being scrubbed now' Scrubber's flag
    */