From 76b676dcb76a6ce74a1e73463d618928a3e661ea Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 21 Jul 2017 16:12:13 -0400 Subject: [PATCH] rbd-mirror: moved wait for deletion logic within image replayer Signed-off-by: Jason Dillaman --- .../rbd_mirror/test_mock_ImageReplayer.cc | 61 +++++++++++++++---- .../rbd_mirror/test_mock_InstanceReplayer.cc | 16 ----- src/tools/rbd_mirror/ImageReplayer.cc | 39 ++++++++++-- src/tools/rbd_mirror/ImageReplayer.h | 8 ++- src/tools/rbd_mirror/InstanceReplayer.cc | 29 +-------- 5 files changed, 89 insertions(+), 64 deletions(-) diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc index a46761d82d3..8344fc6dead 100644 --- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc @@ -66,6 +66,9 @@ template <> struct ImageDeleter { MOCK_METHOD4(schedule_image_delete, void(RadosRef, int64_t, const std::string&, bool)); + MOCK_METHOD4(wait_for_scheduled_deletion, + void(int64_t, const std::string&, Context*, bool)); + MOCK_METHOD2(cancel_waiter, void(int64_t, const std::string&)); }; template<> @@ -320,6 +323,21 @@ public: ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &m_local_image_ctx)); } + void expect_wait_for_scheduled_deletion(MockImageDeleter& mock_image_deleter, + const std::string& global_image_id, + int r) { + EXPECT_CALL(mock_image_deleter, + wait_for_scheduled_deletion(_, global_image_id, _, false)) + .WillOnce(WithArg<2>(Invoke([this, r](Context *ctx) { + m_threads->work_queue->queue(ctx, r); + }))); + } + + void expect_cancel_waiter(MockImageDeleter& mock_image_deleter) { + EXPECT_CALL(mock_image_deleter, cancel_waiter(m_local_io_ctx.get_id(), + "global image id")); + } + bufferlist encode_tag_data(const librbd::journal::TagData &tag_data) { bufferlist bl; ::encode(tag_data, bl); @@ -512,6 +530,7 @@ TEST_F(TestMockImageReplayer, StartStop) { mock_local_image_ctx.journal = &mock_local_journal; journal::MockJournaler mock_remote_journaler; + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; @@ -522,6 +541,7 @@ TEST_F(TestMockImageReplayer, StartStop) { expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", @@ -540,7 +560,6 @@ TEST_F(TestMockImageReplayer, StartStop) { EXPECT_CALL(mock_remote_journaler, start_live_replay(_, _)); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -553,6 +572,7 @@ TEST_F(TestMockImageReplayer, StartStop) { MockCloseImageRequest mock_close_local_image_request; + expect_cancel_waiter(mock_image_deleter); expect_shut_down(mock_local_replay, true, 0); EXPECT_CALL(mock_local_journal, remove_listener(_)); EXPECT_CALL(mock_local_journal, stop_external_replay()); @@ -573,16 +593,17 @@ TEST_F(TestMockImageReplayer, LocalImagePrimary) { create_local_image(); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockReplayStatusFormatter mock_replay_status_formatter; expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "", 0); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -595,6 +616,7 @@ TEST_F(TestMockImageReplayer, LocalImageDNE) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_remote_journaler; + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; @@ -603,6 +625,7 @@ TEST_F(TestMockImageReplayer, LocalImageDNE) { expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, "", "", -ENOENT); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, 0); @@ -612,7 +635,6 @@ TEST_F(TestMockImageReplayer, LocalImageDNE) { EXPECT_CALL(mock_remote_journaler, remove_listener(_)); expect_shut_down(mock_remote_journaler, 0); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -624,16 +646,17 @@ TEST_F(TestMockImageReplayer, PrepareLocalImageError) { create_local_image(); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockReplayStatusFormatter mock_replay_status_formatter; expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", -EINVAL); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -645,6 +668,7 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdDNE) { create_local_image(); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockReplayStatusFormatter mock_replay_status_formatter; @@ -652,12 +676,12 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdDNE) { expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", "", -ENOENT); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -669,6 +693,7 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdNonLinkedDNE) { create_local_image(); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockReplayStatusFormatter mock_replay_status_formatter; @@ -676,12 +701,12 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdNonLinkedDNE) { expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "some other mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", "", -ENOENT); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -693,6 +718,7 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdError) { create_local_image(); librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockReplayStatusFormatter mock_replay_status_formatter; @@ -700,12 +726,12 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdError) { expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, -EINVAL); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -719,6 +745,7 @@ TEST_F(TestMockImageReplayer, BootstrapError) { librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); journal::MockJournaler mock_remote_journaler; + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; @@ -727,6 +754,7 @@ TEST_F(TestMockImageReplayer, BootstrapError) { expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", @@ -737,7 +765,6 @@ TEST_F(TestMockImageReplayer, BootstrapError) { EXPECT_CALL(mock_remote_journaler, remove_listener(_)); expect_shut_down(mock_remote_journaler, 0); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -755,6 +782,7 @@ TEST_F(TestMockImageReplayer, StartExternalReplayError) { mock_local_image_ctx.journal = &mock_local_journal; journal::MockJournaler mock_remote_journaler; + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; @@ -765,6 +793,7 @@ TEST_F(TestMockImageReplayer, StartExternalReplayError) { expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", @@ -788,7 +817,6 @@ TEST_F(TestMockImageReplayer, StartExternalReplayError) { EXPECT_CALL(mock_remote_journaler, remove_listener(_)); expect_shut_down(mock_remote_journaler, 0); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -808,6 +836,7 @@ TEST_F(TestMockImageReplayer, StopError) { mock_local_image_ctx.journal = &mock_local_journal; journal::MockJournaler mock_remote_journaler; + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; @@ -818,6 +847,7 @@ TEST_F(TestMockImageReplayer, StopError) { expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", @@ -836,7 +866,6 @@ TEST_F(TestMockImageReplayer, StopError) { EXPECT_CALL(mock_remote_journaler, start_live_replay(_, _)); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -847,6 +876,7 @@ TEST_F(TestMockImageReplayer, StopError) { MockCloseImageRequest mock_close_local_image_request; + expect_cancel_waiter(mock_image_deleter); expect_shut_down(mock_local_replay, true, -EINVAL); EXPECT_CALL(mock_local_journal, remove_listener(_)); EXPECT_CALL(mock_local_journal, stop_external_replay()); @@ -871,6 +901,7 @@ TEST_F(TestMockImageReplayer, Replay) { mock_local_image_ctx.journal = &mock_local_journal; journal::MockJournaler mock_remote_journaler; + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; @@ -884,6 +915,7 @@ TEST_F(TestMockImageReplayer, Replay) { expect_committed(mock_remote_journaler, 2); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", @@ -902,7 +934,6 @@ TEST_F(TestMockImageReplayer, Replay) { EXPECT_CALL(mock_remote_journaler, start_live_replay(_, _)); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -951,6 +982,7 @@ TEST_F(TestMockImageReplayer, Replay) { // STOP MockCloseImageRequest mock_close_local_image_request; + expect_cancel_waiter(mock_image_deleter); expect_shut_down(mock_local_replay, true, 0); EXPECT_CALL(mock_local_journal, remove_listener(_)); EXPECT_CALL(mock_local_journal, stop_external_replay()); @@ -975,6 +1007,7 @@ TEST_F(TestMockImageReplayer, DecodeError) { mock_local_image_ctx.journal = &mock_local_journal; journal::MockJournaler mock_remote_journaler; + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; @@ -987,6 +1020,7 @@ TEST_F(TestMockImageReplayer, DecodeError) { expect_get_commit_tid_in_debug(mock_replay_entry); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", @@ -1005,7 +1039,6 @@ TEST_F(TestMockImageReplayer, DecodeError) { EXPECT_CALL(mock_remote_journaler, start_live_replay(_, _)); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -1071,6 +1104,7 @@ TEST_F(TestMockImageReplayer, DelayedReplay) { mock_local_image_ctx.journal = &mock_local_journal; journal::MockJournaler mock_remote_journaler; + MockImageDeleter mock_image_deleter; MockPrepareLocalImageRequest mock_prepare_local_image_request; MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; @@ -1084,6 +1118,7 @@ TEST_F(TestMockImageReplayer, DelayedReplay) { expect_committed(mock_remote_journaler, 1); InSequence seq; + expect_wait_for_scheduled_deletion(mock_image_deleter, "global image id", 0); expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", @@ -1102,7 +1137,6 @@ TEST_F(TestMockImageReplayer, DelayedReplay) { EXPECT_CALL(mock_remote_journaler, start_live_replay(_, _)); - MockImageDeleter mock_image_deleter; create_image_replayer(mock_image_deleter); C_SaferCond start_ctx; @@ -1163,6 +1197,7 @@ TEST_F(TestMockImageReplayer, DelayedReplay) { MockCloseImageRequest mock_close_local_image_request; + expect_cancel_waiter(mock_image_deleter); expect_shut_down(mock_local_replay, true, 0); EXPECT_CALL(mock_local_journal, remove_listener(_)); EXPECT_CALL(mock_local_journal, stop_external_replay()); diff --git a/src/test/rbd_mirror/test_mock_InstanceReplayer.cc b/src/test/rbd_mirror/test_mock_InstanceReplayer.cc index accd0c7b20c..f12fcf2989e 100644 --- a/src/test/rbd_mirror/test_mock_InstanceReplayer.cc +++ b/src/test/rbd_mirror/test_mock_InstanceReplayer.cc @@ -42,10 +42,6 @@ struct Threads { template <> struct ImageDeleter { - MOCK_METHOD4(schedule_image_delete, void(RadosRef, int64_t, - const std::string&, bool)); - MOCK_METHOD4(wait_for_scheduled_deletion, - void(int64_t, const std::string&, Context*, bool)); }; template<> @@ -139,16 +135,6 @@ public: TestMockFixture::TearDown(); } - void expect_wait_for_scheduled_deletion(MockImageDeleter& mock_image_deleter, - const std::string& global_image_id, - int r) { - EXPECT_CALL(mock_image_deleter, - wait_for_scheduled_deletion(_, global_image_id, _, false)) - .WillOnce(WithArg<2>(Invoke([this, r](Context *ctx) { - m_threads->work_queue->queue(ctx, r); - }))); - } - MockThreads *m_mock_threads; }; @@ -169,8 +155,6 @@ TEST_F(TestMockInstanceReplayer, AcquireReleaseImage) { EXPECT_CALL(mock_image_replayer, is_blacklisted()) .WillRepeatedly(Return(false)); - expect_wait_for_scheduled_deletion(mock_image_deleter, "global_image_id", 0); - InSequence seq; instance_replayer.init(); diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index 434f50459ca..2cba8ed0619 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -20,6 +20,7 @@ #include "librbd/Operations.h" #include "librbd/Utils.h" #include "librbd/journal/Replay.h" +#include "ImageDeleter.h" #include "ImageReplayer.h" #include "Threads.h" #include "tools/rbd_mirror/image_replayer/BootstrapRequest.h" @@ -405,6 +406,31 @@ void ImageReplayer::start(Context *on_finish, bool manual) return; } + wait_for_deletion(); +} + +template +void ImageReplayer::wait_for_deletion() { + dout(20) << dendl; + + Context *ctx = create_context_callback< + ImageReplayer, &ImageReplayer::handle_wait_for_deletion>(this); + m_image_deleter->wait_for_scheduled_deletion( + m_local_pool_id, m_global_image_id, ctx, false); +} + +template +void ImageReplayer::handle_wait_for_deletion(int r) { + dout(20) << "r=" << r << dendl; + + if (r == -ECANCELED) { + on_start_fail(0, ""); + return; + } else if (r < 0) { + on_start_fail(r, "error waiting for image deletion"); + return; + } + prepare_local_image(); } @@ -679,12 +705,6 @@ void ImageReplayer::handle_start_replay(int r) { update_mirror_image_status(true, boost::none); reschedule_update_status_task(30); - dout(20) << "start succeeded" << dendl; - if (on_finish != nullptr) { - dout(20) << "on finish complete, r=" << r << dendl; - on_finish->complete(r); - } - if (on_replay_interrupted()) { return; } @@ -700,6 +720,11 @@ void ImageReplayer::handle_start_replay(int r) { dout(20) << "m_remote_journaler=" << *m_remote_journaler << dendl; } + dout(20) << "start succeeded" << dendl; + if (on_finish != nullptr) { + dout(20) << "on finish complete, r=" << r << dendl; + on_finish->complete(r); + } } template @@ -746,6 +771,8 @@ void ImageReplayer::stop(Context *on_finish, bool manual, int r, dout(20) << "on_finish=" << on_finish << ", manual=" << manual << ", desc=" << desc << dendl; + m_image_deleter->cancel_waiter(m_local_pool_id, m_global_image_id); + image_replayer::BootstrapRequest *bootstrap_request = nullptr; bool shut_down_replay = false; bool running = true; diff --git a/src/tools/rbd_mirror/ImageReplayer.h b/src/tools/rbd_mirror/ImageReplayer.h index d15adb280b0..c0d7cd17a43 100644 --- a/src/tools/rbd_mirror/ImageReplayer.h +++ b/src/tools/rbd_mirror/ImageReplayer.h @@ -15,7 +15,6 @@ #include "librbd/ImageCtx.h" #include "librbd/journal/Types.h" #include "librbd/journal/TypeTraits.h" -#include "ImageDeleter.h" #include "ProgressContext.h" #include "types.h" #include "tools/rbd_mirror/image_replayer/Types.h" @@ -48,6 +47,7 @@ namespace journal { template class Replay; } namespace rbd { namespace mirror { +template struct ImageDeleter; template struct InstanceWatcher; template struct Threads; @@ -128,6 +128,9 @@ protected: * v * * * * | * + * v * + * WAIT_FOR_DELETION * + * | * * v (error) * * PREPARE_LOCAL_IMAGE * * * * * * * * * * * * * * * * * * * | * @@ -370,6 +373,9 @@ private: void handle_shut_down(int r); void handle_remote_journal_metadata_updated(); + void wait_for_deletion(); + void handle_wait_for_deletion(int r); + void prepare_local_image(); void handle_prepare_local_image(int r); diff --git a/src/tools/rbd_mirror/InstanceReplayer.cc b/src/tools/rbd_mirror/InstanceReplayer.cc index 1df9a44f5f5..5df1af4a9ca 100644 --- a/src/tools/rbd_mirror/InstanceReplayer.cc +++ b/src/tools/rbd_mirror/InstanceReplayer.cc @@ -289,34 +289,7 @@ void InstanceReplayer::start_image_replayer( return; } - FunctionContext *ctx = new FunctionContext( - [this, global_image_id] (int r) { - dout(20) << "image deleter result: r=" << r << ", " - << "global_image_id=" << global_image_id << dendl; - - Mutex::Locker locker(m_lock); - m_async_op_tracker.finish_op(); - - if (r == -ESTALE || r == -ECANCELED) { - return; - } - - auto it = m_image_replayers.find(global_image_id); - if (it == m_image_replayers.end()) { - return; - } - - auto image_replayer = it->second; - if (r >= 0) { - image_replayer->start(nullptr, false); - } else { - start_image_replayer(image_replayer); - } - }); - - m_async_op_tracker.start_op(); - m_image_deleter->wait_for_scheduled_deletion( - m_local_pool_id, image_replayer->get_global_image_id(), ctx, false); + image_replayer->start(nullptr, false); } template -- 2.39.5