PeeringState: restructure activate to avoid snap_trimq

In particular, don't use snap_trimq when manipulating
info.purged_snaps.  Pass the resulting to_trim set
into on_activate.

Signed-off-by: Samuel Just <sjust@redhat.com>

5 files changed, 36 insertions, 31 deletions

diff --git a/src/osd/PG.h b/src/osd/PG.h
index e4bdd94e903..08367076ae6 100644
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -458,6 +458,15 @@ public:
     try_mark_clean();
     return finish_recovery();
   }
+
+  void on_activate(interval_set<snapid_t> snaps) override {
+    ceph_assert(scrubber.callbacks.empty());
+    ceph_assert(callbacks_for_degraded_object.empty());
+    snap_trimq = snaps;
+    release_pg_backoffs();
+    projected_last_update = info.last_update;
+  }
+
   void on_activate_committed() override;
 
   void on_active_actmap() override;
diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc
index 5486008c774..ad1df1979bf 100644
--- a/src/osd/PeeringState.cc
+++ b/src/osd/PeeringState.cc
@@ -1849,8 +1849,6 @@ void PeeringState::activate(
   PeeringCtx *ctx)
 {
   ceph_assert(!is_peered());
-  // ceph_assert(scrubber.callbacks.empty()); TODOSAM
-  // ceph_assert(callbacks_for_degraded_object.empty()); TODOSAM
 
   // twiddle pg state
   state_clear(PG_STATE_DOWN);
@@ -1899,29 +1897,44 @@ void PeeringState::activate(
 	get_osdmap_epoch(),
 	activation_epoch)));
 
+  // init complete pointer
+  if (missing.num_missing() == 0) {
+    psdout(10) << "activate - no missing, moving last_complete " << info.last_complete
+	     << " -> " << info.last_update << dendl;
+    info.last_complete = info.last_update;
+    info.stats.stats.sum.num_objects_missing = 0;
+    pg_log.reset_recovery_pointers();
+  } else {
+    psdout(10) << "activate - not complete, " << missing << dendl;
+    info.stats.stats.sum.num_objects_missing = missing.num_missing();
+    pg_log.activate_not_complete(info);
+  }
+
+  pg->log_weirdness();
+
   if (is_primary()) {
     // initialize snap_trimq
+    interval_set<snapid_t> to_trim;
     if (get_osdmap()->require_osd_release < CEPH_RELEASE_MIMIC) {
       psdout(20) << "activate - purged_snaps " << info.purged_snaps
 		 << " cached_removed_snaps " << pool.cached_removed_snaps
 		 << dendl;
-      pg->snap_trimq = pool.cached_removed_snaps;
+      to_trim = pool.cached_removed_snaps;
     } else {
       auto& removed_snaps_queue = get_osdmap()->get_removed_snaps_queue();
       auto p = removed_snaps_queue.find(info.pgid.pgid.pool());
-      pg->snap_trimq.clear();
       if (p != removed_snaps_queue.end()) {
 	dout(20) << "activate - purged_snaps " << info.purged_snaps
 		 << " removed_snaps " << p->second
 		 << dendl;
 	for (auto q : p->second) {
-	  pg->snap_trimq.insert(q.first, q.second);
+	  to_trim.insert(q.first, q.second);
 	}
       }
     }
     interval_set<snapid_t> purged;
-    purged.intersection_of(pg->snap_trimq, info.purged_snaps);
-    pg->snap_trimq.subtract(purged);
+    purged.intersection_of(to_trim, info.purged_snaps);
+    to_trim.subtract(purged);
 
     if (get_osdmap()->require_osd_release >= CEPH_RELEASE_MIMIC) {
       // adjust purged_snaps: PG may have been inactive while snaps were pruned
@@ -1930,25 +1943,7 @@ void PeeringState::activate(
       // the queue.
       info.purged_snaps.swap(purged);
     }
-  }
 
-  // init complete pointer
-  if (missing.num_missing() == 0) {
-    psdout(10) << "activate - no missing, moving last_complete " << info.last_complete
-	     << " -> " << info.last_update << dendl;
-    info.last_complete = info.last_update;
-    info.stats.stats.sum.num_objects_missing = 0;
-    pg_log.reset_recovery_pointers();
-  } else {
-    psdout(10) << "activate - not complete, " << missing << dendl;
-    info.stats.stats.sum.num_objects_missing = missing.num_missing();
-    pg_log.activate_not_complete(info);
-  }
-
-  pg->log_weirdness();
-
-  // if primary..
-  if (is_primary()) {
     ceph_assert(ctx);
     // start up replicas
 
@@ -2165,6 +2160,7 @@ void PeeringState::activate(
 
       // Always call now so _update_calc_stats() will be accurate
       pg->discover_all_missing(query_map);
+
     }
 
     // num_objects_degraded if calculated should reflect this too, unless no
@@ -2174,8 +2170,7 @@ void PeeringState::activate(
     }
 
     state_set(PG_STATE_ACTIVATING);
-    pg->release_pg_backoffs();
-    pg->projected_last_update = info.last_update;
+    pl->on_activate(std::move(to_trim));
   }
   if (acting.size() >= pool.info.min_size) {
     PG::PGLogEntryHandler handler{pg, &t};
@@ -3804,7 +3799,7 @@ boost::statechart::result PeeringState::Active::react(const AllReplicasActivated
     pg->waiting_for_flush.swap(pg->waiting_for_peered);
   }
 
-  pl->on_activate();
+  pl->on_activate_complete();
 
   return discard_event();
 }
diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h
index ce53bdd49cd..4fc8f15ee6d 100644
--- a/src/osd/PeeringState.h
+++ b/src/osd/PeeringState.h
@@ -121,7 +121,8 @@ public:
     virtual void on_pool_change() = 0;
     virtual void on_role_change() = 0;
     virtual void on_change(ObjectStore::Transaction *t) = 0;
-    virtual void on_activate() = 0;
+    virtual void on_activate(interval_set<snapid_t> to_trim) = 0;
+    virtual void on_activate_complete() = 0;
     virtual void on_new_interval() = 0;
     virtual Context *on_clean() = 0;
     virtual void on_activate_committed() = 0;
diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index b0e2e817de2..8cd0ab3ada1 100644
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -12136,7 +12136,7 @@ void PrimaryLogPG::on_shutdown()
   }
 }
 
-void PrimaryLogPG::on_activate()
+void PrimaryLogPG::on_activate_complete()
 {
   // all clean?
   if (needs_recovery()) {
diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h
index 6d0bd4315b1..bf1eb300cb6 100644
--- a/src/osd/PrimaryLogPG.h
+++ b/src/osd/PrimaryLogPG.h
@@ -1863,7 +1863,7 @@ public:
   void plpg_on_new_interval() override;
   void clear_async_reads();
   void on_change(ObjectStore::Transaction *t) override;
-  void on_activate() override;
+  void on_activate_complete() override;
   void on_flushed() override;
   void on_removal(ObjectStore::Transaction *t) override;
   void on_shutdown() override;