From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Thu, 21 May 2020 18:26:07 +0000 (+0200)
Subject: crimson/osd: synchronize BackfillRecovery with PeeringEvent.
X-Git-Tag: wip-pdonnell-testing-20200918.022351~657^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ecb0f20e8d00c7fd0bc5bc78853e303be69453ce;p=ceph-ci.git

crimson/osd: synchronize BackfillRecovery with PeeringEvent.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
---

diff --git a/src/crimson/osd/backfill_facades.h b/src/crimson/osd/backfill_facades.h
index 645fc898737..d6d6f91e363 100644
--- a/src/crimson/osd/backfill_facades.h
+++ b/src/crimson/osd/backfill_facades.h
@@ -42,6 +42,10 @@ struct BackfillState::PeeringFacade {
     return peering_state.update_complete_backfill_object_stats(hoid, stats);
   }
 
+  bool is_backfilling() const {
+    return peering_state.is_backfilling();
+  }
+
   PeeringFacade(PeeringState& peering_state)
     : peering_state(peering_state) {
   }
diff --git a/src/crimson/osd/backfill_state.cc b/src/crimson/osd/backfill_state.cc
index 7181e31a3a6..44f9fa0177d 100644
--- a/src/crimson/osd/backfill_state.cc
+++ b/src/crimson/osd/backfill_state.cc
@@ -67,6 +67,7 @@ BackfillState::Initial::react(const BackfillState::Triggered& evt)
   logger().debug("{}: backfill triggered", __func__);
   ceph_assert(backfill_state().last_backfill_started == \
               peering_state().earliest_backfill());
+  ceph_assert(peering_state().is_backfilling());
   // initialize BackfillIntervals
   for (const auto& bt : peering_state().get_backfill_targets()) {
     backfill_state().peer_backfill_info[bt].reset(
diff --git a/src/crimson/osd/osd_operations/background_recovery.cc b/src/crimson/osd/osd_operations/background_recovery.cc
index ae7f5c4258b..40407e708d7 100644
--- a/src/crimson/osd/osd_operations/background_recovery.cc
+++ b/src/crimson/osd/osd_operations/background_recovery.cc
@@ -108,6 +108,11 @@ seastar::future<bool> PglogBasedRecovery::do_recovery()
       crimson::common::local_conf()->osd_recovery_max_single_start));
 }
 
+BackfillRecovery::BackfillRecoveryPipeline &BackfillRecovery::bp(PG &pg)
+{
+  return pg.backfill_pipeline;
+}
+
 seastar::future<bool> BackfillRecovery::do_recovery()
 {
   logger().debug("{}", __func__);
@@ -122,7 +127,8 @@ seastar::future<bool> BackfillRecovery::do_recovery()
     // process_event() of our boost::statechart machine is non-reentrant.
     // with the backfill_pipeline we protect it from a second entry from
     // the implementation of BackfillListener.
-    handle.enter(pg->backfill_pipeline.process)
+    // additionally, this stage serves to synchronize with PeeringEvent.
+    handle.enter(bp(*pg).process)
   ).then([this] {
     pg->get_recovery_handler()->dispatch_backfill_event(std::move(evt));
     return seastar::make_ready_future<bool>(false);
diff --git a/src/crimson/osd/osd_operations/background_recovery.h b/src/crimson/osd/osd_operations/background_recovery.h
index ae439b9300b..1fa0e1df30e 100644
--- a/src/crimson/osd/osd_operations/background_recovery.h
+++ b/src/crimson/osd/osd_operations/background_recovery.h
@@ -83,6 +83,7 @@ public:
       "BackfillRecovery::PGPipeline::process"
     };
     friend class BackfillRecovery;
+    friend class PeeringEvent;
   };
 
   template <class EventT>
@@ -91,6 +92,8 @@ public:
     ShardServices &ss,
     epoch_t epoch_started,
     const EventT& evt);
+
+  static BackfillRecoveryPipeline &bp(PG &pg);
 };
 
 template <class EventT>
diff --git a/src/crimson/osd/osd_operations/peering_event.cc b/src/crimson/osd/osd_operations/peering_event.cc
index a7255bbe7a9..d3c6ccf817f 100644
--- a/src/crimson/osd/osd_operations/peering_event.cc
+++ b/src/crimson/osd/osd_operations/peering_event.cc
@@ -76,6 +76,11 @@ seastar::future<> PeeringEvent::start()
 	  pg->osdmap_gate.wait_for_map(evt.get_epoch_sent()));
       }).then([this, pg](auto) {
 	return with_blocking_future(handle.enter(pp(*pg).process));
+      }).then([this, pg] {
+        // TODO: likely we should synchronize also with the pg log-based
+        // recovery.
+	return with_blocking_future(
+          handle.enter(BackfillRecovery::bp(*pg).process));
       }).then([this, pg] {
 	pg->do_peering_event(evt, ctx);
 	handle.exit();
diff --git a/src/crimson/osd/pg_recovery.cc b/src/crimson/osd/pg_recovery.cc
index 4a9ec12a37b..6028a457e46 100644
--- a/src/crimson/osd/pg_recovery.cc
+++ b/src/crimson/osd/pg_recovery.cc
@@ -545,5 +545,12 @@ void PGRecovery::on_backfill_reserved()
     std::make_unique<BackfillState::PeeringFacade>(pg->get_peering_state()),
     std::make_unique<BackfillState::PGFacade>(
       *static_cast<crimson::osd::PG*>(pg)));
+  // yes, it's **not** backfilling yet. The PG_STATE_BACKFILLING
+  // will be set after on_backfill_reserved() returns.
+  // Backfill needs to take this into consideration when scheduling
+  // events -- they must be mutually exclusive with PeeringEvent
+  // instances. Otherwise the execution might begin without having
+  // the state updated.
+  ceph_assert(!pg->get_peering_state().is_backfilling());
   start_backfill_recovery(BackfillState::Triggered{});
 }