From: Mohit Agrawal Date: Mon, 6 Jul 2026 05:08:19 +0000 (+0530) Subject: crimson/osd: fix backfill deadlock in BackfillState::Waiting X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c6782dce474500baeecbd295ab3b74a621c3214f;p=ceph.git crimson/osd: fix backfill deadlock in BackfillState::Waiting BackfillState::Enqueuing unconditionally posts RequestWaiting{} when budget available return false. Waiting can exit only via ObjectPushed which requires an in-flight push. If the budget is exhausted with no pushes in flight no objectPush will ever arrive and the PG stalls permanently. Solution: Introduce a dedicated BudgetBlocked state. When budget_available() is false and tracked_objects_completed()==true post RequestBudgetBlocked{} instead of RequestWaiting{}.BudgetBlocked calls get_backfill_throttle() to suspend until a slots free up, then fires BudgetAvailable{} which transitions directly back to Enqueuing to dispatch the next batch of pushes. Fixes: https://tracker.ceph.com/issues/77804 Signed-off-by: Mohit Agrawal --- diff --git a/src/crimson/osd/backfill_state.cc b/src/crimson/osd/backfill_state.cc index 295113c2f16..e2b68c7ec49 100644 --- a/src/crimson/osd/backfill_state.cc +++ b/src/crimson/osd/backfill_state.cc @@ -398,7 +398,13 @@ BackfillState::Enqueuing::Enqueuing(my_context ctx) do { if (!backfill_listener().budget_available()) { - post_event(RequestWaiting{}); + if (backfill_state().progress_tracker->tracked_objects_completed()) { + INFODPP("backfill budget unavailable with nothing in flight, " + "entering BudgetBlocked", pg()); + post_event(RequestBudgetBlocked{}); + } else { + post_event(RequestWaiting{}); + } return; } else if (should_rescan_replicas(backfill_state().peer_backfill_info, primary_bi)) { @@ -653,6 +659,51 @@ BackfillState::Waiting::react(Triggered evt) return discard_event(); } +// -- BudgetBlocked +BackfillState::BudgetBlocked::BudgetBlocked(my_context ctx) + : my_base(ctx) +{ + LOG_PREFIX(BackfillState::BudgetBlocked::BudgetBlocked); + DEBUGDPP("budget unavailable with nothing in flight, " + "waiting for throttle slot to become available", pg()); + backfill_listener().request_budget_retry(); +} + +boost::statechart::result +BackfillState::BudgetBlocked::react(SuspendBackfill evt) +{ + LOG_PREFIX(BackfillState::BudgetBlocked::react::SuspendBackfill); + DEBUGDPP("suspended within BudgetBlocked", pg()); + backfill_state().on_suspended(); + return discard_event(); +} + +boost::statechart::result +BackfillState::BudgetBlocked::react(Triggered evt) +{ + LOG_PREFIX(BackfillState::BudgetBlocked::react::Triggered); + ceph_assert(backfill_state().is_suspended()); + if (backfill_state().on_resumed()) { + DEBUGDPP("Backfill resumed, going Enqueuing", pg()); + return transit(); + } + return discard_event(); +} + +boost::statechart::result +BackfillState::BudgetBlocked::react(BudgetAvailable evt) +{ + LOG_PREFIX(BackfillState::BudgetBlocked::react::BudgetAvailable); + DEBUGDPP("BudgetBlocked::react() on BudgetAvailable", pg()); + if (!backfill_state().is_suspended()) { + return transit(); + } else { + DEBUGDPP("backfill suspended, not going Enqueuing", pg()); + backfill_state().go_enqueuing_on_resume(); + } + return discard_event(); +} + // -- Done BackfillState::Done::Done(my_context ctx) : my_base(ctx) diff --git a/src/crimson/osd/backfill_state.h b/src/crimson/osd/backfill_state.h index 5870859abef..c47db0c9ad0 100644 --- a/src/crimson/osd/backfill_state.h +++ b/src/crimson/osd/backfill_state.h @@ -63,6 +63,12 @@ struct BackfillState { struct SuspendBackfill : sc::event { }; + struct RequestBudgetBlocked : sc::event { + }; + + struct BudgetAvailable : sc::event { + }; + private: // internal events struct RequestPrimaryScanning : sc::event { @@ -84,6 +90,7 @@ public: struct ReplicasScanning; struct Waiting; struct Done; + struct BudgetBlocked; struct BackfillMachine : sc::state_machine { BackfillMachine(BackfillState& backfill_state, @@ -157,6 +164,7 @@ public: sc::transition, sc::transition, sc::transition, + sc::transition, sc::transition>; explicit Enqueuing(my_context); @@ -281,6 +289,19 @@ public: } }; + struct BudgetBlocked : sc::state, + StateHelper { + using reactions = boost::mpl::list< + sc::custom_reaction, + sc::custom_reaction, + sc::custom_reaction, + sc::transition>; + explicit BudgetBlocked(my_context ctx); + sc::result react(BudgetAvailable); + sc::result react(SuspendBackfill); + sc::result react(Triggered); + }; + BackfillState(BackfillListener& backfill_listener, std::unique_ptr peering_state, std::unique_ptr pg); @@ -387,6 +408,8 @@ struct BackfillState::BackfillListener { virtual void backfilled() = 0; + virtual void request_budget_retry() = 0; + virtual ~BackfillListener() = default; }; @@ -487,5 +510,7 @@ public: #if FMT_VERSION >= 90000 template <> struct fmt::formatter : fmt::ostream_formatter {}; +template <> struct fmt::formatter + : fmt::ostream_formatter {}; #endif diff --git a/src/crimson/osd/pg_recovery.cc b/src/crimson/osd/pg_recovery.cc index 7b252e212cf..62014cfeb4d 100644 --- a/src/crimson/osd/pg_recovery.cc +++ b/src/crimson/osd/pg_recovery.cc @@ -593,6 +593,9 @@ void PGRecovery::enqueue_push( if (!added) return; peering_state.prepare_backfill_for_missing(obj, v, peers); + // release the budget_retry_releaser now that we're dispatching a real + // push -- recover_object_with_throttle will acquire its own slot + budget_retry_releaser.reset(); std::ignore = recover_object_with_throttle(obj, v).\ handle_exception_interruptible([] (auto) { ceph_abort_msg("got exception on backfill's push"); @@ -618,6 +621,10 @@ void PGRecovery::enqueue_drop( { LOG_PREFIX(PGRecovery::update_peers_last_backfill); DEBUGDPP("obj={} v={} target={}", *pg->get_dpp(), obj, v, target); + // release the budget_retry_releaser now that we're dispatching work + // (same as enqueue_push) -- slot was held to guarantee Enqueuing + // found budget available, drops don't need their own throttle slot + budget_retry_releaser.reset(); // allocate a pair if target is seen for the first time auto& req = backfill_drop_requests[target]; if (!req) { @@ -705,9 +712,46 @@ bool PGRecovery::budget_available() const return ss.throttle_available(); } +PGRecovery::interruptible_future<> +PGRecovery::do_request_budget_retry() +{ + LOG_PREFIX(PGRecovery::do_request_budget_retry); + DEBUGDPP("budget unavailable with nothing in flight, " + "waiting for throttle slot", *pg->get_dpp()); + auto releaser = co_await get_backfill_throttle(); + budget_retry_in_flight = false; + if (!backfill_state) { + DEBUGDPP("backfill_state is null, skipping BudgetAvailable " + "(pg cleaned or interval changed)", *pg->get_dpp()); + co_return; + } + // hold the releaser so the slot stays acquired until Enqueuing + budget_retry_releaser.emplace(std::move(releaser)); + if (backfill_state->is_triggered()) { + backfill_state->post_event( + BackfillState::BudgetAvailable{}.intrusive_from_this()); + } else { + backfill_state->process_event( + BackfillState::BudgetAvailable{}.intrusive_from_this()); + } +} + +void PGRecovery::request_budget_retry() +{ + LOG_PREFIX(PGRecovery::request_budget_retry); + if (budget_retry_in_flight) { + DEBUGDPP("budget retry already in flight, skipping", *pg->get_dpp()); + return; + } + budget_retry_in_flight = true; + std::ignore = do_request_budget_retry(); +} + void PGRecovery::on_pg_clean() { replica_scan_throttle_releasers.clear(); + budget_retry_releaser.reset(); + budget_retry_in_flight = false; backfill_state.reset(); } @@ -775,6 +819,8 @@ void PGRecovery::on_activate_complete() LOG_PREFIX(PGRecovery::on_activate_complete); DEBUGDPP("backfill_state={}", *pg->get_dpp(), fmt::ptr(backfill_state.get())); replica_scan_throttle_releasers.clear(); + budget_retry_releaser.reset(); + budget_retry_in_flight = false; backfill_state.reset(); } diff --git a/src/crimson/osd/pg_recovery.h b/src/crimson/osd/pg_recovery.h index 6d2a9e30ea2..c44f14b9e7d 100644 --- a/src/crimson/osd/pg_recovery.h +++ b/src/crimson/osd/pg_recovery.h @@ -126,13 +126,15 @@ private: return backend->recover_object(soid, need); } + interruptible_future<> do_request_budget_retry(); + // backfill begin std::unique_ptr backfill_state; std::map> backfill_drop_requests; std::map replica_scan_throttle_releasers; - + std::optional budget_retry_releaser; template void start_backfill_recovery( const EventT& evt); @@ -154,6 +156,7 @@ private: void update_peers_last_backfill( const hobject_t& new_last_backfill) final; bool budget_available() const final; + void request_budget_retry() final; template void start_peering_event_operation_listener(T &&evt, float delay = 0); @@ -163,6 +166,7 @@ private: friend crimson::osd::BackfillState::PGFacade; friend crimson::osd::PG; + bool budget_retry_in_flight = false; // backfill end };