From: Leonid Usov Date: Wed, 22 May 2024 11:45:09 +0000 (+0300) Subject: squid: mds: command_quiesce_path: do not block the asok thread and return an adequate rc X-Git-Tag: v19.1.1~299^2~12 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=7c3ac6c39f82637f6a408a88ce48543493b88f99;p=ceph.git squid: mds: command_quiesce_path: do not block the asok thread and return an adequate rc Signed-off-by: Leonid Usov (cherry picked from commit df546a4fba0d3851644ce1607340484409a3677d) Fixes: https://tracker.ceph.com/issues/66258 --- diff --git a/qa/tasks/cephfs/test_quiesce.py b/qa/tasks/cephfs/test_quiesce.py index b6ef8cde324bc..c5b015d465a90 100644 --- a/qa/tasks/cephfs/test_quiesce.py +++ b/qa/tasks/cephfs/test_quiesce.py @@ -470,7 +470,7 @@ class TestQuiesce(QuiesceTestCase): self.mount_a.run_shell_payload("ln -s ../.. subvol_quiesce") path = self.mount_a.cephfs_mntpt + "/subvol_quiesce" - J = self.fs.rank_tell(["quiesce", "path", path, '--wait']) + J = self.fs.rank_tell(["quiesce", "path", path, '--wait'], check_status=False) log.debug(f"{J}") self.assertEqual(J['op']['result'], -20) # ENOTDIR: the link is not a directory @@ -484,7 +484,7 @@ class TestQuiesce(QuiesceTestCase): self.mount_a.run_shell_payload("ln -s ../../.. _nogroup") path = self.mount_a.cephfs_mntpt + "/_nogroup/" + self.QUIESCE_SUBVOLUME - J = self.fs.rank_tell(["quiesce", "path", path, '--wait']) + J = self.fs.rank_tell(["quiesce", "path", path, '--wait'], check_status=False) log.debug(f"{J}") self.assertEqual(J['op']['result'], -20) # ENOTDIR: path_traverse: the intermediate link is not a directory @@ -498,7 +498,7 @@ class TestQuiesce(QuiesceTestCase): self.mount_a.run_shell_payload("mkdir dir") path = self.mount_a.cephfs_mntpt + "/dir" - J = self.fs.rank_tell(["quiesce", "path", path, '--wait']) + J = self.fs.rank_tell(["quiesce", "path", path, '--wait'], check_status=False) reqid = self._reqid_tostr(J['op']['reqid']) self._wait_for_quiesce_complete(reqid, path=path) self._verify_quiesce(root=path) @@ -513,7 +513,7 @@ class TestQuiesce(QuiesceTestCase): self.mount_a.run_shell_payload("touch file") path = self.mount_a.cephfs_mntpt + "/file" - J = self.fs.rank_tell(["quiesce", "path", path, '--wait']) + J = self.fs.rank_tell(["quiesce", "path", path, '--wait'], check_status=False) log.debug(f"{J}") self.assertEqual(J['op']['result'], -20) # ENOTDIR @@ -525,9 +525,9 @@ class TestQuiesce(QuiesceTestCase): self._configure_subvolume() - op1 = self.fs.rank_tell(["quiesce", "path", self.subvolume])['op'] + op1 = self.fs.rank_tell(["quiesce", "path", self.subvolume], check_status=False)['op'] op1_reqid = self._reqid_tostr(op1['reqid']) - op2 = self.fs.rank_tell(["quiesce", "path", self.subvolume, '--wait'])['op'] + op2 = self.fs.rank_tell(["quiesce", "path", self.subvolume, '--wait'], check_status=False)['op'] op1 = self.fs.get_op(op1_reqid)['type_data'] # for possible dup result log.debug(f"op1 = {op1}") log.debug(f"op2 = {op2}") @@ -545,7 +545,7 @@ class TestQuiesce(QuiesceTestCase): self.mount_a.run_shell_payload("touch file") self.mount_a.setfattr("file", "ceph.quiesce.block", "1") - J = self.fs.rank_tell(["quiesce", "path", self.subvolume, '--wait']) + J = self.fs.rank_tell(["quiesce", "path", self.subvolume, '--wait'], check_status=False) log.debug(f"{J}") self.assertEqual(J['op']['result'], 0) self.assertEqual(J['state']['inodes_blocked'], 1) @@ -560,7 +560,7 @@ class TestQuiesce(QuiesceTestCase): self._configure_subvolume() self._client_background_workload() - J = self.fs.rank_tell(["quiesce", "path", self.subvolume]) + J = self.fs.rank_tell(["quiesce", "path", self.subvolume], check_status=False) log.debug(f"{J}") reqid = self._reqid_tostr(J['op']['reqid']) self._wait_for_quiesce_complete(reqid) @@ -582,7 +582,7 @@ class TestQuiesce(QuiesceTestCase): # drop cache self.fs.rank_tell(["cache", "drop"]) - J = self.fs.rank_tell(["quiesce", "path", self.subvolume]) + J = self.fs.rank_tell(["quiesce", "path", self.subvolume], check_status=False) log.debug(f"{J}") reqid = self._reqid_tostr(J['op']['reqid']) self._wait_for_quiesce_complete(reqid) @@ -684,7 +684,7 @@ class TestQuiesceMultiRank(QuiesceTestCase): self._client_background_workload() self._wait_distributed_subtrees(2*2, rank="all", path=self.mntpnt) - op = self.fs.rank_tell(["quiesce", "path", self.subvolume, '--wait'], rank=0)['op'] + op = self.fs.rank_tell(["quiesce", "path", self.subvolume, '--wait'], rank=0, check_status=False)['op'] self.assertEqual(op['result'], -1) # EPERM @unittest.skip("https://tracker.ceph.com/issues/66152") @@ -934,8 +934,8 @@ class TestQuiesceSplitAuth(QuiesceTestCase): sleep(2) - op0 = self.fs.rank_tell(["quiesce", "path", self.subvolume], rank=0)['op'] - op1 = self.fs.rank_tell(["quiesce", "path", self.subvolume], rank=1)['op'] + op0 = self.fs.rank_tell(["quiesce", "path", self.subvolume], rank=0, check_status=False)['op'] + op1 = self.fs.rank_tell(["quiesce", "path", self.subvolume], rank=1, check_status=False)['op'] reqid0 = self._reqid_tostr(op0['reqid']) reqid1 = self._reqid_tostr(op1['reqid']) op0 = self._wait_for_quiesce_complete(reqid0, rank=0, timeout=300) diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc index 66a027e77c791..772a0e9fd589b 100644 --- a/src/mds/MDCache.cc +++ b/src/mds/MDCache.cc @@ -13777,6 +13777,7 @@ void MDCache::dispatch_quiesce_inode(const MDRequestRef& mdr) qs.inc_inodes_quiesced(); auto* c = mdr->internal_op_finish; mdr->internal_op_finish = nullptr; // prevent ::request_kill recursion + mdr->result = 0; c->complete(0); /* do not respond/complete so locks are not lost, parent request will complete */ @@ -13876,6 +13877,7 @@ void MDCache::dispatch_quiesce_path(const MDRequestRef& mdr) dout(5) << __func__ << ": skipping recursive quiesce of path for non-auth inode" << dendl; mdr->mark_event("quiesce complete for non-auth tree"); } else if (auto& qops = mdr->more()->quiesce_ops; qops.count(dirino) == 0) { + mdr->mark_event("quiescing root"); MDRequestRef qimdr = request_start_internal(CEPH_MDS_OP_QUIESCE_INODE); qimdr->set_filepath(filepath(dirino)); qimdr->internal_op_finish = new C_MDS_RetryRequest(this, mdr); @@ -13895,12 +13897,12 @@ void MDCache::dispatch_quiesce_path(const MDRequestRef& mdr) mdr->mark_event("quiesce complete"); } + mdr->result = 0; if (qfinisher) { auto* c = mdr->internal_op_finish; mdr->internal_op_finish = nullptr; // prevent ::request_kill recursion c->complete(0); } - mdr->result = 0; /* caller kills this op */ } diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc index 73e48889b584e..ebac99014d689 100644 --- a/src/mds/MDSRank.cc +++ b/src/mds/MDSRank.cc @@ -3019,7 +3019,8 @@ void MDSRankDispatcher::handle_asok_command( std::lock_guard l(mds_lock); mdcache->cache_status(f); } else if (command == "quiesce path") { - r = command_quiesce_path(f, cmdmap, *css); + command_quiesce_path(f, cmdmap, std::move(on_finish)); + return; } else if (command == "lock path") { command_lock_path(f, cmdmap, std::move(on_finish)); return; @@ -3464,51 +3465,55 @@ void MDSRank::command_openfiles_ls(Formatter *f) class C_MDS_QuiescePathCommand : public MDCache::C_MDS_QuiescePath { public: - C_MDS_QuiescePathCommand(MDCache* cache, Context* fin) : C_MDS_QuiescePath(cache), finisher(fin) {} + C_MDS_QuiescePathCommand(MDCache* cache) : C_MDS_QuiescePath(cache) {} void finish(int rc) override { - if (finisher) { - finisher->complete(rc); - finisher = nullptr; + if (auto fin = std::move(finish_once)) { + fin(rc, *this); } } -private: - Context* finisher = nullptr; + std::function finish_once; }; -int MDSRank::command_quiesce_path(Formatter* f, const cmdmap_t& cmdmap, std::ostream& ss) +void MDSRank::command_quiesce_path(Formatter* f, const cmdmap_t& cmdmap, std::function on_finish) { std::string path; - { - bool got = cmd_getval(cmdmap, "path", path); - if (!got) { - ss << "missing path"; - return -CEPHFS_EINVAL; - } + if (!cmd_getval(cmdmap, "path", path)) { + bufferlist bl; + on_finish(-EINVAL, "missing path", bl); + return; } - bool wait = false; - cmd_getval(cmdmap, "wait", wait); + bool wait = cmd_getval_or(cmdmap, "wait", false); C_SaferCond cond; - auto* finisher = new C_MDS_QuiescePathCommand(mdcache, wait ? &cond : nullptr); - auto qs = finisher->qs; - MDRequestRef mdr; - f->open_object_section("quiesce"); - { - std::lock_guard l(mds_lock); - mdr = mdcache->quiesce_path(filepath(path), finisher, f); - if (!wait) { - f->dump_object("op", *mdr); - } - } - if (wait) { - cond.wait(); - std::lock_guard l(mds_lock); - f->dump_object("op", *mdr); + auto* quiesce_ctx = new C_MDS_QuiescePathCommand(mdcache); + + quiesce_ctx->finish_once = [f, respond = std::move(on_finish)](int cephrc, C_MDS_QuiescePathCommand const& cmd) { + f->open_object_section("response"); + f->dump_object("op", *cmd.mdr); + f->dump_object("state", *cmd.qs); + f->close_section(); + + bufferlist bl; + // need to do this manually, because the default asok + // on_finish handler doesn't flush the formatter for rc < 0 + f->flush(bl); + auto rc = cephrc < 0 ? -ceph_to_hostos_errno(-cephrc) : cephrc; + respond(rc, "", bl); + }; + + std::lock_guard l(mds_lock); + + auto mdr = mdcache->quiesce_path(filepath(path), quiesce_ctx, f); + + // This is a little ugly, apologies. + // We should still be under the mds lock for this test to be valid. + // MDCache will delete the quiesce_ctx if it manages to complete syncrhonously, + // so we are testing the `mdr->internal_op_finish` to see if that has happend + if (!wait && mdr && mdr->internal_op_finish) { + ceph_assert(mdr->internal_op_finish == quiesce_ctx); + quiesce_ctx->finish(mdr->result.value_or(0)); } - f->dump_object("state", *qs); - f->close_section(); - return 0; } void MDSRank::command_lock_path(Formatter* f, const cmdmap_t& cmdmap, std::function on_finish) diff --git a/src/mds/MDSRank.h b/src/mds/MDSRank.h index 128050ebe3493..3dfda80ee6907 100644 --- a/src/mds/MDSRank.h +++ b/src/mds/MDSRank.h @@ -526,7 +526,7 @@ class MDSRank { std::ostream &ss); void command_openfiles_ls(Formatter *f); void command_dump_tree(const cmdmap_t &cmdmap, std::ostream &ss, Formatter *f); - int command_quiesce_path(Formatter *f, const cmdmap_t &cmdmap, std::ostream &ss); + void command_quiesce_path(Formatter *f, const cmdmap_t &cmdmap, std::function on_finish); void command_lock_path(Formatter* f, const cmdmap_t& cmdmap, std::function on_finish); void command_dump_inode(Formatter *f, const cmdmap_t &cmdmap, std::ostream &ss); void command_dump_dir(Formatter *f, const cmdmap_t &cmdmap, std::ostream &ss);