From febf0c62630c334558a2c19d86960c7041ed7a42 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Sat, 4 Jul 2026 06:18:36 +0000 Subject: [PATCH] crimson/mgr: prevent pg stats report pile-up in overloaded store When AlienStore's thread pool is saturated (e.g. during PG splitting), pool_statfs() calls inside OSD::get_stats() can get stuck behind hundreds of queued transactions in the free_slots semaphore. Because Client::report() dispatched get_stats() unconditionally every 5 seconds, hundreds of stuck pool_statfs() calls accumulated, further exhausting the semaphore and making recovery impossible. Fix by tracking whether a get_stats() call is already in flight and skipping new dispatches until the current one completes. Emit a cluster-log warning every ~60 seconds when stats reporting is stuck. This limits the damage from thread pool starvation to a single consumed free_slot instead of an unbounded pile-up. Fixes: https://tracker.ceph.com/issues/77930 Signed-off-by: Ronen Friedman --- src/crimson/mgr/client.cc | 26 ++++++++++++++++++++++++-- src/crimson/mgr/client.h | 12 +++++++++++- src/crimson/osd/osd.cc | 8 ++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/crimson/mgr/client.cc b/src/crimson/mgr/client.cc index 3980df47f9e..790f7453fe9 100644 --- a/src/crimson/mgr/client.cc +++ b/src/crimson/mgr/client.cc @@ -26,12 +26,14 @@ namespace crimson::mgr Client::Client(crimson::net::Messenger& msgr, WithStats& with_stats, set_perf_queries_cb_t cb_set, - get_perf_report_cb_t cb_get) + get_perf_report_cb_t cb_get, + stats_warning_cb_t stats_warning_cb) : msgr{msgr}, with_stats{with_stats}, report_timer{[this] {report();}}, set_perf_queries_cb(cb_set), - get_perf_report_cb(cb_get) + get_perf_report_cb(cb_get), + stats_warning_cb(std::move(stats_warning_cb)) {} seastar::future<> Client::start() @@ -214,11 +216,31 @@ void Client::report() LOG_PREFIX(Client::report); DEBUGDPP("", *this); _send_report(); + if (stats_in_flight) { + ++stats_skip_count; + if (stats_skip_count >= STATS_ABANDON_THRESHOLD) { + WARNDPP("pg stats send stuck for {} ticks, abandoning and retrying", + *this, stats_skip_count); + stats_in_flight = false; + } else { + WARNDPP("pg stats send still in flight, skipped (count={})", + *this, stats_skip_count); + if (stats_warning_cb && + stats_skip_count % STATS_WARN_INTERVAL == 0) { + stats_warning_cb(stats_skip_count); + } + return; + } + } + stats_in_flight = true; + stats_skip_count = 0; gates.dispatch_in_background(__func__, *this, [this, FNAME] { DEBUGDPP("dispatching in background", *this); return with_stats.get_stats( ).then([this](auto &&pg_stats) { return send(std::move(pg_stats)); + }).finally([this] { + stats_in_flight = false; }); }); } diff --git a/src/crimson/mgr/client.h b/src/crimson/mgr/client.h index 1ae104f0f7e..ddcec46fee5 100644 --- a/src/crimson/mgr/client.h +++ b/src/crimson/mgr/client.h @@ -35,11 +35,13 @@ class Client : public crimson::net::Dispatcher { using get_perf_report_cb_t = std::function ()>; using set_perf_queries_cb_t = std::function (const ConfigPayload &)>; + using stats_warning_cb_t = std::function; public: Client(crimson::net::Messenger& msgr, WithStats& with_stats, set_perf_queries_cb_t cb_set, - get_perf_report_cb_t cb_get); + get_perf_report_cb_t cb_get, + stats_warning_cb_t stats_warning_cb); seastar::future<> start(); seastar::future<> stop(); seastar::future<> send(MessageURef msg); @@ -75,6 +77,14 @@ private: std::vector daemon_health_metrics; + bool stats_in_flight = false; + uint32_t stats_skip_count = 0; + // emit a cluster-log warning every this many skipped reports (~60s) + static constexpr uint32_t STATS_WARN_INTERVAL = 12; + // abandon a stuck in-flight stats send after this many skips (~5min) + static constexpr uint32_t STATS_ABANDON_THRESHOLD = 5 * STATS_WARN_INTERVAL; + stats_warning_cb_t stats_warning_cb; + void _send_report(); }; diff --git a/src/crimson/osd/osd.cc b/src/crimson/osd/osd.cc index d742eef94be..3ef85860fbb 100644 --- a/src/crimson/osd/osd.cc +++ b/src/crimson/osd/osd.cc @@ -106,11 +106,19 @@ OSD::OSD(int id, uint32_t nonce, mgrc{new crimson::mgr::Client{ *public_msgr, *this, + // one lambda to perf-query them all [this](const ConfigPayload &config_payload) { return set_perf_queries(config_payload); }, + // one lambda to help _send_report() report them all [this] { return get_perf_reports(); + }, + // one lambda to log-warn if pg stats report is stuck + [this](uint32_t skips) { + clog->warn() << fmt::format( + "pg stats report stuck for ~{}s, store may be overloaded", + skips * 5); } }}, store{store}, -- 2.47.3