From a059ac1e27d74f9f8285613b435e22dfcc80d815 Mon Sep 17 00:00:00 2001
From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Wed, 28 Jul 2021 09:36:20 +0800
Subject: [PATCH] crimson/os/seastore/cache: misc cleanup to metrics

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---
 src/crimson/os/seastore/cache.cc | 52 +++++++++++++++-----------------
 src/crimson/os/seastore/cache.h  | 33 +++++++-------------
 2 files changed, 36 insertions(+), 49 deletions(-)

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 2dcd402690553..bc8840e4b248c 100644
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -117,52 +117,40 @@ void Cache::register_metrics()
    * trans_created
    */
   stats.trans_created_by_src.fill(0);
-  auto register_trans_created = [this, &labels_by_src](src_t src) {
-    std::ostringstream oss_desc;
-    oss_desc << "total number of transaction created (src="
-             << src << ")";
+  for (auto& [src, src_label] : labels_by_src) {
     metrics.add_group(
       "cache",
       {
         sm::make_counter(
           "trans_created",
           get_by_src(stats.trans_created_by_src, src),
-          sm::description(oss_desc.str()),
-          {labels_by_src.find(src)->second}
+          sm::description("total number of transaction created"),
+          {src_label}
         ),
       }
     );
-  };
-  for (auto& [src, label] : labels_by_src) {
-    register_trans_created(src);
   }
 
   /*
    * trans_committed
    */
   stats.trans_committed_by_src.fill(0);
-  auto register_trans_committed = [this, &labels_by_src](src_t src) {
-    std::ostringstream oss_desc;
-    oss_desc << "total number of transaction committed (src="
-             << src << ")";
+  for (auto& [src, src_label] : labels_by_src) {
+    if (src == src_t::READ) {
+      // READ transaction won't commit
+      continue;
+    }
     metrics.add_group(
       "cache",
       {
         sm::make_counter(
           "trans_committed",
           get_by_src(stats.trans_committed_by_src, src),
-          sm::description(oss_desc.str()),
-          {labels_by_src.find(src)->second}
+          sm::description("total number of transaction committed"),
+          {src_label}
         ),
       }
     );
-  };
-  for (auto& [src, label] : labels_by_src) {
-    if (src == src_t::READ) {
-      // READ transaction won't commit
-      continue;
-    }
-    register_trans_committed(src);
   }
 
   /*
@@ -188,17 +176,16 @@ void Cache::register_metrics()
   }
 
   /**
-   * read_transactions_successful
+   * trans_read_successful
    */
   stats.read_transactions_successful = 0;
   metrics.add_group(
     "cache",
     {
       sm::make_counter(
-        "read_trans_successful",
+        "trans_read_successful",
         stats.read_transactions_successful,
-        sm::description("total number of successful read transactions"),
-        {}
+        sm::description("total number of successful read transactions")
       ),
     }
   );
@@ -248,7 +235,18 @@ void Cache::register_metrics()
       auto& efforts = get_by_src(stats.invalidated_efforts_by_src, src);
       efforts = {};
       for (auto& [effort_name, effort_label] : labels_by_effort) {
-        auto& effort = efforts.get_by_name(effort_name);
+        auto& effort = [&effort_name, &efforts]() -> effort_t& {
+          if (effort_name == "READ") {
+            return efforts.read;
+          } else if (effort_name == "MUTATE") {
+            return efforts.mutate;
+          } else if (effort_name == "RETIRE") {
+            return efforts.retire;
+          } else {
+            assert(effort_name == "FRESH");
+            return efforts.fresh;
+          }
+        }();
         metrics.add_group(
           "cache",
           {
diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index f3f3c035e082a..c67b55741bec9 100644
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -620,19 +620,6 @@ private:
     uint64_t mutate_delta_bytes = 0;
     effort_t retire;
     effort_t fresh;
-
-    effort_t& get_by_name(const std::string& effort_name) {
-      if (effort_name == "READ") {
-        return read;
-      } else if (effort_name == "MUTATE") {
-        return mutate;
-      } else if (effort_name == "RETIRE") {
-        return retire;
-      } else {
-        ceph_assert(effort_name == "FRESH");
-        return fresh;
-      }
-    }
   };
 
   template <typename CounterT>
@@ -643,17 +630,19 @@ private:
     counter_by_extent_t<effort_t> mutate_by_ext;
     counter_by_extent_t<uint64_t> delta_bytes_by_ext;
     counter_by_extent_t<effort_t> retire_by_ext;
-    counter_by_extent_t<effort_t > fresh_by_ext;
+    counter_by_extent_t<effort_t> fresh_by_ext;
   };
 
+  template <typename CounterT>
+  using counter_by_src_t = std::array<CounterT, Transaction::SRC_MAX>;
+
   struct {
-    std::array<uint64_t, Transaction::SRC_MAX> trans_created_by_src;
-    std::array<uint64_t, Transaction::SRC_MAX> trans_committed_by_src;
-    std::array<trans_byextent_efforts_t,
-               Transaction::SRC_MAX> committed_efforts_by_src;
-    std::array<counter_by_extent_t<uint64_t>, Transaction::SRC_MAX> trans_invalidated;
-    std::array<trans_efforts_t, Transaction::SRC_MAX> invalidated_efforts_by_src;
-    std::array<query_counters_t, Transaction::SRC_MAX> cache_query_by_src;
+    counter_by_src_t<uint64_t> trans_created_by_src;
+    counter_by_src_t<uint64_t> trans_committed_by_src;
+    counter_by_src_t<trans_byextent_efforts_t>      committed_efforts_by_src;
+    counter_by_src_t<counter_by_extent_t<uint64_t>> trans_invalidated;
+    counter_by_src_t<trans_efforts_t>  invalidated_efforts_by_src;
+    counter_by_src_t<query_counters_t> cache_query_by_src;
     uint64_t read_transactions_successful;
     effort_t read_effort_successful;
     uint64_t dirty_bytes;
@@ -661,7 +650,7 @@ private:
 
   template <typename CounterT>
   CounterT& get_by_src(
-      std::array<CounterT, Transaction::SRC_MAX>& counters_by_src,
+      counter_by_src_t<CounterT>& counters_by_src,
       Transaction::src_t src) {
     assert(static_cast<std::size_t>(src) < counters_by_src.size());
     return counters_by_src[static_cast<std::size_t>(src)];
-- 
2.39.5