From 5692ce7c4a4bbb60802385f151d344e402b4f618 Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Thu, 28 Jan 2021 13:56:30 +0800
Subject: [PATCH] osd,crimson: pass optional<pg_stat_t> to
 prepare_stats_for_publish()

to simplify the interface. also save the overhead of passing an empty
pg_stat_t if the existing one is invalidated before calling
prepare_stats_for_publish().

Signed-off-by: Kefu Chai <kchai@redhat.com>
---
 src/crimson/osd/pg.cc   |  4 +---
 src/osd/PG.cc           | 11 ++++-------
 src/osd/PG.h            |  3 +--
 src/osd/PeeringState.cc |  7 +++----
 src/osd/PeeringState.h  |  3 +--
 5 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 17452c511bb63..a53fabd7cd5c7 100644
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -145,10 +145,8 @@ void PG::publish_stats_to_osd()
 {
   if (!is_primary())
     return;
-  const bool is_stats_valid = pg_stats.has_value();
   if (auto new_pg_stats = peering_state.prepare_stats_for_publish(
-        is_stats_valid,
-        pg_stats.value_or(pg_stat_t{}),
+        pg_stats,
         object_stat_collection_t());
       new_pg_stats.has_value()) {
     pg_stats = std::move(new_pg_stats);
diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 5a01fac7fb0d1..fef4b65dfcda3 100644
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -207,7 +207,6 @@ PG::PG(OSDService *o, OSDMapRef curmap,
   recovery_queued(false),
   recovery_ops_active(0),
   backfill_reserving(false),
-  pg_stats_publish_valid(false),
   finish_sync_event(NULL),
   scrub_after_recovery(false),
   active_pushes(0),
@@ -823,12 +822,10 @@ void PG::publish_stats_to_osd()
 
   std::lock_guard l{pg_stats_publish_lock};
   auto stats = recovery_state.prepare_stats_for_publish(
-    pg_stats_publish_valid,
     pg_stats_publish,
     unstable_stats);
   if (stats) {
-    pg_stats_publish = stats.value();
-    pg_stats_publish_valid = true;
+    pg_stats_publish = std::move(stats);
   }
 }
 
@@ -841,7 +838,7 @@ void PG::clear_publish_stats()
 {
   dout(15) << "clear_stats" << dendl;
   std::lock_guard l{pg_stats_publish_lock};
-  pg_stats_publish_valid = false;
+  pg_stats_publish.reset();
 }
 
 /**
@@ -2797,8 +2794,8 @@ void PG::dump_missing(Formatter *f)
 void PG::get_pg_stats(std::function<void(const pg_stat_t&, epoch_t lec)> f)
 {
   std::lock_guard l{pg_stats_publish_lock};
-  if (pg_stats_publish_valid) {
-    f(pg_stats_publish, pg_stats_publish.get_effective_last_epoch_clean());
+  if (pg_stats_publish) {
+    f(*pg_stats_publish, pg_stats_publish->get_effective_last_epoch_clean());
   }
 }
 
diff --git a/src/osd/PG.h b/src/osd/PG.h
index cad031c10861f..67a1c80459df6 100644
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -947,8 +947,7 @@ protected:
   // publish stats
   ceph::mutex pg_stats_publish_lock =
     ceph::make_mutex("PG::pg_stats_publish_lock");
-  bool pg_stats_publish_valid;
-  pg_stat_t pg_stats_publish;
+  std::optional<pg_stat_t> pg_stats_publish;
 
   friend class TestOpsSocketHook;
   void publish_stats_to_osd() override;
diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc
index 0a74aca8ac469..8a9439e1375db 100644
--- a/src/osd/PeeringState.cc
+++ b/src/osd/PeeringState.cc
@@ -3849,8 +3849,7 @@ out:
 }
 
 std::optional<pg_stat_t> PeeringState::prepare_stats_for_publish(
-  bool pg_stats_publish_valid,
-  const pg_stat_t &pg_stats_publish,
+  const std::optional<pg_stat_t> &pg_stats_publish,
   const object_stat_collection_t &unstable_stats)
 {
   if (info.stats.stats.sum.num_scrub_errors) {
@@ -3904,9 +3903,9 @@ std::optional<pg_stat_t> PeeringState::prepare_stats_for_publish(
   psdout(20) << __func__ << " reporting purged_snaps "
 	     << pre_publish.purged_snaps << dendl;
 
-  if (pg_stats_publish_valid && pre_publish == pg_stats_publish &&
+  if (pg_stats_publish && pre_publish == *pg_stats_publish &&
       info.stats.last_fresh > cutoff) {
-    psdout(15) << "publish_stats_to_osd " << pg_stats_publish.reported_epoch
+    psdout(15) << "publish_stats_to_osd " << pg_stats_publish->reported_epoch
 	       << ": no change since " << info.stats.last_fresh << dendl;
     return std::nullopt;
   } else {
diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h
index 1ae5073d1f372..a74b65eafa6ac 100644
--- a/src/osd/PeeringState.h
+++ b/src/osd/PeeringState.h
@@ -1827,8 +1827,7 @@ public:
    * pg_stats_publish adding in unstable_stats.
    */
   std::optional<pg_stat_t> prepare_stats_for_publish(
-    bool pg_stats_publish_valid,
-    const pg_stat_t &pg_stats_publish,
+    const std::optional<pg_stat_t> &pg_stats_publish,
     const object_stat_collection_t &unstable_stats);
 
   /**
-- 
2.39.5