osd/: add helpers to add remaining info dirtiers into PeeringState

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

7 files changed, 154 insertions, 102 deletions

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index e63af28e2d9..38d7f495c45 100644
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -191,8 +191,6 @@ PG::PG(OSDService *o, OSDMapRef curmap,
   upset(recovery_state.upset),
   actingset(recovery_state.actingset),
   acting_recovery_backfill(recovery_state.acting_recovery_backfill),
-  dirty_info(recovery_state.dirty_info),
-  dirty_big_info(recovery_state.dirty_big_info),
   info(recovery_state.info),
   past_intervals(recovery_state.past_intervals),
   pg_log(recovery_state.pg_log),
@@ -262,8 +260,7 @@ void PG::lock(bool no_lockdep) const
 {
   _lock.Lock(no_lockdep);
   // if we have unrecorded dirty state with the lock dropped, there is a bug
-  ceph_assert(!dirty_info);
-  ceph_assert(!dirty_big_info);
+  ceph_assert(!recovery_state.debug_has_dirty_state());
 
   dout(30) << "lock" << dendl;
 }
@@ -925,9 +922,7 @@ void PG::upgrade(ObjectStore *store)
     t.omap_setkeys(coll, pgmeta_oid, v);
   }
 
-  dirty_info = true;
-  dirty_big_info = true;
-  write_if_dirty(t);
+  recovery_state.force_write_state(t);
 
   ObjectStore::CollectionHandle ch = store->open_collection(coll);
   int r = store->queue_transaction(ch, std::move(t));
@@ -3885,8 +3880,6 @@ void PG::do_delete_work(ObjectStore::Transaction *t)
 	      info.pgid.get_split_bits(pool.info.get_pg_num()));
       _init(*t, info.pgid, &pool.info);
       recovery_state.reset_last_persisted();
-      dirty_info = true;
-      dirty_big_info = true;
     } else {
       recovery_state.set_delete_complete();
 
diff --git a/src/osd/PG.h b/src/osd/PG.h
index e3fbf1e6526..933a1476c1f 100644
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -188,8 +188,6 @@ protected:
   set<pg_shard_t> &upset;
   set<pg_shard_t> &actingset;
   set<pg_shard_t> &acting_recovery_backfill;
-  bool &dirty_info;
-  bool &dirty_big_info;
   pg_info_t &info;
   PastIntervals &past_intervals;
   PGLog &pg_log;
@@ -250,8 +248,7 @@ public:
   void lock(bool no_lockdep = false) const;
   void unlock() const {
     //generic_dout(0) << this << " " << info.pgid << " unlock" << dendl;
-    ceph_assert(!dirty_info);
-    ceph_assert(!dirty_big_info);
+    ceph_assert(!recovery_state.debug_has_dirty_state());
     _lock.Unlock();
   }
   bool is_locked() const {
diff --git a/src/osd/PGLog.cc b/src/osd/PGLog.cc
index bd9cbc1b71b..64d923d5123 100644
--- a/src/osd/PGLog.cc
+++ b/src/osd/PGLog.cc
@@ -621,7 +621,7 @@ void PGLog::write_log_and_missing(
   const ghobject_t &log_oid,
   bool require_rollback)
 {
-  if (is_dirty()) {
+  if (needs_write()) {
     dout(6) << "write_log_and_missing with: "
 	     << "dirty_to: " << dirty_to
 	     << ", dirty_from: " << dirty_from
diff --git a/src/osd/PGLog.h b/src/osd/PGLog.h
index 7ac66eda870..0ad9899d505 100644
--- a/src/osd/PGLog.h
+++ b/src/osd/PGLog.h
@@ -588,9 +588,12 @@ protected:
       dirty_from_dups = from;
   }
 public:
+  bool needs_write() const {
+    return !touched_log || is_dirty();
+  }
+
   bool is_dirty() const {
-    return !touched_log ||
-      (dirty_to != eversion_t()) ||
+    return (dirty_to != eversion_t()) ||
       (dirty_from != eversion_t::max()) ||
       (writeout_from != eversion_t::max()) ||
       !(trimmed.empty()) ||
@@ -601,6 +604,7 @@ public:
       (write_from_dups != eversion_t::max()) ||
       rebuilt_missing_with_deletes;
   }
+
   void mark_log_for_rewrite() {
     mark_dirty_to(eversion_t::max());
     mark_dirty_from(eversion_t());
diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc
index 00ccd4b5dd3..d0de0f8f013 100644
--- a/src/osd/PeeringState.cc
+++ b/src/osd/PeeringState.cc
@@ -3589,6 +3589,74 @@ void PeeringState::append_log(
   write_if_dirty(t);
 }
 
+void PeeringState::recover_got(
+  const hobject_t &oid, eversion_t v,
+  bool is_delete,
+  ObjectStore::Transaction &t)
+{
+  if (v > pg_log.get_can_rollback_to()) {
+    /* This can only happen during a repair, and even then, it would
+     * be one heck of a race.  If we are repairing the object, the
+     * write in question must be fully committed, so it's not valid
+     * to roll it back anyway (and we'll be rolled forward shortly
+     * anyway) */
+    PGLog::LogEntryHandlerRef handler{pl->get_log_handler(&t)};
+    pg_log.roll_forward_to(v, handler.get());
+  }
+
+  psdout(10) << "got missing " << oid << " v " << v << dendl;
+  pg_log.recover_got(oid, v, info);
+  if (pg_log.get_log().complete_to != pg_log.get_log().log.end()) {
+    psdout(10) << "last_complete now " << info.last_complete
+	       << " log.complete_to " << pg_log.get_log().complete_to->version
+	       << dendl;
+  } else {
+    psdout(10) << "last_complete now " << info.last_complete
+	       << " log.complete_to at end" << dendl;
+    //below is not true in the repair case.
+    //assert(missing.num_missing() == 0);  // otherwise, complete_to was wrong.
+    ceph_assert(info.last_complete == info.last_update);
+  }
+
+  if (is_primary()) {
+    ceph_assert(missing_loc.needs_recovery(oid));
+    if (!is_delete)
+      missing_loc.add_location(oid, pg_whoami);
+  }
+
+  // update pg
+  dirty_info = true;
+  write_if_dirty(t);
+}
+
+void PeeringState::update_backfill_progress(
+  const hobject_t &updated_backfill,
+  const pg_stat_t &updated_stats,
+  bool preserve_local_num_bytes,
+  ObjectStore::Transaction &t) {
+  info.set_last_backfill(updated_backfill);
+  if (preserve_local_num_bytes) {
+    psdout(25) << __func__ << " primary " << updated_stats.stats.sum.num_bytes
+	       << " local " << info.stats.stats.sum.num_bytes << dendl;
+    int64_t bytes = info.stats.stats.sum.num_bytes;
+    info.stats = updated_stats;
+    info.stats.stats.sum.num_bytes = bytes;
+  } else {
+    psdout(20) << __func__ << " final " << updated_stats.stats.sum.num_bytes
+	       << " replaces local " << info.stats.stats.sum.num_bytes << dendl;
+    info.stats = updated_stats;
+  }
+
+  dirty_info = true;
+  write_if_dirty(t);
+}
+
+void PeeringState::adjust_purged_snaps(
+  std::function<void(interval_set<snapid_t> &snaps)> f) {
+  f(info.purged_snaps);
+  dirty_info = true;
+  dirty_big_info = true;
+}
 
 /*------------ Peering State Machine----------------*/
 #undef dout_prefix
@@ -5554,6 +5622,16 @@ PeeringState::Deleting::Deleting(my_context ctx)
   DECLARE_LOCALS
   ps->deleting = true;
   ObjectStore::Transaction* t = context<PeeringMachine>().get_cur_transaction();
+
+  // adjust info to backfill
+  ps->info.set_last_backfill(hobject_t());
+  ps->pg_log.reset_backfill();
+  ps->dirty_info = true;
+
+  // clear log
+  PGLog::LogEntryHandlerRef rollbacker{pl->get_log_handler(t)};
+  ps->pg_log.roll_forward(rollbacker.get());
+
   pl->on_removal(t);
 }
 
diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h
index 3857f6d5827..b8f2b0ea58d 100644
--- a/src/osd/PeeringState.h
+++ b/src/osd/PeeringState.h
@@ -1419,24 +1419,8 @@ public:
     boost::optional<eversion_t> trim_to,
     boost::optional<eversion_t> roll_forward_to);
 
-  /**
-   * Merge entries updating missing as necessary on all
-   * acting_recovery_backfill logs and missings (also missing_loc)
-   */
-  void merge_new_log_entries(
-    const mempool::osd_pglog::list<pg_log_entry_t> &entries,
-    ObjectStore::Transaction &t,
-    boost::optional<eversion_t> trim_to,
-    boost::optional<eversion_t> roll_forward_to);
-
   void add_log_entry(const pg_log_entry_t& e, bool applied);
-  void append_log(
-    const vector<pg_log_entry_t>& logv,
-    eversion_t trim_to,
-    eversion_t roll_forward_to,
-    ObjectStore::Transaction &t,
-    bool transaction_applied,
-    bool async);
+
 public:
   PeeringState(
     CephContext *cct,
@@ -1522,6 +1506,35 @@ public:
     const pg_stat_t &pg_stats_publish,
     const object_stat_collection_t &unstable_stats);
 
+  /**
+   * Merge entries updating missing as necessary on all
+   * acting_recovery_backfill logs and missings (also missing_loc)
+   */
+  void merge_new_log_entries(
+    const mempool::osd_pglog::list<pg_log_entry_t> &entries,
+    ObjectStore::Transaction &t,
+    boost::optional<eversion_t> trim_to,
+    boost::optional<eversion_t> roll_forward_to);
+
+  void append_log(
+    const vector<pg_log_entry_t>& logv,
+    eversion_t trim_to,
+    eversion_t roll_forward_to,
+    ObjectStore::Transaction &t,
+    bool transaction_applied,
+    bool async);
+
+  void recover_got(
+    const hobject_t &oid, eversion_t v,
+    bool is_delete,
+    ObjectStore::Transaction &t);
+
+  void update_backfill_progress(
+    const hobject_t &updated_backfill,
+    const pg_stat_t &updated_stats,
+    bool preserve_local_num_bytes,
+    ObjectStore::Transaction &t);
+
   void dump_history(Formatter *f) const {
     state_history.dump(f);
   }
@@ -1569,6 +1582,8 @@ public:
   /// resets last_persisted_osdmap
   void reset_last_persisted() {
     last_persisted_osdmap = 0;
+    dirty_info = true;
+    dirty_big_info = true;
   }
 
   void shutdown() {
@@ -1579,11 +1594,14 @@ public:
     deleted = true;
   }
 
-  template <typename Func>
-  void adjust_purged_snaps(Func f) {
-    f(info.purged_snaps);
+  void adjust_purged_snaps(
+    std::function<void(interval_set<snapid_t> &snaps)> f);
+
+
+  void force_write_state(ObjectStore::Transaction &t) {
     dirty_info = true;
     dirty_big_info = true;
+    write_if_dirty(t);
   }
 
   bool is_deleting() const {
@@ -1743,6 +1761,10 @@ public:
     return min_last_complete_ondisk;
   }
 
+  bool debug_has_dirty_state() const {
+    return dirty_info || dirty_big_info;
+  }
+
   std::string get_pg_state_string() const {
     return pg_state_string(state);
   }
diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index bc56693556d..6787b5b9f63 100644
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -397,16 +397,11 @@ void PrimaryLogPG::on_local_recover(
   // keep track of active pushes for scrub
   ++active_pushes;
 
-  if (recovery_info.version > pg_log.get_can_rollback_to()) {
-    /* This can only happen during a repair, and even then, it would
-     * be one heck of a race.  If we are repairing the object, the
-     * write in question must be fully committed, so it's not valid
-     * to roll it back anyway (and we'll be rolled forward shortly
-     * anyway) */
-    PGLogEntryHandler h{this, t};
-    pg_log.roll_forward_to(recovery_info.version, &h);
-  }
-  recover_got(recovery_info.soid, recovery_info.version);
+  recovery_state.recover_got(
+    recovery_info.soid,
+    recovery_info.version,
+    is_delete,
+    *t);
 
   if (is_primary()) {
     if (!is_delete) {
@@ -423,9 +418,6 @@ void PrimaryLogPG::on_local_recover(
     t->register_on_applied(new C_OSD_AppliedRecoveredObject(this, obc));
 
     publish_stats_to_osd();
-    ceph_assert(missing_loc.needs_recovery(hoid));
-    if (!is_delete)
-      missing_loc.add_location(hoid, pg_whoami);
     release_backoffs(hoid);
     if (!is_unreadable_object(hoid)) {
       auto unreadable_object_entry = waiting_for_unreadable_object.find(hoid);
@@ -446,10 +438,6 @@ void PrimaryLogPG::on_local_recover(
       this,
       get_osdmap_epoch(),
       info.last_complete));
-
-  // update pg
-  dirty_info = true;
-  write_if_dirty(*t);
 }
 
 void PrimaryLogPG::on_global_recover(
@@ -4261,24 +4249,13 @@ void PrimaryLogPG::do_backfill(OpRequestRef op)
     {
       ceph_assert(cct->_conf->osd_kill_backfill_at != 2);
 
-      info.set_last_backfill(m->last_backfill);
-      // During backfill submit_push_data() tracks num_bytes which is needed in case
-      // backfill stops and starts again.  We want to know how many bytes this
-      // pg is consuming on the disk in order to compute amount of new data
-      // reserved to hold backfill if it won't fit.
-      if (m->op == MOSDPGBackfill::OP_BACKFILL_PROGRESS) {
-        dout(25) << __func__ << " primary " << m->stats.stats.sum.num_bytes << " local " << info.stats.stats.sum.num_bytes << dendl;
-        int64_t bytes = info.stats.stats.sum.num_bytes;
-        info.stats = m->stats;
-        info.stats.stats.sum.num_bytes = bytes;
-      } else {
-        dout(20) << __func__ << " final " << m->stats.stats.sum.num_bytes << " replaces local " << info.stats.stats.sum.num_bytes << dendl;
-        info.stats = m->stats;
-      }
-
       ObjectStore::Transaction t;
-      dirty_info = true;
-      write_if_dirty(t);
+      recovery_state.update_backfill_progress(
+	m->last_backfill,
+	m->stats,
+	m->op == MOSDPGBackfill::OP_BACKFILL_PROGRESS,
+	t);
+
       int tr = osd->store->queue_transaction(ch, std::move(t), NULL);
       ceph_assert(tr == 0);
     }
@@ -11623,23 +11600,6 @@ void PrimaryLogPG::_applied_recovered_object_replica()
   }
 }
 
-void PrimaryLogPG::recover_got(hobject_t oid, eversion_t v)
-{
-  dout(10) << "got missing " << oid << " v " << v << dendl;
-  pg_log.recover_got(oid, v, info);
-  if (pg_log.get_log().complete_to != pg_log.get_log().log.end()) {
-    dout(10) << "last_complete now " << info.last_complete
-	     << " log.complete_to " << pg_log.get_log().complete_to->version
-	     << dendl;
-  } else {
-    dout(10) << "last_complete now " << info.last_complete
-	     << " log.complete_to at end" << dendl;
-    //below is not true in the repair case.
-    //assert(missing.num_missing() == 0);  // otherwise, complete_to was wrong.
-    ceph_assert(info.last_complete == info.last_update);
-  }
-}
-
 void PrimaryLogPG::primary_failed(const hobject_t &soid)
 {
   list<pg_shard_t> fl = { pg_whoami };
@@ -12014,15 +11974,6 @@ void PrimaryLogPG::on_removal(ObjectStore::Transaction *t)
 {
   dout(10) << __func__ << dendl;
 
-  // adjust info to backfill
-  info.set_last_backfill(hobject_t());
-  pg_log.reset_backfill();
-  dirty_info = true;
-
-  // clear log
-  PGLogEntryHandler rollbacker{this, t};
-  pg_log.roll_forward(&rollbacker);
-
   on_shutdown();
 
   t->register_on_commit(new C_DeleteMore(this, get_osdmap_epoch()));
@@ -12606,8 +12557,11 @@ uint64_t PrimaryLogPG::recover_primary(uint64_t max, ThreadPool::TPHandle &handl
 	      ceph_assert(!pool.info.require_rollback());
 	      t.setattr(coll, ghobject_t(soid), OI_ATTR, b2);
 
-	      recover_got(soid, latest->version);
-	      missing_loc.add_location(soid, pg_whoami);
+	      recovery_state.recover_got(
+		soid,
+		latest->version,
+		false,
+		t);
 
 	      ++active_pushes;
 
@@ -15315,15 +15269,19 @@ boost::statechart::result PrimaryLogPG::AwaitAsyncWork::react(const DoSnapWork&)
     ldout(pg->cct, 10) << "adding snap " << snap_to_trim
 		       << " to purged_snaps"
 		       << dendl;
-    pg->info.purged_snaps.insert(snap_to_trim);
+
+    ObjectStore::Transaction t;
+    pg->recovery_state.adjust_purged_snaps(
+      [snap_to_trim](auto &purged_snaps) {
+	purged_snaps.insert(snap_to_trim);
+      });
+    pg->write_if_dirty(t);
+
     pg->snap_trimq.erase(snap_to_trim);
     ldout(pg->cct, 10) << "purged_snaps now "
 		       << pg->info.purged_snaps << ", snap_trimq now "
 		       << pg->snap_trimq << dendl;
 
-    ObjectStore::Transaction t;
-    pg->recovery_state.dirty_big_info = true;
-    pg->write_if_dirty(t);
     int tr = pg->osd->store->queue_transaction(pg->ch, std::move(t), NULL);
     ceph_assert(tr == 0);