From 80be0ae0aae2cd6310c0967e1f52817bcab2f12c Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Thu, 20 Jun 2024 13:26:53 +0800 Subject: [PATCH] crimson/osd/osd_operations/client_request: hang client requests when the object is missing in the whole cluster Fixes: https://tracker.ceph.com/issues/65696 Signed-off-by: Xuehan Xu --- .../osd/osd_operations/client_request.cc | 17 +++++++++++++++-- .../osd/osd_operations/client_request_common.cc | 15 +++++++++++---- .../osd/osd_operations/client_request_common.h | 2 +- .../osd_operations/internal_client_request.cc | 10 +++++++++- src/crimson/osd/pg_recovery.cc | 1 + src/crimson/osd/recovery_backend.h | 17 +++++++++++++++++ 6 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc index 33d0f4c0f41..37efecb11b8 100644 --- a/src/crimson/osd/osd_operations/client_request.cc +++ b/src/crimson/osd/osd_operations/client_request.cc @@ -286,6 +286,7 @@ ClientRequest::recover_missing_snaps( ObjectContextRef head, std::set &snaps) { + LOG_PREFIX(ClientRequest::process_op); co_await ihref.enter_stage( client_pp(*pg).recover_missing_snaps, *this); for (auto &snap : snaps) { @@ -299,7 +300,12 @@ ClientRequest::recover_missing_snaps( * we skip the oid as there is no corresponding clone to recover. * See https://tracker.ceph.com/issues/63821 */ if (oid) { - co_await do_recover_missing(pg, *oid, m->get_reqid()); + auto unfound = co_await do_recover_missing(pg, *oid, m->get_reqid()); + if (unfound) { + DEBUGDPP("{} unfound, hang it for now", *pg, m->get_hobj().get_head()); + co_await interruptor::make_interruptible( + pg->get_recovery_backend()->add_unfound(m->get_hobj().get_head())); + } } } } @@ -317,7 +323,14 @@ ClientRequest::process_op( "Skipping recover_missings on non primary pg for soid {}", *pg, m->get_hobj()); } else { - co_await do_recover_missing(pg, m->get_hobj().get_head(), m->get_reqid()); + auto unfound = co_await do_recover_missing( + pg, 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( + pg->get_recovery_backend()->add_unfound(m->get_hobj().get_head())); + } + std::set snaps = snaps_need_to_recover(); if (!snaps.empty()) { // call with_obc() in order, but wait concurrently for loading. diff --git a/src/crimson/osd/osd_operations/client_request_common.cc b/src/crimson/osd/osd_operations/client_request_common.cc index c4439d5bb35..a56d58d2066 100644 --- a/src/crimson/osd/osd_operations/client_request_common.cc +++ b/src/crimson/osd/osd_operations/client_request_common.cc @@ -13,7 +13,7 @@ namespace { namespace crimson::osd { -typename InterruptibleOperation::template interruptible_future<> +typename InterruptibleOperation::template interruptible_future CommonClientRequest::do_recover_missing( Ref pg, const hobject_t& soid, @@ -45,22 +45,29 @@ CommonClientRequest::do_recover_missing( if (!needs_recovery_or_backfill) { logger().debug("{} reqid {} nothing to recover {}", __func__, reqid, soid); - return seastar::now(); + return seastar::make_ready_future(false); } + if (pg->get_peering_state().get_missing_loc().is_unfound(soid)) { + return seastar::make_ready_future(true); + } logger().debug("{} reqid {} need to wait for recovery, {} version {}", __func__, reqid, soid, ver); if (pg->get_recovery_backend()->is_recovering(soid)) { logger().debug("{} reqid {} object {} version {}, already recovering", __func__, reqid, soid, ver); - return pg->get_recovery_backend()->get_recovering(soid).wait_for_recovered(); + return pg->get_recovery_backend()->get_recovering( + soid).wait_for_recovered( + ).then([] { + return seastar::make_ready_future(false); + }); } else { logger().debug("{} reqid {} object {} version {}, starting recovery", __func__, reqid, soid, ver); auto [op, fut] = pg->get_shard_services().start_operation( soid, ver, pg, pg->get_shard_services(), pg->get_osdmap_epoch()); - return std::move(fut); + return fut.then([] { return seastar::make_ready_future(false); }); } } diff --git a/src/crimson/osd/osd_operations/client_request_common.h b/src/crimson/osd/osd_operations/client_request_common.h index 85f118d64c1..951bf653799 100644 --- a/src/crimson/osd/osd_operations/client_request_common.h +++ b/src/crimson/osd/osd_operations/client_request_common.h @@ -11,7 +11,7 @@ namespace crimson::osd { struct CommonClientRequest { - static InterruptibleOperation::template interruptible_future<> + static InterruptibleOperation::template interruptible_future do_recover_missing( Ref pg, const hobject_t& soid, diff --git a/src/crimson/osd/osd_operations/internal_client_request.cc b/src/crimson/osd/osd_operations/internal_client_request.cc index 22d7f3e492a..2968a6f4385 100644 --- a/src/crimson/osd/osd_operations/internal_client_request.cc +++ b/src/crimson/osd/osd_operations/internal_client_request.cc @@ -70,7 +70,12 @@ seastar::future<> InternalClientRequest::start() client_pp().recover_missing); }).then_interruptible([this] { return do_recover_missing(pg, get_target_oid(), osd_reqid_t()); - }).then_interruptible([this] { + }).then_interruptible([this](bool unfound) { + if (unfound) { + throw std::system_error( + std::make_error_code(std::errc::operation_canceled), + fmt::format("{} is unfound, drop it!", get_target_oid())); + } return enter_stage( client_pp().get_obc); }).then_interruptible([this] () -> PG::load_obc_iertr::future<> { @@ -128,6 +133,9 @@ seastar::future<> InternalClientRequest::start() }, pg, start_epoch); }).then([this] { track_event(); + }).handle_exception_type([](std::system_error &error) { + logger().debug("error {}, message: {}", error.code(), error.what()); + return seastar::now(); }).finally([this] { logger().debug("{}: exit", *this); handle.exit(); diff --git a/src/crimson/osd/pg_recovery.cc b/src/crimson/osd/pg_recovery.cc index 05f8c6e1f96..4ec68729607 100644 --- a/src/crimson/osd/pg_recovery.cc +++ b/src/crimson/osd/pg_recovery.cc @@ -431,6 +431,7 @@ void PGRecovery::on_global_recover ( auto& recovery_waiter = pg->get_recovery_backend()->get_recovering(soid); recovery_waiter.set_recovered(); pg->get_recovery_backend()->remove_recovering(soid); + pg->get_recovery_backend()->found_and_remove(soid); } void PGRecovery::on_failed_recover( diff --git a/src/crimson/osd/recovery_backend.h b/src/crimson/osd/recovery_backend.h index f5a365c1558..1225a920a1d 100644 --- a/src/crimson/osd/recovery_backend.h +++ b/src/crimson/osd/recovery_backend.h @@ -50,6 +50,18 @@ public: assert(it->second); return {*(it->second), added}; } + seastar::future<> add_unfound(const hobject_t &soid) { + auto [it, added] = unfound.emplace(soid, seastar::shared_promise()); + return it->second.get_shared_future(); + } + void found_and_remove(const hobject_t &soid) { + auto it = unfound.find(soid); + if (it != unfound.end()) { + auto &found_promise = it->second; + found_promise.set_value(); + unfound.erase(it); + } + } WaitForObjectRecovery& get_recovering(const hobject_t& soid) { assert(is_recovering(soid)); return *(recovering.at(soid)); @@ -91,6 +103,10 @@ public: for (auto& [soid, recovery_waiter] : recovering) { recovery_waiter->stop(); } + for (auto& [soid, promise] : unfound) { + promise.set_exception( + crimson::common::system_shutdown_exception()); + } return on_stop(); } protected: @@ -236,6 +252,7 @@ public: using WaitForObjectRecoveryRef = boost::intrusive_ptr; protected: std::map recovering; + std::map> unfound; hobject_t get_temp_recovery_object( const hobject_t& target, eversion_t version) const; -- 2.47.3