From 59c8ee4381aedc25da820e8d7396df73fb3b9ead Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Thu, 12 Jan 2023 17:16:15 +0000 Subject: [PATCH] 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 --- src/crimson/osd/object_context_loader.cc | 5 +++-- src/crimson/osd/object_context_loader.h | 7 +++---- src/crimson/osd/ops_executer.cc | 3 +-- src/crimson/osd/pg.cc | 4 +++- src/crimson/osd/pg.h | 1 + src/crimson/osd/recovery_backend.cc | 2 +- src/crimson/osd/shard_services.cc | 1 - src/crimson/osd/shard_services.h | 7 ------- 8 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc index 3cd12aead07e3..23aa00a335312 100644 --- 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( [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(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 d5f54779f4460..329fb79237ba7 100644 --- a/src/crimson/osd/object_context_loader.h +++ b/src/crimson/osd/object_context_loader.h @@ -3,7 +3,6 @@ #include #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 fcb56aeeddc2d..dbccb8dcdf49e 100644 --- a/src/crimson/osd/ops_executer.cc +++ b/src/crimson/osd/ops_executer.cc @@ -956,8 +956,7 @@ std::pair 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 d4d76d1787eca..6e5075b0559dc 100644 --- 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 a1b0a70ccd1ab..689a0df97b012 100644 --- 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 ca595a6ffaf5b..f76db8ac0c40e 100644 --- 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 a1d2580b358d4..1ea5193719663 100644 --- 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(seastar::this_shard_id()) << (std::numeric_limits::digits - 8)), diff --git a/src/crimson/osd/shard_services.h b/src/crimson/osd/shard_services.h index 19eccc68f4545..ece06b3ada5a3 100644 --- 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) -- 2.39.5