crimson/osd: remove explicit pg counter

The main user of num_pgs was get_target_pg_log_entries(), but we
don't really want to be accessing a global counter there if we
can avoid it.  Instead, compute a core-local target and use the
core-local pg count.

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

diff --git a/src/crimson/osd/osd_operations/pg_advance_map.cc b/src/crimson/osd/osd_operations/pg_advance_map.cc
index 0bc7678642b8991e7588ffcf065d9e4ec0445b26..57477105b8f93284d0c26259637c0c45fbb1cddb 100644 (file)
--- a/src/crimson/osd/osd_operations/pg_advance_map.cc
+++ b/src/crimson/osd/osd_operations/pg_advance_map.cc
@@ -82,7 +82,6 @@ seastar::future<> PGAdvanceMap::start()
         logger().debug("{}: map activated", *this);
         if (do_init) {
           shard_services.pg_created(pg->get_pgid(), pg);
-          shard_services.inc_pg_num();
           logger().info("PGAdvanceMap::start new pg {}", *pg);
         }
         return seastar::when_all_succeed(

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 15b6122575200ce2b1f8f3597638f75e49a79936..bd22756fdf59d0b5aefbf1aa1979607921085149 100644 (file)
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -242,12 +242,13 @@ void PG::recheck_readable()
 
 unsigned PG::get_target_pg_log_entries() const
 {
-  const unsigned num_pgs = shard_services.get_pg_num();
-  const unsigned target =
-    local_conf().get_val<uint64_t>("osd_target_pg_log_entries_per_osd");
+  const unsigned local_num_pgs = shard_services.get_num_local_pgs();
+  const unsigned local_target =
+    local_conf().get_val<uint64_t>("osd_target_pg_log_entries_per_osd") /
+    seastar::smp::count;
   const unsigned min_pg_log_entries =
     local_conf().get_val<uint64_t>("osd_min_pg_log_entries");
-  if (num_pgs > 0 && target > 0) {
+  if (local_num_pgs > 0 && local_target > 0) {
     // target an even spread of our budgeted log entries across all
     // PGs.  note that while we only get to control the entry count
     // for primary PGs, we'll normally be responsible for a mix of
@@ -255,7 +256,7 @@ unsigned PG::get_target_pg_log_entries() const
     // will work out.
     const unsigned max_pg_log_entries =
       local_conf().get_val<uint64_t>("osd_max_pg_log_entries");
-    return std::clamp(target / num_pgs,
+    return std::clamp(local_target / local_num_pgs,
                      min_pg_log_entries,
                      max_pg_log_entries);
   } else {
@@ -353,7 +354,6 @@ std::pair<ghobject_t, bool>
 PG::do_delete_work(ceph::os::Transaction &t, ghobject_t _next)
 {
   // TODO
-  shard_services.dec_pg_num();
   return {_next, false};
 }
 

diff --git a/src/crimson/osd/pg_map.h b/src/crimson/osd/pg_map.h
index 0eabf34eb6bc5ebd0843fc23e42e03e5612a4b92..626523521b715d70339dcea9534919b11ea74889 100644 (file)
--- a/src/crimson/osd/pg_map.h
+++ b/src/crimson/osd/pg_map.h
@@ -141,6 +141,7 @@ public:
 
   pgs_t& get_pgs() { return pgs; }
   const pgs_t& get_pgs() const { return pgs; }
+  auto get_pg_count() const { return pgs.size(); }
   PGMap() = default;
   ~PGMap();
 };

diff --git a/src/crimson/osd/pg_shard_manager.cc b/src/crimson/osd/pg_shard_manager.cc
index 8045e0d4a602c2e85be9687b478f27a7e30389e6..fef77d6166a12ae67c92037374dc6c4a5d42ca7f 100644 (file)
--- a/src/crimson/osd/pg_shard_manager.cc
+++ b/src/crimson/osd/pg_shard_manager.cc
@@ -54,10 +54,9 @@ seastar::future<> PGShardManager::load_pgs()
              ShardServices &shard_services) {
              return shard_services.load_pg(
                pgid
-             ).then([pgid, &per_shard_state, &shard_services](auto &&pg) {
+             ).then([pgid, &per_shard_state](auto &&pg) {
                logger().info("load_pgs: loaded {}", pgid);
                per_shard_state.pg_map.pg_loaded(pgid, std::move(pg));
-               shard_services.inc_pg_num();
                return seastar::now();
              });
            });

diff --git a/src/crimson/osd/shard_services.h b/src/crimson/osd/shard_services.h
index 5614815381a72338ba0c8dab28a37e3125e29780..ca0ae397cb0fe4d83f9a7f9957b7df6b5eed0e95 100644 (file)
--- a/src/crimson/osd/shard_services.h
+++ b/src/crimson/osd/shard_services.h
@@ -211,16 +211,6 @@ public:
 
   // TODO: add config to control mapping
   PGShardMapping pg_to_shard_mapping{0, 1};
-  unsigned num_pgs = 0;
-  unsigned get_pg_num() const {
-    return num_pgs;
-  }
-  void inc_pg_num() {
-    ++num_pgs;
-  }
-  void dec_pg_num() {
-    --num_pgs;
-  }
 
   std::set<pg_t> pg_created;
   seastar::future<> send_pg_created(pg_t pgid);
@@ -362,6 +352,11 @@ public:
     return dispatch_context({}, std::move(ctx));
   }
 
+  /// Return core-local pg count * number of cores
+  unsigned get_num_local_pgs() const {
+    return local_state.pg_map.get_pg_count();
+  }
+
   // OSDMapService
   cached_map_t get_map() const final { return local_state.get_osdmap(); }
   epoch_t get_up_epoch() const final { return local_state.up_epoch; }
@@ -379,7 +374,6 @@ public:
   }
 
   FORWARD_TO_OSD_SINGLETON(get_pool_info)
-  FORWARD_TO_OSD_SINGLETON(get_pg_num)
   FORWARD(with_throttle_while, with_throttle_while, local_state.throttler)
 
   FORWARD_TO_OSD_SINGLETON(osdmap_subscribe)
@@ -388,8 +382,6 @@ public:
   FORWARD_TO_OSD_SINGLETON(remove_want_pg_temp)
   FORWARD_TO_OSD_SINGLETON(requeue_pg_temp)
   FORWARD_TO_OSD_SINGLETON(send_pg_created)
-  FORWARD_TO_OSD_SINGLETON(inc_pg_num)
-  FORWARD_TO_OSD_SINGLETON(dec_pg_num)
   FORWARD_TO_OSD_SINGLETON(send_alive)
   FORWARD_TO_OSD_SINGLETON(send_pg_temp)
   FORWARD_CONST(get_mnow, get_mnow, osd_singleton_state)