Merge pull request ceph#53151 from xxhdx1985126/wip-crimson-backfill-fixes

crimson/osd/backfill_state: fixes two corner cases in backfilling

Reviewed-by: Radosław Zarzyński <[email protected]>

Author: Matan-B

src/crimson/osd/backfill_state.cc

@@ -73,6 +73,8 @@ BackfillState::Initial::Initial(my_context ctx)
   }
   ceph_assert(peering_state().get_backfill_targets().size());
   ceph_assert(!backfill_state().last_backfill_started.is_max());
+  backfill_state().replicas_in_backfill =
+    peering_state().get_backfill_targets().size();
 }
 
 boost::statechart::result
@@ -350,9 +352,14 @@ BackfillState::Enqueuing::Enqueuing(my_context ctx)
     logger().debug("{}: reached end for current local chunk",
                    __func__);
     post_event(RequestPrimaryScanning{});
-  } else if (backfill_state().progress_tracker->tracked_objects_completed()) {
-    post_event(RequestDone{});
   } else {
+    if (backfill_state().progress_tracker->tracked_objects_completed()
+	&& Enqueuing::all_enqueued(peering_state(),
+				   backfill_state().backfill_info,
+				   backfill_state().peer_backfill_info)) {
+      backfill_state().last_backfill_started = hobject_t::get_max();
+      backfill_listener().update_peers_last_backfill(hobject_t::get_max());
+    }
     logger().debug("{}: reached end for both local and all peers "
                    "but still has in-flight operations", __func__);
     post_event(RequestWaiting{});
@@ -470,8 +477,6 @@ BackfillState::Waiting::react(ObjectPushed evt)
                                backfill_state().backfill_info,
                                backfill_state().peer_backfill_info)) {
     return transit<Enqueuing>();
-  } else if (backfill_state().progress_tracker->tracked_objects_completed()) {
-    return transit<Done>();
   } else {
     // we still have something to wait on
     logger().debug("Waiting::react() on ObjectPushed; still waiting");

src/crimson/osd/backfill_state.h

@@ -55,6 +55,9 @@ struct BackfillState {
   struct Triggered : sc::event<Triggered> {
   };
 
+  struct RequestDone : sc::event<RequestDone> {
+  };
+
 private:
   // internal events
   struct RequestPrimaryScanning : sc::event<RequestPrimaryScanning> {
@@ -66,9 +69,6 @@ struct BackfillState {
   struct RequestWaiting : sc::event<RequestWaiting> {
   };
 
-  struct RequestDone : sc::event<RequestDone> {
-  };
-
   class ProgressTracker;
 
 public:
@@ -149,7 +149,6 @@ struct BackfillState {
       sc::transition<RequestPrimaryScanning, PrimaryScanning>,
       sc::transition<RequestReplicasScanning, ReplicasScanning>,
       sc::transition<RequestWaiting, Waiting>,
-      sc::transition<RequestDone, Done>,
       sc::transition<sc::event_base, Crashed>>;
     explicit Enqueuing(my_context);
 
@@ -206,6 +205,7 @@ struct BackfillState {
     using reactions = boost::mpl::list<
       sc::custom_reaction<ObjectPushed>,
       sc::custom_reaction<PrimaryScanned>,
+      sc::transition<RequestDone, Done>,
       sc::transition<sc::event_base, Crashed>>;
     explicit PrimaryScanning(my_context);
     sc::result react(ObjectPushed);
@@ -218,6 +218,7 @@ struct BackfillState {
     using reactions = boost::mpl::list<
       sc::custom_reaction<ObjectPushed>,
       sc::custom_reaction<ReplicaScanned>,
+      sc::transition<RequestDone, Done>,
       sc::transition<sc::event_base, Crashed>>;
     explicit ReplicasScanning(my_context);
     // collect scanning result; if all results are collected, transition
@@ -241,6 +242,7 @@ struct BackfillState {
                    StateHelper<Waiting> {
     using reactions = boost::mpl::list<
       sc::custom_reaction<ObjectPushed>,
+      sc::transition<RequestDone, Done>,
       sc::transition<sc::event_base, Crashed>>;
     explicit Waiting(my_context);
     sc::result react(ObjectPushed);
@@ -266,12 +268,21 @@ struct BackfillState {
   hobject_t get_last_backfill_started() const {
     return last_backfill_started;
   }
+
+  void backfill_target_done() {
+    ceph_assert(replicas_in_backfill > 0);
+    replicas_in_backfill--;
+    if (!replicas_in_backfill) {
+      backfill_machine.process_event(RequestDone{});
+    }
+  }
 private:
   hobject_t last_backfill_started;
   BackfillInterval backfill_info;
   std::map<pg_shard_t, BackfillInterval> peer_backfill_info;
   BackfillMachine backfill_machine;
   std::unique_ptr<ProgressTracker> progress_tracker;
+  size_t replicas_in_backfill = 0;
 };
 
 // BackfillListener -- an interface used by the backfill FSM to request

src/crimson/osd/pg_recovery.h

@@ -36,6 +36,9 @@ class PGRecovery : public crimson::osd::BackfillState::BackfillListener {
   void on_backfill_reserved();
   void dispatch_backfill_event(
     boost::intrusive_ptr<const boost::statechart::event_base> evt);
+  void backfill_target_finished() {
+    backfill_state->backfill_target_done();
+  }
 
   seastar::future<> stop() { return seastar::now(); }
 private:

src/crimson/osd/recovery_backend.cc

@@ -167,8 +167,8 @@ RecoveryBackend::handle_backfill_finish_ack(
   logger().debug("{}", __func__);
   ceph_assert(pg.is_primary());
   ceph_assert(crimson::common::local_conf()->osd_kill_backfill_at != 3);
-  // TODO:
-  // finish_recovery_op(hobject_t::get_max());
+  auto recovery_handler = pg.get_recovery_handler();
+  recovery_handler->backfill_target_finished();
   return seastar::now();
 }
 
@@ -264,8 +264,9 @@ RecoveryBackend::scan_for_backfill(
       bi.end = std::move(next);
       bi.version = pg.get_info().last_update;
       bi.objects = std::move(*version_map);
-      logger().debug("{} BackfillInterval filled, leaving",
-                     "scan_for_backfill");
+      logger().debug("{} BackfillInterval filled, leaving, {}",
+                     "scan_for_backfill",
+		     bi);
       return seastar::make_ready_future<BackfillInterval>(std::move(bi));
     });
   });

src/test/crimson/test_backfill.cc

@@ -1,3 +1,6 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
 #include <algorithm>
 #include <cstdlib>
 #include <deque>
@@ -308,6 +311,9 @@ void BackfillFixture::maybe_flush()
 void BackfillFixture::update_peers_last_backfill(
   const hobject_t& new_last_backfill)
 {
+  if (new_last_backfill.is_max()) {
+    schedule_event(crimson::osd::BackfillState::RequestDone{});
+  }
 }
 
 bool BackfillFixture::budget_available() const
@@ -355,6 +361,7 @@ TEST(backfill, same_primary_same_replica)
     reference_store.objs
   ).get_result();
 
+  cluster_fixture.next_round();
   cluster_fixture.next_round();
   EXPECT_CALL(cluster_fixture, backfilled);
   cluster_fixture.next_round();
@@ -377,6 +384,7 @@ TEST(backfill, one_empty_replica)
   cluster_fixture.next_round();
   cluster_fixture.next_round();
   cluster_fixture.next_round(2);
+  cluster_fixture.next_round();
   EXPECT_CALL(cluster_fixture, backfilled);
   cluster_fixture.next_round();
   EXPECT_TRUE(cluster_fixture.all_stores_look_like(reference_store));