From 38cc750f1fbb06146573e639778835fdb56f0b33 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Wed, 6 Sep 2023 11:15:31 +0800 Subject: [PATCH] crimson/osd/osd_operations/client_request: recover the head and other necessary objects before proceeding Signed-off-by: Xuehan Xu --- src/crimson/osd/object_context_loader.cc | 16 +++---- src/crimson/osd/object_context_loader.h | 5 +- .../osd/osd_operations/client_request.cc | 17 ++----- .../osd/osd_operations/client_request.h | 10 ++++ .../osd_operations/client_request_common.cc | 46 +++++++++++++++++++ .../osd_operations/client_request_common.h | 7 +++ .../osd/osd_operations/common/pg_pipeline.h | 2 +- .../osd_operations/internal_client_request.cc | 2 +- src/crimson/osd/pg.cc | 4 +- src/crimson/osd/pg.h | 2 +- src/crimson/osd/pg_backend.cc | 2 +- .../osd/replicated_recovery_backend.cc | 4 +- 12 files changed, 84 insertions(+), 33 deletions(-) diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc index 0a4d74c0d70c7..d7d2b6d98c67a 100644 --- a/src/crimson/osd/object_context_loader.cc +++ b/src/crimson/osd/object_context_loader.cc @@ -22,7 +22,7 @@ using crimson::common::local_conf; return get_or_load_obc(obc, existed) .safe_then_interruptible( [func = std::move(func)](auto obc) { - return std::move(func)(std::move(obc)); + return std::move(func)(obc, obc); }); }).finally([FNAME, this, obc=std::move(obc)] { DEBUGDPP("released object {}", dpp, obc->get_oid()); @@ -39,7 +39,7 @@ using crimson::common::local_conf; assert(!oid.is_head()); return with_obc( oid.get_head(), - [FNAME, oid, func=std::move(func), this](auto head) mutable + [FNAME, oid, func=std::move(func), this](auto head, auto) mutable -> load_obc_iertr::future<> { if (!head->obs.exists) { ERRORDPP("head doesn't exist for object {}", dpp, head->obs.oi.soid); @@ -70,12 +70,12 @@ using crimson::common::local_conf; 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]() + func=std::move(func), head=std::move(head), this]() mutable -> load_obc_iertr::future<> { auto loaded = get_or_load_obc(clone, existed); return loaded.safe_then_interruptible( - [func = std::move(func)](auto clone) { - return std::move(func)(std::move(clone)); + [func = std::move(func), head=std::move(head)](auto clone) mutable { + return std::move(func)(std::move(head), std::move(clone)); }); }); } @@ -84,13 +84,13 @@ using crimson::common::local_conf; ObjectContextLoader::load_obc_iertr::future<> ObjectContextLoader::with_clone_obc_direct( hobject_t oid, - with_both_obc_func_t&& func) + with_obc_func_t&& func) { LOG_PREFIX(ObjectContextLoader::with_clone_obc_direct); assert(!oid.is_head()); return with_obc( oid.get_head(), - [FNAME, oid, func=std::move(func), this](auto head) mutable + [FNAME, oid, func=std::move(func), this](auto head, auto) mutable -> load_obc_iertr::future<> { if (!head->obs.exists) { ERRORDPP("head doesn't exist for object {}", dpp, head->obs.oi.soid); @@ -228,5 +228,5 @@ using crimson::common::local_conf; template ObjectContextLoader::load_obc_iertr::future<> ObjectContextLoader::with_clone_obc_direct( hobject_t, - with_both_obc_func_t&&); + with_obc_func_t&&); } diff --git a/src/crimson/osd/object_context_loader.h b/src/crimson/osd/object_context_loader.h index 3ab7f6ad80fdc..0cd50623abc25 100644 --- a/src/crimson/osd/object_context_loader.h +++ b/src/crimson/osd/object_context_loader.h @@ -30,9 +30,6 @@ public: load_obc_ertr>; using with_obc_func_t = - std::function (ObjectContextRef)>; - - using with_both_obc_func_t = std::function (ObjectContextRef, ObjectContextRef)>; // Use this variant by default @@ -55,7 +52,7 @@ public: template load_obc_iertr::future<> with_clone_obc_direct( hobject_t oid, - with_both_obc_func_t&& func); + with_obc_func_t&& func); load_obc_iertr::future<> reload_obc(ObjectContext& obc) const; diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc index d208e2e53d97e..ac441185cdacc 100644 --- a/src/crimson/osd/osd_operations/client_request.cc +++ b/src/crimson/osd/osd_operations/client_request.cc @@ -225,18 +225,9 @@ ClientRequest::interruptible_future<> ClientRequest::process_op(instance_handle_t &ihref, Ref &pg) { return ihref.enter_stage( - client_pp(*pg).recover_missing, - *this - ).then_interruptible( - [this, pg]() mutable { - LOG_PREFIX(ClientRequest::process_op); - if (pg->is_primary()) { - return do_recover_missing(pg, m->get_hobj()); - } else { - DEBUGI("process_op: Skipping do_recover_missing" - "on non primary pg"); - return interruptor::now(); - } + client_pp(*pg).recover_missing, *this + ).then_interruptible([pg, this]() mutable { + return recover_missings(pg, m->get_hobj(), snaps_need_to_recover()); }).then_interruptible([this, pg, &ihref]() mutable { return pg->already_complete(m->get_reqid()).then_interruptible( [this, pg, &ihref](auto completed) mutable @@ -256,7 +247,7 @@ ClientRequest::process_op(instance_handle_t &ihref, Ref &pg) op_info.set_from_op(&*m, *pg->get_osdmap()); return pg->with_locked_obc( m->get_hobj(), op_info, - [this, pg, &ihref](auto obc) mutable { + [this, pg, &ihref](auto head, auto obc) mutable { LOG_PREFIX(ClientRequest::process_op); DEBUGI("{}: got obc {}", *this, obc->obs); return ihref.enter_stage( diff --git a/src/crimson/osd/osd_operations/client_request.h b/src/crimson/osd/osd_operations/client_request.h index 3c8c146a4f2f4..43535e156c5b9 100644 --- a/src/crimson/osd/osd_operations/client_request.h +++ b/src/crimson/osd/osd_operations/client_request.h @@ -160,6 +160,16 @@ public: } auto get_instance_handle() { return instance_handle; } + std::vector snaps_need_to_recover() { + std::vector ret; + for (auto &op : m->ops) { + if (op.op.op == CEPH_OSD_OP_ROLLBACK) { + ret.emplace_back((snapid_t)op.op.snap.snapid); + } + } + return ret; + } + using ordering_hook_t = boost::intrusive::list_member_hook<>; ordering_hook_t ordering_hook; class Orderer { diff --git a/src/crimson/osd/osd_operations/client_request_common.cc b/src/crimson/osd/osd_operations/client_request_common.cc index cfd22c774e06e..903da59a3dcec 100644 --- a/src/crimson/osd/osd_operations/client_request_common.cc +++ b/src/crimson/osd/osd_operations/client_request_common.cc @@ -11,8 +11,54 @@ namespace { } } +SET_SUBSYS(osd); + namespace crimson::osd { +InterruptibleOperation::template interruptible_future<> +CommonClientRequest::recover_missings( + Ref &pg, + const hobject_t& soid, + std::vector &&snaps) +{ + using interruptor = InterruptibleOperation::interruptor; + LOG_PREFIX(CommonClientRequest::recover_missings); + auto fut = interruptor::now(); + if (!pg->is_primary()) { + DEBUGI("process_op: Skipping do_recover_missing on non primary pg"); + return fut; + } + if (!soid.is_head()) { + fut = do_recover_missing(pg, soid.get_head()); + } + return seastar::do_with( + std::move(snaps), + [pg, soid, fut=std::move(fut)](auto &snaps) mutable { + return fut.then_interruptible([&snaps, pg, soid]() mutable { + return pg->obc_loader.with_obc( + soid.get_head(), + [&snaps, pg, soid](auto head, auto) mutable { + auto oid = resolve_oid(head->get_head_ss(), soid); + assert(oid); + return do_recover_missing(pg, *oid + ).then_interruptible([&snaps, pg, soid, head]() mutable { + return InterruptibleOperation::interruptor::do_for_each( + snaps, + [pg, soid, head](auto &snap) mutable { + auto coid = head->obs.oi.soid; + coid.snap = snap; + auto oid = resolve_oid(head->get_head_ss(), coid); + assert(oid); + return do_recover_missing(pg, *oid); + }); + }); + }); + }).handle_error_interruptible( + crimson::ct_error::assert_all("unexpected error") + ); + }); +} + typename InterruptibleOperation::template interruptible_future<> CommonClientRequest::do_recover_missing( Ref& pg, const hobject_t& soid) diff --git a/src/crimson/osd/osd_operations/client_request_common.h b/src/crimson/osd/osd_operations/client_request_common.h index 6a8a789668c18..46aa038e3432b 100644 --- a/src/crimson/osd/osd_operations/client_request_common.h +++ b/src/crimson/osd/osd_operations/client_request_common.h @@ -10,6 +10,13 @@ namespace crimson::osd { struct CommonClientRequest { + + static InterruptibleOperation::template interruptible_future<> + recover_missings( + Ref &pg, + const hobject_t& soid, + std::vector &&snaps); + static InterruptibleOperation::template interruptible_future<> do_recover_missing(Ref& pg, const hobject_t& soid); diff --git a/src/crimson/osd/osd_operations/common/pg_pipeline.h b/src/crimson/osd/osd_operations/common/pg_pipeline.h index 58fa07b8b4d25..1e5d0e511f674 100644 --- a/src/crimson/osd/osd_operations/common/pg_pipeline.h +++ b/src/crimson/osd/osd_operations/common/pg_pipeline.h @@ -19,7 +19,7 @@ protected: } wait_for_active; struct RecoverMissing : OrderedExclusivePhaseT { static constexpr auto type_name = "CommonPGPipeline::recover_missing"; - } recover_missing; + } recover_missing, recover_missing2; struct GetOBC : OrderedExclusivePhaseT { static constexpr auto type_name = "CommonPGPipeline::get_obc"; } get_obc; diff --git a/src/crimson/osd/osd_operations/internal_client_request.cc b/src/crimson/osd/osd_operations/internal_client_request.cc index c2a371d274f6c..bfa09fbe97a2e 100644 --- a/src/crimson/osd/osd_operations/internal_client_request.cc +++ b/src/crimson/osd/osd_operations/internal_client_request.cc @@ -85,7 +85,7 @@ seastar::future<> InternalClientRequest::start() std::as_const(osd_ops), pg->get_pgid().pgid, *pg->get_osdmap()); assert(ret == 0); return pg->with_locked_obc(get_target_oid(), op_info, - [&osd_ops, this](auto obc) { + [&osd_ops, this](auto, auto obc) { return enter_stage(client_pp().process ).then_interruptible( [obc=std::move(obc), &osd_ops, this] { diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc index c4dce8a8ee305..f2e01fe10341b 100644 --- a/src/crimson/osd/pg.cc +++ b/src/crimson/osd/pg.cc @@ -1209,9 +1209,9 @@ PG::with_locked_obc(const hobject_t &hobj, throw crimson::common::system_shutdown_exception(); } const hobject_t oid = get_oid(hobj); - auto wrapper = [f=std::move(f), this](auto obc) { + auto wrapper = [f=std::move(f), this](auto head, auto obc) { check_blocklisted_obc_watchers(obc); - return f(obc); + return f(head, obc); }; switch (get_lock_type(op_info)) { case RWState::RWREAD: diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h index 6a0231e452f60..f2b6bb611e423 100644 --- a/src/crimson/osd/pg.h +++ b/src/crimson/osd/pg.h @@ -504,7 +504,7 @@ public: public: using with_obc_func_t = - std::function (ObjectContextRef)>; + std::function (ObjectContextRef, ObjectContextRef)>; load_obc_iertr::future<> with_locked_obc( const hobject_t &hobj, diff --git a/src/crimson/osd/pg_backend.cc b/src/crimson/osd/pg_backend.cc index 02acb9a55d3f5..5e3e7a91c0f74 100644 --- a/src/crimson/osd/pg_backend.cc +++ b/src/crimson/osd/pg_backend.cc @@ -787,7 +787,7 @@ PGBackend::rollback_iertr::future<> PGBackend::rollback( return obc_loader.with_clone_obc_only( head, target_coid, [this, &os, &txn, &delta_stats, &osd_op_params] - (auto resolved_obc) { + (auto, auto resolved_obc) { if (resolved_obc->obs.oi.soid.is_head()) { // no-op: The resolved oid returned the head object logger().debug("PGBackend::rollback: loaded head_obc: {}" diff --git a/src/crimson/osd/replicated_recovery_backend.cc b/src/crimson/osd/replicated_recovery_backend.cc index bd301cc2b672b..8aaffef104d51 100644 --- a/src/crimson/osd/replicated_recovery_backend.cc +++ b/src/crimson/osd/replicated_recovery_backend.cc @@ -34,7 +34,7 @@ ReplicatedRecoveryBackend::recover_object( return maybe_pull_missing_obj(soid, need).then_interruptible([this, soid, need] { logger().debug("recover_object: loading obc: {}", soid); return pg.obc_loader.with_obc(soid, - [this, soid, need](auto obc) { + [this, soid, need](auto, auto obc) { logger().debug("recover_object: loaded obc: {}", obc->obs.oi.soid); auto& recovery_waiter = get_recovering(soid); recovery_waiter.obc = obc; @@ -689,7 +689,7 @@ ReplicatedRecoveryBackend::_handle_pull_response( if (pull_info.recovery_progress.first) { prepare_waiter = pg.obc_loader.with_obc( pull_info.recovery_info.soid, - [&pull_info, &recovery_waiter, &push_op](auto obc) { + [&pull_info, &recovery_waiter, &push_op](auto, auto obc) { pull_info.obc = obc; recovery_waiter.obc = obc; obc->obs.oi.decode_no_oid(push_op.attrset.at(OI_ATTR), push_op.soid); -- 2.39.5