From 484d3534f4e278092ceca45c32ca3ff31b30bd24 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Sun, 20 Feb 2022 17:33:08 +0100 Subject: [PATCH] rbd-mirror: synchronize with in-flight stop in ImageReplayer::stop() Complete on_finish right away only if the replayer is stopped (meaning that it is legible to be restarted immediately, possibly from on_finish itself). This is the behaviour pretty much anyone would assume and also what ImageReplayer::restart() relies on. Fixes: https://tracker.ceph.com/issues/54344 Signed-off-by: Ilya Dryomov (cherry picked from commit 8965a0f2a6f7bdbe732be94b1ee269cab5be0a2a) --- .../rbd_mirror/test_mock_ImageReplayer.cc | 104 ++++++++++++++++++ src/tools/rbd_mirror/ImageReplayer.cc | 27 +++-- 2 files changed, 121 insertions(+), 10 deletions(-) diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc index 9f40797dd4f4c..c19d4923ab962 100644 --- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc @@ -840,5 +840,109 @@ TEST_F(TestMockImageReplayer, ReplayerRenamed) { ASSERT_EQ(image_spec, m_image_replayer->get_name()); } +TEST_F(TestMockImageReplayer, StopJoinInterruptedReplayer) { + // START + create_local_image(); + librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + + MockThreads mock_threads(m_threads); + expect_work_queue_repeatedly(mock_threads); + expect_add_event_after_repeatedly(mock_threads); + + MockReplayer mock_replayer; + expect_get_replay_status(mock_replayer); + expect_set_mirror_image_status_repeatedly(); + + InSequence seq; + MockBootstrapRequest mock_bootstrap_request; + MockStateBuilder mock_state_builder; + expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx, + false, false, 0); + + expect_create_replayer(mock_state_builder, mock_replayer); + expect_init(mock_replayer, 0); + + create_image_replayer(mock_threads); + + C_SaferCond start_ctx; + m_image_replayer->start(&start_ctx); + ASSERT_EQ(0, start_ctx.wait()); + + // NOTIFY + EXPECT_CALL(mock_replayer, is_resync_requested()) + .WillOnce(Return(false)); + EXPECT_CALL(mock_replayer, is_replaying()) + .WillOnce(Return(false)); + EXPECT_CALL(mock_replayer, get_error_code()) + .WillOnce(Return(-EINVAL)); + EXPECT_CALL(mock_replayer, get_error_description()) + .WillOnce(Return("INVALID")); + const double DELAY = 10; + EXPECT_CALL(mock_replayer, shut_down(_)) + .WillOnce(Invoke([this, DELAY](Context* ctx) { + m_threads->timer->add_event_after(DELAY, ctx); + })); + EXPECT_CALL(mock_replayer, destroy()); + expect_close(mock_state_builder, 0); + expect_mirror_image_status_exists(false); + + mock_replayer.replayer_listener->handle_notification(); + ASSERT_FALSE(m_image_replayer->is_running()); + + C_SaferCond stop_ctx; + m_image_replayer->stop(&stop_ctx); + ASSERT_EQ(ETIMEDOUT, stop_ctx.wait_for(DELAY * 3 / 4)); + ASSERT_EQ(0, stop_ctx.wait_for(DELAY)); +} + +TEST_F(TestMockImageReplayer, StopJoinRequestedStop) { + // START + create_local_image(); + librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + + MockThreads mock_threads(m_threads); + expect_work_queue_repeatedly(mock_threads); + expect_add_event_after_repeatedly(mock_threads); + + MockReplayer mock_replayer; + expect_get_replay_status(mock_replayer); + expect_set_mirror_image_status_repeatedly(); + + InSequence seq; + MockBootstrapRequest mock_bootstrap_request; + MockStateBuilder mock_state_builder; + expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx, + false, false, 0); + + expect_create_replayer(mock_state_builder, mock_replayer); + expect_init(mock_replayer, 0); + + create_image_replayer(mock_threads); + + C_SaferCond start_ctx; + m_image_replayer->start(&start_ctx); + ASSERT_EQ(0, start_ctx.wait()); + + // STOP + const double DELAY = 10; + EXPECT_CALL(mock_replayer, shut_down(_)) + .WillOnce(Invoke([this, DELAY](Context* ctx) { + m_threads->timer->add_event_after(DELAY, ctx); + })); + EXPECT_CALL(mock_replayer, destroy()); + expect_close(mock_state_builder, 0); + expect_mirror_image_status_exists(false); + + C_SaferCond stop_ctx1; + m_image_replayer->stop(&stop_ctx1); + + C_SaferCond stop_ctx2; + m_image_replayer->stop(&stop_ctx2); + ASSERT_EQ(ETIMEDOUT, stop_ctx2.wait_for(DELAY * 3 / 4)); + ASSERT_EQ(0, stop_ctx2.wait_for(DELAY)); + + ASSERT_EQ(0, stop_ctx1.wait_for(0)); +} + } // namespace mirror } // namespace rbd diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index 3d643f8aa69c8..191e714bde6e5 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -522,12 +522,11 @@ void ImageReplayer::stop(Context *on_finish, bool manual, bool restart) image_replayer::BootstrapRequest *bootstrap_request = nullptr; bool shut_down_replay = false; - bool running = true; + bool is_stopped = false; { std::lock_guard locker{m_lock}; if (!is_running_()) { - running = false; if (manual && !m_manual_stop) { dout(10) << "marking manual" << dendl; m_manual_stop = true; @@ -536,6 +535,15 @@ void ImageReplayer::stop(Context *on_finish, bool manual, bool restart) dout(10) << "canceling restart" << dendl; m_restart_requested = false; } + if (is_stopped_()) { + dout(10) << "already stopped" << dendl; + is_stopped = true; + } else { + dout(10) << "joining in-flight stop" << dendl; + if (on_finish != nullptr) { + m_on_stop_contexts.push_back(on_finish); + } + } } else { if (m_state == STATE_STARTING) { dout(10) << "canceling start" << dendl; @@ -557,6 +565,13 @@ void ImageReplayer::stop(Context *on_finish, bool manual, bool restart) } } + if (is_stopped) { + if (on_finish) { + on_finish->complete(-EINVAL); + } + return; + } + // avoid holding lock since bootstrap request will update status if (bootstrap_request != nullptr) { dout(10) << "canceling bootstrap" << dendl; @@ -564,14 +579,6 @@ void ImageReplayer::stop(Context *on_finish, bool manual, bool restart) bootstrap_request->put(); } - if (!running) { - dout(20) << "not running" << dendl; - if (on_finish) { - on_finish->complete(-EINVAL); - } - return; - } - if (shut_down_replay) { on_stop_journal_replay(); } -- 2.39.5