osd/scrub: set_reserving_now() signature modified

set_reserving_now() can now return a failure status, indicating
a race between two PGs to start scrubbing on the same OSD.

The scrubber FSM is modified to handle the failure.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

diff --git a/src/osd/scrubber/osd_scrub_sched.cc b/src/osd/scrubber/osd_scrub_sched.cc
index 9170e369ac47f0a729659f457b7f3c80f890e606..baec441f2623e70c866fc59a53490107047d5c86 100644 (file)
--- a/src/osd/scrubber/osd_scrub_sched.cc
+++ b/src/osd/scrubber/osd_scrub_sched.cc
@@ -490,3 +490,22 @@ int ScrubQueue::get_blocked_pgs_count() const
 {
   return blocked_scrubs_cnt;
 }
+
+// ////////////////////////////////////////////////////////////////////////// //
+// ScrubQueue - maintaining the 'some PG is reserving' flag
+
+bool ScrubQueue::set_reserving_now()
+{
+  auto was_set = a_pg_is_reserving.exchange(true);
+  return !was_set;
+}
+
+void ScrubQueue::clear_reserving_now()
+{
+  a_pg_is_reserving = false;
+}
+
+bool ScrubQueue::is_reserving_now() const
+{
+  return a_pg_is_reserving;
+}

diff --git a/src/osd/scrubber/osd_scrub_sched.h b/src/osd/scrubber/osd_scrub_sched.h
index 99b9d30a49f73d2335ee70b2781b1fae97d91281..4727f3c7d5b7dcc64379dbb06737219f75d2cf8d 100644 (file)
--- a/src/osd/scrubber/osd_scrub_sched.h
+++ b/src/osd/scrubber/osd_scrub_sched.h
@@ -277,10 +277,22 @@ class ScrubQueue {
   /**
    * 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.
    */
-  void set_reserving_now() { a_pg_is_reserving = true; }
-  void clear_reserving_now() { a_pg_is_reserving = false; }
-  bool is_reserving_now() const { return a_pg_is_reserving; }
+
+  /**
+   * 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();
+  void clear_reserving_now();
+  bool is_reserving_now() const;
+
 
   bool can_inc_scrubs() const;
   bool inc_scrubs_local();

diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc
index 327e9f2b2f87533515e45f2fe398c447dec9c35b..339fe1e7155ac9ae3ec9032a2cd303c78b70bc73 100644 (file)
--- a/src/osd/scrubber/pg_scrubber.cc
+++ b/src/osd/scrubber/pg_scrubber.cc
@@ -1687,9 +1687,9 @@ void PgScrubber::on_replica_reservation_timeout()
   }
 }
 
-void PgScrubber::set_reserving_now()
+bool PgScrubber::set_reserving_now()
 {
-  m_osds->get_scrub_services().set_reserving_now();
+  return m_osds->get_scrub_services().set_reserving_now();
 }
 
 void PgScrubber::clear_reserving_now()

diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h
index 42ac2b48807c414002f129555658f2a4dcb2ebad..5810f1fd986fa72f53758a3ec6c57996b5ff061e 100644 (file)
--- a/src/osd/scrubber/pg_scrubber.h
+++ b/src/osd/scrubber/pg_scrubber.h
@@ -547,7 +547,7 @@ class PgScrubber : public ScrubPgIF,
 
   void reserve_replicas() final;
 
-  void set_reserving_now() final;
+  bool set_reserving_now() final;
   void clear_reserving_now() final;
 
   [[nodiscard]] bool was_epoch_changed() const final;

diff --git a/src/osd/scrubber/scrub_machine.cc b/src/osd/scrubber/scrub_machine.cc
index c3bb89b4dad2c6f8c0f0788d4bce9a7c2d0e0330..0d52d5b76d77b61d8727bff70eec91316e90099c 100644 (file)
--- a/src/osd/scrubber/scrub_machine.cc
+++ b/src/osd/scrubber/scrub_machine.cc
@@ -120,7 +120,14 @@ ReservingReplicas::ReservingReplicas(my_context ctx)
 
   // prevent the OSD from starting another scrub while we are trying to secure
   // replicas resources
-  scrbr->set_reserving_now();
+  if (!scrbr->set_reserving_now()) {
+    dout(1) << "ReservingReplicas::ReservingReplicas() some other PG is "
+               "already reserving replicas resources"
+            << dendl;
+    post_event(ReservationFailure{});
+    return;
+  }
+  m_holding_isreserving_flag = true;
   scrbr->reserve_replicas();
 
   auto timeout = scrbr->get_cct()->_conf.get_val<
@@ -136,7 +143,9 @@ ReservingReplicas::ReservingReplicas(my_context ctx)
 ReservingReplicas::~ReservingReplicas()
 {
   DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases
-  scrbr->clear_reserving_now();
+  if (m_holding_isreserving_flag) {
+    scrbr->clear_reserving_now();
+  }
 }
 
 sc::result ReservingReplicas::react(const ReservationTimeout&)

diff --git a/src/osd/scrubber/scrub_machine.h b/src/osd/scrubber/scrub_machine.h
index ffa6f4e6d3bf76366e306fb78ecfd4167f88e870..658abfa494f1b07c92ca8990ad107ba12d5f245f 100644 (file)
--- a/src/osd/scrubber/scrub_machine.h
+++ b/src/osd/scrubber/scrub_machine.h
@@ -328,6 +328,9 @@ struct ReservingReplicas : sc::state<ReservingReplicas, ScrubMachine>,
     ceph::coarse_real_clock::now();
   ScrubMachine::timer_event_token_t m_timeout_token;
 
+  /// if true - we must 'clear_reserving_now()' upon exit
+  bool m_holding_isreserving_flag{false};
+
   sc::result react(const FullReset&);
 
   sc::result react(const ReservationTimeout&);

diff --git a/src/osd/scrubber/scrub_machine_lstnr.h b/src/osd/scrubber/scrub_machine_lstnr.h
index 8f4dd8201c105e7f352cda6062d974eb8ea96858..cfef666e1b118594a73cf7bb8e4e9b433bc04b93 100644 (file)
--- a/src/osd/scrubber/scrub_machine_lstnr.h
+++ b/src/osd/scrubber/scrub_machine_lstnr.h
@@ -190,8 +190,11 @@ struct ScrubMachineListener {
    * 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 void set_reserving_now() = 0;
+  virtual bool set_reserving_now() = 0;
   virtual void clear_reserving_now() = 0;
 
   /**