From ee11626dc7cb1b8719ba3ca41d3ca0ce6d3af1a0 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Thu, 3 Apr 2025 03:42:11 +0000 Subject: [PATCH] crimson: remove CommonClientRequest, move do_recover_missing to PG do_recover_missing was the only thing left, and inheriting from a class to get a static method is somewhat confusing. Simply move do_recover_missing to PG. Signed-off-by: Samuel Just --- src/crimson/osd/CMakeLists.txt | 1 - .../osd/osd_operations/client_request.cc | 6 +- .../osd/osd_operations/client_request.h | 4 +- .../osd_operations/client_request_common.cc | 80 ------------------- .../osd_operations/client_request_common.h | 21 ----- .../osd_operations/internal_client_request.cc | 4 +- .../osd_operations/internal_client_request.h | 4 +- src/crimson/osd/pg.cc | 63 +++++++++++++++ src/crimson/osd/pg.h | 4 + 9 files changed, 74 insertions(+), 113 deletions(-) delete mode 100644 src/crimson/osd/osd_operations/client_request_common.cc delete mode 100644 src/crimson/osd/osd_operations/client_request_common.h diff --git a/src/crimson/osd/CMakeLists.txt b/src/crimson/osd/CMakeLists.txt index 5796500080eaa..2832c9d864b6b 100644 --- a/src/crimson/osd/CMakeLists.txt +++ b/src/crimson/osd/CMakeLists.txt @@ -19,7 +19,6 @@ add_executable(crimson-osd ops_executer.cc osd_operation.cc osd_operations/client_request.cc - osd_operations/client_request_common.cc osd_operations/internal_client_request.cc osd_operations/peering_event.cc osd_operations/pg_advance_map.cc diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc index e97525f9ad13b..b29cf29c5ed42 100644 --- a/src/crimson/osd/osd_operations/client_request.cc +++ b/src/crimson/osd/osd_operations/client_request.cc @@ -321,7 +321,7 @@ ClientRequest::recover_missing_snaps( co_await std::move(resolve_oids); for (auto &oid : ret) { - auto unfound = co_await do_recover_missing(pg, oid, m->get_reqid()); + auto unfound = co_await pg->do_recover_missing(oid, m->get_reqid()); if (unfound) { DEBUGDPP("{} unfound, hang it for now", *pg, oid); co_await interruptor::make_interruptible( @@ -347,8 +347,8 @@ ClientRequest::process_op( "Skipping recover_missings on non primary pg for soid {}", *pg, m->get_hobj()); } else { - auto unfound = co_await do_recover_missing( - pg, m->get_hobj().get_head(), m->get_reqid()); + auto unfound = co_await pg->do_recover_missing( + m->get_hobj().get_head(), m->get_reqid()); if (unfound) { DEBUGDPP("{} unfound, hang it for now", *pg, m->get_hobj().get_head()); co_await interruptor::make_interruptible( diff --git a/src/crimson/osd/osd_operations/client_request.h b/src/crimson/osd/osd_operations/client_request.h index c1ea50acc3536..bdc3ff0d24159 100644 --- a/src/crimson/osd/osd_operations/client_request.h +++ b/src/crimson/osd/osd_operations/client_request.h @@ -15,7 +15,6 @@ #include "crimson/osd/object_context_loader.h" #include "crimson/osd/osdmap_gate.h" #include "crimson/osd/osd_operation.h" -#include "crimson/osd/osd_operations/client_request_common.h" #include "crimson/osd/pg_activation_blocker.h" #include "crimson/osd/pg_map.h" #include "crimson/osd/scrub/pg_scrubber.h" @@ -28,8 +27,7 @@ class PG; class OSD; class ShardServices; -class ClientRequest final : public PhasedOperationT, - private CommonClientRequest { +class ClientRequest final : public PhasedOperationT { // Initially set to primary core, updated to pg core after with_pg() ShardServices *shard_services = nullptr; diff --git a/src/crimson/osd/osd_operations/client_request_common.cc b/src/crimson/osd/osd_operations/client_request_common.cc deleted file mode 100644 index 06b9d4f757f11..0000000000000 --- a/src/crimson/osd/osd_operations/client_request_common.cc +++ /dev/null @@ -1,80 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:nil -*- -// vim: ts=8 sw=2 smarttab expandtab - -#include "crimson/common/coroutine.h" -#include "crimson/common/log.h" -#include "crimson/osd/osd_operations/client_request_common.h" -#include "crimson/osd/pg.h" -#include "crimson/osd/osd_operations/background_recovery.h" - -SET_SUBSYS(osd); - -namespace crimson::osd { - -typename InterruptibleOperation::template interruptible_future -CommonClientRequest::do_recover_missing( - Ref pg, - const hobject_t& soid, - const osd_reqid_t& reqid) -{ - LOG_PREFIX(CommonCLientRequest::do_recover_missing); - DEBUGDPP( - "reqid {} check for recovery, {}", - *pg, reqid, soid); - assert(pg->is_primary()); - eversion_t ver; - auto &peering_state = pg->get_peering_state(); - auto &missing_loc = peering_state.get_missing_loc(); - bool needs_recovery_or_backfill = false; - - if (pg->is_unreadable_object(soid)) { - DEBUGDPP( - "reqid {}, {} is unreadable", - *pg, reqid, soid); - ceph_assert(missing_loc.needs_recovery(soid, &ver)); - needs_recovery_or_backfill = true; - } - - if (pg->is_degraded_or_backfilling_object(soid)) { - DEBUGDPP( - "reqid {}, {} is degraded or backfilling", - *pg, reqid, soid); - if (missing_loc.needs_recovery(soid, &ver)) { - needs_recovery_or_backfill = true; - } - } - - if (!needs_recovery_or_backfill) { - DEBUGDPP( - "reqid {} nothing to recover {}", - *pg, reqid, soid); - co_return false; - } - - if (pg->get_peering_state().get_missing_loc().is_unfound(soid)) { - co_return true; - } - DEBUGDPP( - "reqid {} need to wait for recovery, {} version {}", - *pg, reqid, soid); - if (pg->get_recovery_backend()->is_recovering(soid)) { - DEBUGDPP( - "reqid {} object {} version {}, already recovering", - *pg, reqid, soid, ver); - co_await PG::interruptor::make_interruptible( - pg->get_recovery_backend()->get_recovering( - soid).wait_for_recovered()); - co_return false; - } else { - DEBUGDPP( - "reqid {} object {} version {}, starting recovery", - *pg, reqid, soid, ver); - auto [op, fut] = - pg->get_shard_services().start_operation( - soid, ver, pg, pg->get_shard_services(), pg->get_osdmap_epoch()); - co_await PG::interruptor::make_interruptible(std::move(fut)); - co_return false; - } -} - -} // namespace crimson::osd diff --git a/src/crimson/osd/osd_operations/client_request_common.h b/src/crimson/osd/osd_operations/client_request_common.h deleted file mode 100644 index 4c3cf42777bdb..0000000000000 --- a/src/crimson/osd/osd_operations/client_request_common.h +++ /dev/null @@ -1,21 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- -// vim: ts=8 sw=2 smarttab - -#pragma once - -#include "crimson/common/operation.h" -#include "crimson/common/type_helpers.h" -#include "crimson/osd/osd_operation.h" - -namespace crimson::osd { - -struct CommonClientRequest { - - static InterruptibleOperation::template interruptible_future - do_recover_missing( - Ref pg, - const hobject_t& soid, - const osd_reqid_t& reqid); -}; - -} // namespace crimson::osd diff --git a/src/crimson/osd/osd_operations/internal_client_request.cc b/src/crimson/osd/osd_operations/internal_client_request.cc index b8f7646bc7433..e486b62ad7dc4 100644 --- a/src/crimson/osd/osd_operations/internal_client_request.cc +++ b/src/crimson/osd/osd_operations/internal_client_request.cc @@ -64,8 +64,8 @@ InternalClientRequest::with_interruption() co_await enter_stage(obc_orderer->obc_pp().process); - bool unfound = co_await do_recover_missing( - pg, get_target_oid(), osd_reqid_t()); + bool unfound = co_await pg->do_recover_missing( + get_target_oid(), osd_reqid_t()); if (unfound) { throw std::system_error( diff --git a/src/crimson/osd/osd_operations/internal_client_request.h b/src/crimson/osd/osd_operations/internal_client_request.h index 1cfde4ab080ec..c4c98bfcec1cf 100644 --- a/src/crimson/osd/osd_operations/internal_client_request.h +++ b/src/crimson/osd/osd_operations/internal_client_request.h @@ -6,14 +6,12 @@ #include "crimson/common/type_helpers.h" #include "crimson/osd/object_context_loader.h" #include "crimson/osd/osd_operation.h" -#include "crimson/osd/osd_operations/client_request_common.h" #include "crimson/osd/pg.h" #include "crimson/osd/pg_activation_blocker.h" namespace crimson::osd { -class InternalClientRequest : public PhasedOperationT, - private CommonClientRequest { +class InternalClientRequest : public PhasedOperationT { public: explicit InternalClientRequest(Ref pg); ~InternalClientRequest(); diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc index cb0689d2e31eb..155ca2d3565aa 100644 --- a/src/crimson/osd/pg.cc +++ b/src/crimson/osd/pg.cc @@ -660,6 +660,69 @@ void PG::on_active_advmap(const OSDMapRef &osdmap) } } +PG::interruptible_future PG::do_recover_missing( + const hobject_t& soid, + const osd_reqid_t& reqid) +{ + LOG_PREFIX(PG::do_recover_missing); + DEBUGDPP( + "reqid {} check for recovery, {}", + *this, reqid, soid); + assert(is_primary()); + eversion_t ver; + auto &missing_loc = peering_state.get_missing_loc(); + bool needs_recovery_or_backfill = false; + + if (is_unreadable_object(soid)) { + DEBUGDPP( + "reqid {}, {} is unreadable", + *this, reqid, soid); + ceph_assert(missing_loc.needs_recovery(soid, &ver)); + needs_recovery_or_backfill = true; + } + + if (is_degraded_or_backfilling_object(soid)) { + DEBUGDPP( + "reqid {}, {} is degraded or backfilling", + *this, reqid, soid); + if (missing_loc.needs_recovery(soid, &ver)) { + needs_recovery_or_backfill = true; + } + } + + if (!needs_recovery_or_backfill) { + DEBUGDPP( + "reqid {} nothing to recover {}", + *this, reqid, soid); + co_return false; + } + + if (peering_state.get_missing_loc().is_unfound(soid)) { + co_return true; + } + DEBUGDPP( + "reqid {} need to wait for recovery, {} version {}", + *this, reqid, soid); + if (recovery_backend->is_recovering(soid)) { + DEBUGDPP( + "reqid {} object {} version {}, already recovering", + *this, reqid, soid, ver); + co_await PG::interruptor::make_interruptible( + recovery_backend->get_recovering( + soid).wait_for_recovered()); + co_return false; + } else { + DEBUGDPP( + "reqid {} object {} version {}, starting recovery", + *this, reqid, soid, ver); + auto [op, fut] = + shard_services.start_operation( + soid, ver, this, shard_services, get_osdmap_epoch()); + co_await PG::interruptor::make_interruptible(std::move(fut)); + co_return false; + } +} + void PG::scrub_requested(scrub_level_t scrub_level, scrub_type_t scrub_type) { /* We don't actually route the scrub request message into the state machine. diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h index a8cc6eefa792b..42a283098dcdd 100644 --- a/src/crimson/osd/pg.h +++ b/src/crimson/osd/pg.h @@ -756,6 +756,10 @@ private: public: PeeringState peering_state; + interruptible_future do_recover_missing( + const hobject_t& soid, + const osd_reqid_t& reqid); + // scrub state friend class ScrubScan; -- 2.39.5