From: Sridhar Seshasayee Date: Mon, 2 Feb 2026 08:44:15 +0000 (+0530) Subject: mgr/DaemonServer: Modify offline_pg_report to handle set or vector types X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=083b39f254fecc45db02dcd1ff4f19032d890943;p=ceph.git mgr/DaemonServer: Modify offline_pg_report to handle set or vector types The offline_pg_report structure to be used by both the 'ok-to-stop' and 'ok-to-upgrade' commands is modified to handle either std::set or std::vector type containers. This is necessitated due to the differences in the way both commands work. For the 'ok-to-upgrade' command logic to work optimally, the items in the specified crush bucket including items found in the subtree must be strictly ordered. The earlier std::set container re-orders the items upon insertion by sorting the items which results in the offline pg check to report sub-optimal results. Therefore, the offline_pg_report struct is modified to use std::variant, std::set> as a ContainerType and handled accordingly in dump() using std::visit(). This ensures backward compatibility with the existing 'ok-to-stop' command while catering to the requirements of the new 'ok-to-upgrade' command. Signed-off-by: Sridhar Seshasayee (cherry picked from commit cf54988c504a7819c88bbd78d1a7f766b705ca43) Conflicts: src/mgr/DaemonServer.cc - In DaemonServer::_check_offlines_pgs(), resolve a compiler error when printing the list of osds which is of type std::variant. Since tentacle branch is still not transitioned to use std::variant instead of boost::variant, the operator<< is not handled like in the main branch(see commit: 09d2731). Therefore, use std::visit() to print the list of osds that could be of types std::set or std::vector by using a lambda and passing the CephContext pointer and a pointer to the offline pg report. --- diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc index 203017fd0ef2..b3ed7ac98f15 100644 --- a/src/mgr/DaemonServer.cc +++ b/src/mgr/DaemonServer.cc @@ -1009,7 +1009,7 @@ void DaemonServer::log_access_denied( } void DaemonServer::_check_offlines_pgs( - const std::set& osds, + const ContainerType& osds, const OSDMap& osdmap, const PGMap& pgmap, offline_pg_report *report) @@ -1027,20 +1027,36 @@ void DaemonServer::_check_offlines_pgs( } if (q.second.state & PG_STATE_DEGRADED) { for (auto& anm : q.second.avail_no_missing) { - if (osds.count(anm.osd)) { - found = true; - continue; - } + std::visit([anm, &found](auto& container) { + using T = std::decay_t; + if constexpr (std::is_same_v>) { + found = container.count(anm.osd); + } else if constexpr (std::is_same_v>) { + auto it = std::find(container.begin(), container.end(), anm.osd); + found = (it != container.end()); + } + }, osds); + if (found) { + continue; + } if (anm.osd != CRUSH_ITEM_NONE) { pg_acting.insert(anm.osd); } } } else { for (auto& a : q.second.acting) { - if (osds.count(a)) { - found = true; - continue; - } + std::visit([a, &found](auto& container) { + using T = std::decay_t; + if constexpr (std::is_same_v>) { + found = container.count(a); + } else if constexpr (std::is_same_v>) { + auto it = std::find(container.begin(), container.end(), a); + found = (it != container.end()); + } + }, osds); + if (found) { + continue; + } if (a != CRUSH_ITEM_NONE) { pg_acting.insert(a); } @@ -1074,10 +1090,13 @@ void DaemonServer::_check_offlines_pgs( } } } - dout(20) << osds << " -> " << report->ok.size() << " ok, " - << report->not_ok.size() << " not ok, " - << report->unknown.size() << " unknown" - << dendl; + CephContext *pcct = cct; // Resolve context outside lambda + std::visit([pcct, report](const auto& container) { + ldout(pcct, 20) << container << " -> " << report->ok.size() << " ok, " + << report->not_ok.size() << " not ok, " + << report->unknown.size() << " unknown" + << dendl; + }, osds); } void DaemonServer::_maximize_ok_to_stop_set( diff --git a/src/mgr/DaemonServer.h b/src/mgr/DaemonServer.h index 121fa6ab5ba7..6023e600811b 100644 --- a/src/mgr/DaemonServer.h +++ b/src/mgr/DaemonServer.h @@ -53,7 +53,8 @@ struct MDSPerfMetricQuery; struct offline_pg_report { - std::set osds; + using ContainerType = std::variant, std::set>; + ContainerType osds; std::set ok, not_ok, unknown; std::set ok_become_degraded, ok_become_more_degraded; // ok std::set bad_no_pool, bad_already_inactive, bad_become_inactive; // not ok @@ -65,9 +66,11 @@ struct offline_pg_report { void dump(Formatter *f) const { f->dump_bool("ok_to_stop", ok_to_stop()); f->open_array_section("osds"); - for (auto o : osds) { - f->dump_int("osd", o); - } + std::visit([&f](auto&& container) { + for (const auto& o : container) { + f->dump_int("osd", o); + } + }, osds); f->close_section(); f->dump_unsigned("num_ok_pgs", ok.size()); f->dump_unsigned("num_not_ok_pgs", not_ok.size()); @@ -172,6 +175,7 @@ protected: class DaemonServerHook *asok_hook; private: + using ContainerType = std::variant, std::set>; friend class ReplyOnFinish; bool _reply(MCommand* m, int ret, const std::string& s, const bufferlist& payload); @@ -179,7 +183,7 @@ private: void _prune_pending_service_map(); void _check_offlines_pgs( - const std::set& osds, + const ContainerType& osds, const OSDMap& osdmap, const PGMap& pgmap, offline_pg_report *report);