From: Leonid Usov Date: Thu, 6 Jun 2024 11:48:56 +0000 (+0300) Subject: squid: mds: QuiesceDbRequest: update the internal encoding of ops X-Git-Tag: v19.1.1~261^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=3df11b71070d12a577e3a6bce96abbac2abc66a9;p=ceph.git squid: mds: QuiesceDbRequest: update the internal encoding of ops Excluding the last root from a set will automatically mark it as QS_CANCELED. Hence, it makes more sense if `exclude` and `cancel` share the same op code, rather than `exclude` and `release`. Fixes: https://tracker.ceph.com/issues/66400 Signed-off-by: Leonid Usov Fixes: https://tracker.ceph.com/issues/66383 (cherry picked from commit dad52497817c372fd7c61a88a210b5a3613cb807) --- diff --git a/src/mds/MDSRankQuiesce.cc b/src/mds/MDSRankQuiesce.cc index 0262cee63f44e..4727d137f628a 100644 --- a/src/mds/MDSRankQuiesce.cc +++ b/src/mds/MDSRankQuiesce.cc @@ -197,9 +197,9 @@ void MDSRank::command_quiesce_db(const cmdmap_t& cmdmap, std::function void exclude_roots(R&& roots) { - set_roots(EXCLUDE_OR_RELEASE, std::forward(roots)); + set_roots(EXCLUDE_OR_CANCEL, std::forward(roots)); } - void release_roots() { - set_roots(EXCLUDE_OR_RELEASE, {}); + void release() { + set_roots(RESET_OR_RELEASE, {}); } template void reset_roots(R&& roots) { - set_roots(RESET_OR_CANCEL, std::forward(roots)); + set_roots(RESET_OR_RELEASE, std::forward(roots)); } - void cancel_roots() + void cancel() { - set_roots(RESET_OR_CANCEL, {}); + set_roots(EXCLUDE_OR_CANCEL, {}); } template @@ -522,10 +522,10 @@ struct QuiesceDbRequest { switch (control.roots_op) { case INCLUDE_OR_QUERY: return roots.empty() ? "query" : "include"; - case EXCLUDE_OR_RELEASE: - return roots.empty() ? "release" : "exclude"; - case RESET_OR_CANCEL: - return roots.empty() ? "cancel" : "reset"; + case EXCLUDE_OR_CANCEL: + return roots.empty() ? "cancel" : "exclude"; + case RESET_OR_RELEASE: + return roots.empty() ? "release" : "reset"; default: return ""; } @@ -547,11 +547,11 @@ operator<<(std::basic_ostream& os, const QuiesceDbRequest& req) os << "q-req[" << req.op_string(); if (req.set_id) { - os << " \"" << req.set_id << "\""; + os << " \"" << *req.set_id << "\""; } if (req.if_version) { - os << " ?v:" << req.if_version; + os << " ?v:" << *req.if_version; } if (req.await) { diff --git a/src/mds/QuiesceDbManager.cc b/src/mds/QuiesceDbManager.cc index 88844f09c817e..696386bea8e21 100644 --- a/src/mds/QuiesceDbManager.cc +++ b/src/mds/QuiesceDbManager.cc @@ -447,7 +447,7 @@ void QuiesceDbManager::complete_requests() { } // non-zero result codes are all errors - dout(10) << "completing request '" << req->request << " with rc: " << -res << dendl; + dout(10) << "completing " << req->request << " with rc: " << -res << dendl; req->complete(-res); } done_requests.clear(); @@ -589,6 +589,8 @@ int QuiesceDbManager::leader_process_request(RequestContext* req_ctx) return EINVAL; } + dout(20) << request << dendl; + const auto db_age = db.get_age(); if (request.is_cancel_all()) { diff --git a/src/test/mds/TestQuiesceDb.cc b/src/test/mds/TestQuiesceDb.cc index 19c8d9b6163d2..4a3f8ac597a59 100644 --- a/src/test/mds/TestQuiesceDb.cc +++ b/src/test/mds/TestQuiesceDb.cc @@ -562,7 +562,7 @@ TEST_F(QuiesceDbTest, QuiesceRequestValidation) return !testing::Test::HasFailure(); }; - const auto ops = std::array { QuiesceDbRequest::RootsOp::INCLUDE_OR_QUERY, QuiesceDbRequest::RootsOp::EXCLUDE_OR_RELEASE, QuiesceDbRequest::RootsOp::RESET_OR_CANCEL, QuiesceDbRequest::RootsOp::__INVALID }; + const auto ops = std::array { QuiesceDbRequest::RootsOp::INCLUDE_OR_QUERY, QuiesceDbRequest::RootsOp::EXCLUDE_OR_CANCEL, QuiesceDbRequest::RootsOp::RESET_OR_RELEASE, QuiesceDbRequest::RootsOp::__INVALID }; const auto strings = nullopt_and_default(); const auto versions = nullopt_and_default(); const auto intervals = nullopt_and_default(); @@ -766,7 +766,7 @@ TEST_F(QuiesceDbTest, SetModification) // cancel with no set_id should cancel all active sets ASSERT_EQ(OK(), run_request([](auto& r) { - r.control.roots_op = QuiesceDbRequest::RootsOp::RESET_OR_CANCEL; + r.cancel(); })); ASSERT_TRUE(db(mds_gid_t(1)).sets.at("set1").members.at("file:/root4").excluded); @@ -829,7 +829,7 @@ TEST_F(QuiesceDbTest, Timeouts) { ASSERT_EQ(OK(), run_request([](auto& r) { r.set_id = "set2"; - r.release_roots(); + r.release(); })); ASSERT_EQ(QuiesceState::QS_RELEASING, last_request->response.sets.at("set2").rstate.state); @@ -939,7 +939,7 @@ TEST_F(QuiesceDbTest, InterruptedQuiesceAwait) ASSERT_EQ(OK(), run_request([](auto& r) { r.set_id = "set1"; r.expiration = sec(100); - r.timeout = sec(10); + r.timeout = sec(100); r.roots.emplace("root1"); })); @@ -1006,7 +1006,7 @@ TEST_F(QuiesceDbTest, InterruptedQuiesceAwait) ASSERT_EQ(OK(), run_request([](auto& r) { r.set_id = "set1"; - r.reset_roots({}); + r.cancel(); })); EXPECT_EQ(ERR(ECANCELED), await3.wait_result()); @@ -1069,7 +1069,7 @@ TEST_F(QuiesceDbTest, RepeatedQuiesceAwait) { for (int i = 0; i < 2; i++) { ASSERT_EQ(ERR(EINPROGRESS), run_request([=](auto& r) { r.set_id = "set1"; - r.release_roots(); + r.release(); r.await = (expiration*2)/5; })); } @@ -1077,7 +1077,7 @@ TEST_F(QuiesceDbTest, RepeatedQuiesceAwait) { // NB: the ETIMEDOUT is the await result, while the set itself should be EXPIRED EXPECT_EQ(ERR(ETIMEDOUT), run_request([=](auto& r) { r.set_id = "set1"; - r.release_roots(); + r.release(); r.await = expiration; })); @@ -1090,7 +1090,7 @@ TEST_F(QuiesceDbTest, RepeatedQuiesceAwait) { EXPECT_EQ(ERR(EPERM), run_request([](auto& r) { r.set_id = "set1"; - r.release_roots(); + r.release(); })); } @@ -1115,7 +1115,7 @@ TEST_F(QuiesceDbTest, ReleaseAwait) for (auto&& set_id : { "set1", "set2" }) { ASSERT_EQ(ERR(EPERM), run_request([set_id](auto& r) { r.set_id = set_id; - r.release_roots(); + r.release(); r.await = sec(1); })) << "bad release-await " << set_id; } @@ -1134,13 +1134,13 @@ TEST_F(QuiesceDbTest, ReleaseAwait) auto & release_await1 = start_request([](auto &r) { r.set_id = "set1"; - r.release_roots(); + r.release(); r.await = sec(100); }); auto& release_await2 = start_request([](auto& r) { r.set_id = "set2"; - r.release_roots(); + r.release(); r.await = sec(100); }); @@ -1155,7 +1155,7 @@ TEST_F(QuiesceDbTest, ReleaseAwait) // we can request release again without any version bump EXPECT_EQ(OK(), run_request([](auto& r) { r.set_id = "set1"; - r.release_roots(); + r.release(); })); EXPECT_EQ(releasing_v1, last_request->response.sets.at("set1").version ); @@ -1163,7 +1163,7 @@ TEST_F(QuiesceDbTest, ReleaseAwait) // we can release-await with a short await timeout EXPECT_EQ(ERR(EINPROGRESS), run_request([](auto& r) { r.set_id = "set1"; - r.release_roots(); + r.release(); r.await = sec(0.1); })); @@ -1197,7 +1197,7 @@ TEST_F(QuiesceDbTest, ReleaseAwait) // await again auto& release_await22 = start_request([](auto& r) { r.set_id = "set2"; - r.release_roots(); + r.release(); r.await = sec(100); }); @@ -1235,18 +1235,18 @@ TEST_F(QuiesceDbTest, ReleaseAwait) // it should be OK to request release or release-await on a RELEASED set EXPECT_EQ(OK(), run_request([](auto& r) { r.set_id = "set1"; - r.release_roots(); + r.release(); })); EXPECT_EQ(OK(), run_request([](auto& r) { r.set_id = "set1"; - r.release_roots(); + r.release(); r.await = sec(0.1); })); // it's invalid to send a release without a set id EXPECT_EQ(ERR(EINVAL), run_request([](auto& r) { - r.release_roots(); + r.release(); })); } @@ -1453,7 +1453,7 @@ TEST_F(QuiesceDbTest, MultiRankRelease) // release roots ASSERT_EQ(OK(), run_request([](auto& r) { r.set_id = "set1"; - r.release_roots(); + r.release(); })); EXPECT_EQ(QS_RELEASING, last_request->response.sets.at("set1").rstate.state); @@ -1463,7 +1463,7 @@ TEST_F(QuiesceDbTest, MultiRankRelease) auto &async_release = start_request([](auto& r) { r.set_id = "set2"; r.await = sec(100); - r.release_roots(); + r.release(); }); EXPECT_EQ(NA(), async_release.check_result()); @@ -1471,7 +1471,7 @@ TEST_F(QuiesceDbTest, MultiRankRelease) // shouldn't hurt to run release twice for set 1 ASSERT_EQ(OK(), run_request([](auto& r) { r.set_id = "set1"; - r.release_roots(); + r.release(); })); EXPECT_EQ(releasing_v, last_request->response.sets.at("set1").version); @@ -1522,7 +1522,7 @@ TEST_F(QuiesceDbTest, MultiRankRelease) ASSERT_EQ(OK(), run_request([set_id](auto& r) { r.set_id = set_id; r.await = sec(100); - r.release_roots(); + r.release(); })); ASSERT_EQ(ERR(EPERM), run_request([set_id](auto& r) { r.set_id = set_id;