crimson/osd: make the ObjectContextRegistry per-PG

This patch moves the OBC registry from ShardServices (which
is basicaly a gateway to a bunch of PGs) into PG itself.
Dividing OBCs by PG (they truly belong to) minimizes the space
to search when e.g. checking whether a client is blocked or
not. Therefore, this commit is enabler of more performant
PR #47637.

The changeset draws the assumption that OBC registry and all
its users live on the same CPU core. It looks valid to me.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc
index 3cd12aead07e3b95d7786858da75ce7eb65ba118..23aa00a335312193d0a11490d3de5b98f4d4675b 100644 (file)
--- a/src/crimson/osd/object_context_loader.cc
+++ b/src/crimson/osd/object_context_loader.cc
@@ -1,4 +1,5 @@
 #include "crimson/osd/object_context_loader.h"
+#include "osd/osd_types_fmt.h"
 
 SET_SUBSYS(osd);
 
@@ -66,7 +67,7 @@ using crimson::common::local_conf;
         crimson::ct_error::enoent::make()
       };
     }
-    auto [clone, existed] = shard_services.get_cached_obc(*coid);
+    auto [clone, existed] = obc_registry.get_cached_obc(*coid);
     return clone->template with_lock<State, IOInterruptCondition>(
       [existed=existed, clone=std::move(clone),
        func=std::move(func), head=std::move(head), this]()
@@ -87,7 +88,7 @@ using crimson::common::local_conf;
   {
     if (oid.is_head()) {
       auto [obc, existed] =
-        shard_services.get_cached_obc(std::move(oid));
+        obc_registry.get_cached_obc(std::move(oid));
       return with_head_obc<State>(std::move(obc),
                                   existed,
                                   std::move(func));

diff --git a/src/crimson/osd/object_context_loader.h b/src/crimson/osd/object_context_loader.h
index d5f54779f44602ef4b503a59467ff93f1b2afed3..329fb79237ba7fcde5fc45fd297b7f954fbb80f9 100644 (file)
--- a/src/crimson/osd/object_context_loader.h
+++ b/src/crimson/osd/object_context_loader.h
@@ -3,7 +3,6 @@
 #include <seastar/core/future.hh>
 #include "crimson/common/errorator.h"
 #include "crimson/osd/object_context.h"
-#include "crimson/osd/shard_services.h"
 #include "crimson/osd/pg_backend.h"
 
 namespace crimson::osd {
@@ -14,10 +13,10 @@ public:
     ObjectContext::obc_accessing_option_t>;
 
   ObjectContextLoader(
-    ShardServices& _shard_services,
+    ObjectContextRegistry& _obc_services,
     PGBackend& _backend,
     DoutPrefixProvider& dpp)
-    : shard_services{_shard_services},
+    : obc_registry{_obc_services},
       backend{_backend},
       dpp{dpp}
     {}
@@ -67,7 +66,7 @@ public:
   void notify_on_change(bool is_primary);
 
 private:
-  ShardServices &shard_services;
+  ObjectContextRegistry& obc_registry;
   PGBackend& backend;
   DoutPrefixProvider& dpp;
   obc_accessing_list_t obc_set_accessing;

diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc
index fcb56aeeddc2da08e85959af96345f5e90d045c7..dbccb8dcdf49e22963828f7670418bd5f898934f 100644 (file)
--- a/src/crimson/osd/ops_executer.cc
+++ b/src/crimson/osd/ops_executer.cc
@@ -956,8 +956,7 @@ std::pair<object_info_t, ObjectContextRef> OpsExecuter::prepare_clone(
   if (pg->is_primary()) {
     // lookup_or_create
     auto [c_obc, existed] =
-      pg->get_shard_services().get_cached_obc(
-        std::move(coid));
+      pg->obc_registry.get_cached_obc(std::move(coid));
     assert(!existed);
     c_obc->obs.oi = static_snap_oi;
     c_obc->obs.exists = true;

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index d4d76d1787ecaa4ad7989a34581b6e3ee7f8ee19..6e5075b0559dc4925b25fd2071940deb105d8221 100644 (file)
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -134,8 +134,10 @@ PG::PG(
       osdmap,
       this,
       this),
+    obc_registry{
+      local_conf()},
     obc_loader{
-      shard_services,
+      obc_registry,
       *backend.get(),
       *this},
     wait_for_active_blocker(this)

diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
index a1b0a70ccd1abbd510b223748f878ce00fd94f5c..689a0df97b012806b5d93462574c2314027ce202 100644 (file)
--- a/src/crimson/osd/pg.h
+++ b/src/crimson/osd/pg.h
@@ -618,6 +618,7 @@ private:
   eversion_t projected_last_update;
 
 public:
+  ObjectContextRegistry obc_registry;
   ObjectContextLoader obc_loader;
 
   // PeeringListener

diff --git a/src/crimson/osd/recovery_backend.cc b/src/crimson/osd/recovery_backend.cc
index ca595a6ffaf5b18ba6e66bf87c2482e0c77a8d80..f76db8ac0c40e3300d8b0cec9aa98c8610ffe7a2 100644 (file)
--- a/src/crimson/osd/recovery_backend.cc
+++ b/src/crimson/osd/recovery_backend.cc
@@ -183,7 +183,7 @@ RecoveryBackend::scan_for_backfill(
        -> interruptible_future<> {
        crimson::osd::ObjectContextRef obc;
        if (pg.is_primary()) {
-         obc = shard_services.maybe_get_cached_obc(object);
+         obc = pg.obc_registry.maybe_get_cached_obc(object);
        }
        if (obc) {
          if (obc->obs.exists) {

diff --git a/src/crimson/osd/shard_services.cc b/src/crimson/osd/shard_services.cc
index a1d2580b358d479f9071bd43b3d69cdd18801f81..1ea51937196635a8b79701745ba28fb591f17dae 100644 (file)
--- a/src/crimson/osd/shard_services.cc
+++ b/src/crimson/osd/shard_services.cc
@@ -43,7 +43,6 @@ PerShardState::PerShardState(
     store(store),
     perf(perf), recoverystate_perf(recoverystate_perf),
     throttler(crimson::common::local_conf()),
-    obc_registry(crimson::common::local_conf()),
     next_tid(
       static_cast<ceph_tid_t>(seastar::this_shard_id()) <<
       (std::numeric_limits<ceph_tid_t>::digits - 8)),

diff --git a/src/crimson/osd/shard_services.h b/src/crimson/osd/shard_services.h
index 19eccc68f4545826da5fc3b55fa2bca129c8690e..ece06b3ada5a3499913ec8ed9823024c7021704d 100644 (file)
--- a/src/crimson/osd/shard_services.h
+++ b/src/crimson/osd/shard_services.h
@@ -92,8 +92,6 @@ class PerShardState {
     up_epoch = epoch;
   }
 
-  crimson::osd::ObjectContextRegistry obc_registry;
-
   // prevent creating new osd operations when system is shutting down,
   // this is necessary because there are chances that a new operation
   // is created, after the interruption of all ongoing operations, and
@@ -457,11 +455,6 @@ public:
 
   FORWARD(pg_created, pg_created, local_state.pg_map)
 
-  FORWARD(
-    maybe_get_cached_obc, maybe_get_cached_obc, local_state.obc_registry)
-  FORWARD(
-    get_cached_obc, get_cached_obc, local_state.obc_registry)
-
   FORWARD_TO_OSD_SINGLETON_TARGET(
     local_update_priority,
     local_reserver.update_priority)