osd/scrub: fix replica sub-states

The replica states hierarchy was not properly specified.
Fixing it here.
Also - removing the (never yet invoked) call to
replica_handling_done() in the replica state machine.
That call must precede the reply to the Primary.

Fixes: https://tracker.ceph.com/issues/63509
Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

diff --git a/src/osd/scrubber/scrub_machine.cc b/src/osd/scrubber/scrub_machine.cc
index 40b43b6e07702fedd75d04d5c39ab3daf88d3eea..99286acaa12b175ac8aa0aeaaf9941b898ac0038 100644 (file)
--- a/src/osd/scrubber/scrub_machine.cc
+++ b/src/osd/scrubber/scrub_machine.cc
@@ -674,38 +674,42 @@ ReservedReplica::~ReservedReplica()
 
 ReplicaIdle::ReplicaIdle(my_context ctx)
     : my_base(ctx)
-    , NamedSimply(context<ScrubMachine>().m_scrbr, "ReplicaIdle")
-{
-  dout(10) << "-- state -->> ReplicaIdle" << dendl;
-}
-
-ReplicaIdle::~ReplicaIdle()
+    , NamedSimply(
+         context<ScrubMachine>().m_scrbr,
+         "ReservedReplica/ReplicaIdle")
 {
+  dout(10) << "-- state -->> ReservedReplica/ReplicaIdle" << dendl;
 }
 
+ReplicaIdle::~ReplicaIdle() = default;
 
 // ----------------------- ReplicaActiveOp --------------------------------
 
 ReplicaActiveOp::ReplicaActiveOp(my_context ctx)
     : my_base(ctx)
-    , NamedSimply(context<ScrubMachine>().m_scrbr, "ReplicaActiveOp")
+    , NamedSimply(
+         context<ScrubMachine>().m_scrbr,
+         "ReservedReplica/ReplicaActiveOp")
 {
-  dout(10) << "-- state -->> ReplicaActiveOp" << dendl;
+  dout(10) << "-- state -->> ReservedReplica/ReplicaActiveOp" << dendl;
 }
 
-ReplicaActiveOp::~ReplicaActiveOp()
-{
-  DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases
-  scrbr->replica_handling_done();
-}
+/**
+ * \note: here is too late to call replica_handling_done(). See the
+ * comment in build_replica_map_chunk()
+ */
+ReplicaActiveOp::~ReplicaActiveOp() = default;
 
 // ----------------------- ReplicaWaitUpdates --------------------------------
 
 ReplicaWaitUpdates::ReplicaWaitUpdates(my_context ctx)
     : my_base(ctx)
-    , NamedSimply(context<ScrubMachine>().m_scrbr, "ReplicaWaitUpdates")
+    , NamedSimply(
+         context<ScrubMachine>().m_scrbr,
+         "ReservedReplica/ReplicaActiveOp/ReplicaWaitUpdates")
 {
-  dout(10) << "-- state -->> ReplicaWaitUpdates" << dendl;
+  dout(10) << "-- state -->> ReservedReplica/ReplicaActiveOp/ReplicaWaitUpdates"
+          << dendl;
   DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases
   scrbr->on_replica_init();
 }
@@ -732,10 +736,13 @@ sc::result ReplicaWaitUpdates::react(const ReplicaPushesUpd&)
 
 ReplicaBuildingMap::ReplicaBuildingMap(my_context ctx)
     : my_base(ctx)
-    , NamedSimply(context<ScrubMachine>().m_scrbr, "ReplicaBuildingMap")
+    , NamedSimply(
+         context<ScrubMachine>().m_scrbr,
+         "ReservedReplica/ReplicaActiveOp/ReplicaBuildingMap")
 {
   DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases
-  dout(10) << "-- state -->> ReplicaBuildingMap" << dendl;
+  dout(10) << "-- state -->> ReservedReplica/ReplicaActiveOp/ReplicaBuildingMap"
+          << dendl;
   // and as we might have skipped ReplicaWaitUpdates:
   scrbr->on_replica_init();
   post_event(SchedReplica{});

diff --git a/src/osd/scrubber/scrub_machine.h b/src/osd/scrubber/scrub_machine.h
index cbce07fe183c6d21e0238a8f2ddfe71d642b4a0d..a67a85b8e112bf954a7ed09ca6d22b3f21215330 100644 (file)
--- a/src/osd/scrubber/scrub_machine.h
+++ b/src/osd/scrubber/scrub_machine.h
@@ -628,7 +628,7 @@ struct ReplicaIdle : sc::state<ReplicaIdle, ReservedReplica>,
 };
 
 /**
- * ReservedActiveOp
+ * ReplicaActiveOp
  *
  * Lifetime matches handling for a single map request op
  */
@@ -646,7 +646,7 @@ struct ReplicaActiveOp
  * - the details of the Primary's request were internalized by PgScrubber;
  * - 'active' scrubbing is set
  */
-struct ReplicaWaitUpdates : sc::state<ReplicaWaitUpdates, ReservedReplica>,
+struct ReplicaWaitUpdates : sc::state<ReplicaWaitUpdates, ReplicaActiveOp>,
                            NamedSimply {
   explicit ReplicaWaitUpdates(my_context ctx);
   using reactions = mpl::list<sc::custom_reaction<ReplicaPushesUpd>>;
@@ -655,7 +655,7 @@ struct ReplicaWaitUpdates : sc::state<ReplicaWaitUpdates, ReservedReplica>,
 };
 
 
-struct ReplicaBuildingMap : sc::state<ReplicaBuildingMap, ReservedReplica>
+struct ReplicaBuildingMap : sc::state<ReplicaBuildingMap, ReplicaActiveOp>
                          , NamedSimply {
   explicit ReplicaBuildingMap(my_context ctx);
   using reactions = mpl::list<sc::custom_reaction<SchedReplica>>;