Skip to content

Commit 9a4c585

Browse files
committed
mds/quiesce-agt: never send a synchronous ack
Defer to the agent thread to perform all acking. This avoids race conditions between the updating thread and the acking thread. Fixes: https://tracker.ceph.com/issues/66219 Signed-off-by: Leonid Usov <[email protected]>
1 parent 4ab40ea commit 9a4c585

File tree

2 files changed

+19
-22
lines changed

2 files changed

+19
-22
lines changed

src/mds/QuiesceAgent.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,9 @@ bool QuiesceAgent::db_update(QuiesceMap& map)
8282
// ack with the known state stored in `map`
8383
set_pending_roots(map.db_version, std::move(new_roots));
8484

85-
// always send a synchronous ack
86-
return true;
85+
// to avoid ack races with the agent_thread,
86+
// never send a synchronous ack
87+
return false;
8788
}
8889

8990
void* QuiesceAgent::agent_thread_main() {

src/test/mds/TestQuiesceAgent.cc

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -278,16 +278,19 @@ TEST_F(QuiesceAgentTest, DbUpdates) {
278278

279279
// manipulate root0 and root1 as if they were quiesced and root2 as if it was released
280280
auto& root0 = *roots.at("root0");
281-
root0.quiesce_result = 0;
282-
EXPECT_EQ(QS_QUIESCED, root0.get_actual_state());
281+
complete_quiesce("root0", 0);
283282

284283
auto& root1 = *roots.at("root1");
285-
root1.quiesce_result = 0;
286-
EXPECT_EQ(QS_QUIESCED, root1.get_actual_state());
284+
complete_quiesce("root1", 0);
287285

288286
auto& root2 = *roots.at("root2");
289-
root2.quiesce_result = 0;
290-
root2.cancel_result = 0;
287+
complete_quiesce("root2", 0);
288+
root2.cancel_result = root2.cancel(*root2.quiesce_request);
289+
290+
EXPECT_TRUE(await_idle());
291+
292+
EXPECT_EQ(QS_QUIESCED, root0.get_actual_state());
293+
EXPECT_EQ(QS_QUIESCED, root1.get_actual_state());
291294
EXPECT_EQ(QS_RELEASED, root2.get_actual_state());
292295
}
293296

@@ -501,13 +504,10 @@ TEST_F(QuiesceAgentTest, DuplicateQuiesceRequest) {
501504
{ "root1", QS_QUIESCING },
502505
{ "root2", QS_QUIESCING },
503506
{ "root3", QS_QUIESCING },
504-
});
507+
}, WaitForAgent::No);
505508

506-
ASSERT_TRUE(ack.has_value());
507-
EXPECT_EQ(3, ack->db_version);
508-
// even though root1 is already quiesced,
509-
// we should not know about it synchronously
510-
EXPECT_EQ(0, ack->roots.size());
509+
// no sync update
510+
EXPECT_FALSE(ack.has_value());
511511
}
512512

513513
EXPECT_TRUE(await_idle());
@@ -600,21 +600,17 @@ TEST_F(QuiesceAgentTest, RapidDbUpdates)
600600
auto ack = update(2, {
601601
{ "root1", QS_QUIESCING },
602602
{ "root2", QS_QUIESCING },
603-
});
603+
}, WaitForAgent::No);
604604

605-
ASSERT_TRUE(ack.has_value());
606-
EXPECT_EQ(2, ack->db_version);
607-
EXPECT_EQ(0, ack->roots.size());
605+
EXPECT_FALSE(ack.has_value());
608606
};
609607

610608
{
611609
auto ack = update(1, {
612610
{ "root1", QS_QUIESCING },
613-
});
611+
}, WaitForAgent::No);
614612

615-
ASSERT_TRUE(ack.has_value());
616-
EXPECT_EQ(1, ack->db_version);
617-
EXPECT_EQ(0, ack->roots.size());
613+
EXPECT_FALSE(ack.has_value());
618614
}
619615

620616
EXPECT_TRUE(await_idle_v(2));

0 commit comments

Comments
 (0)