From 08d8bfbb5bc37388c25b838a37d68a8eac5dd0eb Mon Sep 17 00:00:00 2001
From: Leonid Usov <leonid.usov@ibm.com>
Date: Wed, 12 Jun 2024 08:39:53 +0300
Subject: [PATCH] mds/quiesce-db: calculate_quiesce_map: aggregate quiesce
 roots' TTL as `max`

Signed-off-by: Leonid Usov <leonid.usov@ibm.com>
Fixes: https://tracker.ceph.com/issues/66438
(cherry picked from commit 0fce4d7c3d98b543d4abaf7f04b86c69a1a6e955)
---
 src/mds/QuiesceDbManager.cc   |  7 ++--
 src/test/mds/TestQuiesceDb.cc | 78 ++++++++++++++++++++++++++++++++++-
 2 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/src/mds/QuiesceDbManager.cc b/src/mds/QuiesceDbManager.cc
index 696386bea8e21..177ff37cd40da 100644
--- a/src/mds/QuiesceDbManager.cc
+++ b/src/mds/QuiesceDbManager.cc
@@ -1208,10 +1208,11 @@ void QuiesceDbManager::calculate_quiesce_map(QuiesceMap &map)
         auto ttl = get_root_ttl(set, member, db_age);
         auto root_it = map.roots.try_emplace(root, QuiesceMap::RootInfo { requested, ttl }).first;
 
-        // the min below resolves conditions when members representing the same root have different state/ttl
-        // e.g. if at least one member is QUIESCING then the root should be QUIESCING
+        // the logic below resolves conditions when members representing the same root have different state/ttl
+        // The state should be min, e.g. QUIESCING if at least one member is QUIESCING
+        // The ttl should be large enough to cover all aggregated states, i.e. max
         root_it->second.state = std::min(root_it->second.state, requested);
-        root_it->second.ttl = std::min(root_it->second.ttl, ttl);
+        root_it->second.ttl = std::max(root_it->second.ttl, ttl);
       }
     }
   }
diff --git a/src/test/mds/TestQuiesceDb.cc b/src/test/mds/TestQuiesceDb.cc
index 4a3f8ac597a59..3ceaea7422db4 100644
--- a/src/test/mds/TestQuiesceDb.cc
+++ b/src/test/mds/TestQuiesceDb.cc
@@ -1659,4 +1659,80 @@ TEST_F(QuiesceDbTest, AckDuringEpochMismatch)
     r.set_id = "set1";
     r.await = sec(10);
   }));
-}
\ No newline at end of file
+}
+
+/* ==================================== */
+TEST_F(QuiesceDbTest, QuiesceRootMerge)
+{
+  ASSERT_NO_FATAL_FAILURE(configure_cluster({ mds_gid_t(1) }));
+  managers.at(mds_gid_t(1))->reset_agent_callback(QUIESCING_AGENT_CB);
+
+  ASSERT_EQ(OK(), run_request([](auto& r) {
+    r.set_id = "set1";
+    r.timeout = sec(60);
+    r.expiration = sec(60);
+    r.await = sec(60);
+    r.include_roots({ "root1", "root2" });
+  }));
+
+  EXPECT_EQ(QS_QUIESCED, last_request->response.sets.at("set1").rstate.state);
+  auto set1_exp = last_request->response.sets.at("set1").expiration;
+
+  // reset the agent callback to SILENT so that
+  // our sets stay RELEASING and QUIESCING forever
+  managers.at(mds_gid_t(1))->reset_agent_callback(SILENT_AGENT_CB);
+
+  ASSERT_EQ(OK(), run_request([](auto& r) {
+    r.set_id = "set1";
+    r.release();
+  }));
+
+  EXPECT_EQ(QS_RELEASING, last_request->response.sets.at("set1").rstate.state);
+
+  ASSERT_EQ(OK(), run_request([=](auto& r) {
+    r.set_id = "set2";
+    r.timeout = set1_exp*2;
+    r.expiration = set1_exp*2;
+    r.include_roots({ "root2", "root3" });
+  }));
+
+  EXPECT_EQ(QS_QUIESCING, last_request->response.sets.at("set2").rstate.state);
+
+  // at this point, we should expect to have root1 RELEASING, root3 QUIESCING
+  // and root2, which is shared, should take the min state (QUIESCING) and the max ttl
+
+  auto agent_map = [this]() -> std::optional<QuiesceMap> {
+    std::promise<QuiesceMap> agent_map_promise;
+    auto agent_map_future = agent_map_promise.get_future();
+
+    managers.at(mds_gid_t(1))->reset_agent_callback([&agent_map_promise](QuiesceMap& map) -> bool {
+      try {
+        agent_map_promise.set_value(map);
+      } catch (std::future_error) {
+        // ignore this if we accidentally get called more than once
+      }
+      return false;
+    });
+
+    if (std::future_status::ready == agent_map_future.wait_for(std::chrono::seconds(10))) {
+      return agent_map_future.get();
+    }
+    else {
+      return std::nullopt;
+    }
+  }();
+
+  ASSERT_TRUE(agent_map.has_value());
+  EXPECT_EQ(3, agent_map->roots.size());
+
+  {
+    auto const & r1 = agent_map->roots.at("file:/root1");
+    auto const & r2 = agent_map->roots.at("file:/root2");
+    auto const & r3 = agent_map->roots.at("file:/root3");
+
+    EXPECT_EQ(QS_RELEASING, r1.state);
+    EXPECT_EQ(QS_QUIESCING, r2.state);
+    EXPECT_EQ(QS_QUIESCING, r3.state);
+    EXPECT_EQ(std::max(r1.ttl, r3.ttl), r2.ttl);
+  }
+}
-- 
2.39.5