From: Leonid Usov Date: Tue, 27 Feb 2024 11:36:16 +0000 (+0200) Subject: mds/quiesce-db: incorporate review comments X-Git-Tag: testing/wip-root-testing-20240411.174241~99^2~6 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=f4faf6b7d135e4756657424c1c1a7f5fd92ad942;p=ceph-ci.git mds/quiesce-db: incorporate review comments Signed-off-by: Leonid Usov (cherry picked from commit 9846d35a2ca0fc68c0464657616d259b19273b79) --- diff --git a/src/mds/BoostUrlImpl.cc b/src/mds/BoostUrlImpl.cc index c2e992a06e9..479f4c6d75d 100644 --- a/src/mds/BoostUrlImpl.cc +++ b/src/mds/BoostUrlImpl.cc @@ -1 +1,8 @@ +/* + * https://www.boost.org/doc/libs/1_82_0/libs/url/doc/html/url/overview.html#url.overview.requirements + * + * To use the library as header-only; that is, to eliminate the requirement + * to link a program to a static or dynamic Boost.URL library, + * simply place the following line in exactly one source file in your project. + */ #include diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc index 12f6865b2bc..c14ca9e1cb1 100644 --- a/src/mds/MDSRank.cc +++ b/src/mds/MDSRank.cc @@ -48,8 +48,6 @@ #include "QuiesceDbManager.h" #include "QuiesceAgent.h" -#include - #define dout_context g_ceph_context #define dout_subsys ceph_subsys_mds #undef dout_prefix diff --git a/src/mds/MDSRankQuiesce.cc b/src/mds/MDSRankQuiesce.cc index 0b753865698..820cd33fcff 100644 --- a/src/mds/MDSRankQuiesce.cc +++ b/src/mds/MDSRankQuiesce.cc @@ -349,7 +349,7 @@ void MDSRank::quiesce_cluster_update() { ++it; } else { // just ack. - dout(20) << "INTACTIVE RESPONDER: reporting '" << it->first << "' as " << it->second.state << dendl; + dout(20) << "INACTIVE RESPONDER: reporting '" << it->first << "' as " << it->second.state << dendl; it = quiesce_map.roots.erase(it); } break; @@ -416,21 +416,21 @@ void MDSRank::quiesce_agent_setup() { using RequestHandle = QuiesceInterface::RequestHandle; using QuiescingRoot = std::pair; - auto quiesce_requests = std::make_shared>(); + auto dummy_requests = std::make_shared>(); QuiesceAgent::ControlInterface ci; - ci.submit_request = [this, quiesce_requests](QuiesceRoot root, Context* c) + ci.submit_request = [this, dummy_requests](QuiesceRoot root, Context* c) -> std::optional { auto uri = boost::urls::parse_uri_reference(root); if (!uri) { dout(5) << "error parsing the quiesce root as an URI: " << uri.error() << dendl; c->complete(uri.error()); return std::nullopt; - } else { - dout(20) << "parsed root '" << root <<"' as : " << uri->path() << " " << uri->query() << dendl; } + dout(10) << "submit_request: " << uri << dendl; + std::chrono::milliseconds quiesce_delay_ms = 0ms; if (auto pit = uri->params().find("delayms"); pit != uri->params().end()) { try { @@ -481,7 +481,6 @@ void MDSRank::quiesce_agent_setup() { } auto path = uri->path(); - dout(20) << "got request to quiesce '" << path << "'" << dendl; std::lock_guard l(mds_lock); @@ -496,10 +495,10 @@ void MDSRank::quiesce_agent_setup() { auto mdr = mdcache->quiesce_path(filepath(path), qc, nullptr, quiesce_delay_ms); return mdr ? mdr->reqid : std::optional(); } else { - /* dummy quiesce/fail */ + /* we use this branch to allow for quiesce emulation for testing purposes */ // always create a new request id auto req_id = metareqid_t(entity_name_t::MDS(whoami), issue_tid()); - auto [it, inserted] = quiesce_requests->try_emplace(path, req_id, c); + auto [it, inserted] = dummy_requests->try_emplace(path, req_id, c); if (!inserted) { dout(3) << "duplicate quiesce request for root '" << it->first << "'" << dendl; @@ -535,11 +534,12 @@ void MDSRank::quiesce_agent_setup() { delay = debug_quiesce_after.value(); } - auto quiesce_task = new LambdaContext([quiesce_requests, req_id, do_fail, this](int) { + auto quiesce_task = new LambdaContext([dummy_requests, req_id, do_fail, this](int) { // the mds lock should be held by the timer + ceph_assert(ceph_mutex_is_locked_by_me(mds_lock)); dout(20) << "quiesce_task: callback by the timer" << dendl; - auto it = std::ranges::find(*quiesce_requests, req_id, [](auto x) { return x.second.first; }); - if (it != quiesce_requests->end() && it->second.second != nullptr) { + auto it = std::ranges::find(*dummy_requests, req_id, [](auto x) { return x.second.first; }); + if (it != dummy_requests->end() && it->second.second != nullptr) { dout(20) << "quiesce_task: completing the root '" << it->first << "' as failed: " << do_fail << dendl; it->second.second->complete(do_fail ? -EBADF : 0); it->second.second = nullptr; @@ -556,25 +556,30 @@ void MDSRank::quiesce_agent_setup() { } }; - ci.cancel_request = [this, quiesce_requests](RequestHandle h) { + ci.cancel_request = [this, dummy_requests](RequestHandle h) { std::lock_guard l(mds_lock); if (mdcache->have_request(h)) { auto qimdr = mdcache->request_get(h); mdcache->request_kill(qimdr); + // no reason to waste time checking for dummy requests return 0; } - auto it = std::ranges::find(*quiesce_requests, h, [](auto x) { return x.second.first; }); - if (it != quiesce_requests->end()) { + // if we get here then it could be a test (dummy) quiesce + auto it = std::ranges::find(*dummy_requests, h, [](auto x) { return x.second.first; }); + if (it != dummy_requests->end()) { if (auto ctx = it->second.second; ctx) { dout(20) << "canceling request with id '" << h << "' for root '" << it->first << "'" << dendl; ctx->complete(-ECANCELED); } - quiesce_requests->erase(it); + dummy_requests->erase(it); return 0; } + // we must indicate that the handle wasn't found + // so that the agent can properly report a missing + // outstanding quiesce, preventing a RELEASED transition return ENOENT; }; diff --git a/src/mds/QuiesceAgent.h b/src/mds/QuiesceAgent.h index f5be435f2a2..4b1ef84b4a5 100644 --- a/src/mds/QuiesceAgent.h +++ b/src/mds/QuiesceAgent.h @@ -34,7 +34,7 @@ class QuiesceAgent { }; ~QuiesceAgent() { - agent_thread.kill(SIGTERM); + shutdown(); } /// @brief WARNING: will reset syncrhonously @@ -72,7 +72,10 @@ class QuiesceAgent { stop_agent_thread = true; agent_cond.notify_all(); l.unlock(); - agent_thread.join(); + + if (agent_thread.is_started()) { + agent_thread.join(); + } current.clear(); pending.clear(); diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py index 84a86920f79..e78f32749cb 100644 --- a/src/pybind/mgr/mgr_module.py +++ b/src/pybind/mgr/mgr_module.py @@ -1753,6 +1753,18 @@ class MgrModule(ceph_module.BaseMgrModule, MgrModuleLoggingMixin): MDS_STATE_ACTIVE_ORD = MDS_STATE_ORD["up:active"] def get_quiesce_leader_info(self, fscid: str) -> Optional[dict]: + """ + Helper for `tell_quiesce_leader` to chose the mds to send the command to. + + Quiesce DB is managed by a leader which is selected based on the current MDSMap + The logic is currently implemented both here and on the MDS side, + see MDSRank::quiesce_cluster_update(). + + Ideally, this logic should be part of the MDSMonitor and the result should + be exposed via a dedicated field in the map, but until that is implemented + this function will have to be kept in sync with the corresponding logic + on the MDS side + """ leader_info: Optional[dict] = None for fs in self.get("fs_map")['filesystems']: @@ -1786,6 +1798,8 @@ class MgrModule(ceph_module.BaseMgrModule, MgrModuleLoggingMixin): self.log.warn("Couldn't resolve the quiesce leader for fscid %s" % fscid) return (-errno.ENOENT, "", "Couldn't resolve the quiesce leader for fscid %s" % fscid) self.log.debug("resolved quiesce leader for fscid {fscid} at daemon '{name}' gid {gid} rank {rank} ({state})".format(fscid=fscid, **qleader)) + # We use the one_shot here to cover for cases when the mds crashes + # without this parameter the client may get stuck awaiting response from a dead MDS return self.tell_command('mds', str(qleader['gid']), cmd_dict, one_shot=True) def send_command(