mds/quiesce-db: incorporate review comments

Signed-off-by: Leonid Usov <leonid.usov@ibm.com>
(cherry picked from commit 9846d35a2ca0fc68c0464657616d259b19273b79)

diff --git a/src/mds/BoostUrlImpl.cc b/src/mds/BoostUrlImpl.cc
index c2e992a06e90cc0d9712f313384c7cc1ed91e545..479f4c6d75d0dff3c2591294fef274386bf2e4a4 100644 (file)
--- 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 <boost/url/src.hpp>

diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc
index 12f6865b2bc19d7556fcb0fd1fb9fde8fd360c18..c14ca9e1cb1e364af1df6fbb7533067204b58c07 100644 (file)
--- a/src/mds/MDSRank.cc
+++ b/src/mds/MDSRank.cc
@@ -48,8 +48,6 @@
 #include "QuiesceDbManager.h"
 #include "QuiesceAgent.h"
 
-#include <cmath>
-
 #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 0b753865698c24ec8cf29dc6f3ac7653493b9628..820cd33fcffa83097a0323cbd307be9d01c62594 100644 (file)
--- 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<RequestHandle, Context*>;
-  auto quiesce_requests = std::make_shared<std::unordered_map<QuiesceRoot, QuiescingRoot>>();
+  auto dummy_requests = std::make_shared<std::unordered_map<QuiesceRoot, QuiescingRoot>>();
 
   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<RequestHandle> {
     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<RequestHandle>();
     } 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 f5be435f2a2019a421beed1886828491fba19d16..4b1ef84b4a54bbd40ac33c0807aa353065f69337 100644 (file)
--- 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 84a86920f79ab723083428d9700f9c0840933db0..e78f32749cbb01a6b0821c372b17cb486af8fddb 100644 (file)
--- 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(