From 74bd4f230a0cb7b709f2cb5c6db3dc79f0d8dede Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 3 May 2017 21:36:21 -0400 Subject: [PATCH] rbd-mirror: ensure missing images are re-synced when detected Fixes: http://tracker.ceph.com/issues/19811 Signed-off-by: Jason Dillaman --- .../test_mock_BootstrapRequest.cc | 234 ++++++++++++++---- .../image_replayer/BootstrapRequest.cc | 12 +- .../image_replayer/BootstrapRequest.h | 1 - 3 files changed, 195 insertions(+), 52 deletions(-) diff --git a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc index b31ab74fb58..a7c7b27b7a4 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc @@ -3,7 +3,6 @@ #include "test/rbd_mirror/test_mock_fixture.h" #include "librbd/journal/TypeTraits.h" -#include "tools/rbd_mirror/ImageSync.h" #include "tools/rbd_mirror/ImageSyncThrottler.h" #include "tools/rbd_mirror/Threads.h" #include "tools/rbd_mirror/image_replayer/BootstrapRequest.h" @@ -44,40 +43,6 @@ namespace mirror { class ProgressContext; -template<> -struct ImageSync { - static ImageSync* s_instance; - Context *on_finish = nullptr; - - static ImageSync* create(librbd::MockTestImageCtx *local_image_ctx, - librbd::MockTestImageCtx *remote_image_ctx, - SafeTimer *timer, Mutex *timer_lock, - const std::string &mirror_uuid, - ::journal::MockJournaler *journaler, - librbd::journal::MirrorPeerClientMeta *client_meta, - ContextWQ *work_queue, Context *on_finish, - ProgressContext *progress_ctx = nullptr) { - assert(s_instance != nullptr); - return s_instance; - } - - ImageSync() { - assert(s_instance == nullptr); - s_instance = this; - } - - void put() { - } - - void get() { - } - - MOCK_METHOD0(send, void()); - MOCK_METHOD0(cancel, void()); -}; - -ImageSync* ImageSync::s_instance = nullptr; - template<> struct ImageSyncThrottler { MOCK_METHOD10(start_sync, void(librbd::MockTestImageCtx *local_image_ctx, @@ -124,6 +89,7 @@ struct CloseImageRequest { template<> struct CreateImageRequest { static CreateImageRequest* s_instance; + std::string *local_image_id = nullptr; Context *on_finish = nullptr; static CreateImageRequest* create(librados::IoCtx &local_io_ctx, @@ -135,7 +101,7 @@ struct CreateImageRequest { std::string *local_image_id, Context *on_finish) { assert(s_instance != nullptr); - *local_image_id = "local image id"; + s_instance->local_image_id = local_image_id; s_instance->on_finish = on_finish; return s_instance; } @@ -278,6 +244,7 @@ public: typedef ImageSyncThrottlerRef MockImageSyncThrottler; typedef BootstrapRequest MockBootstrapRequest; typedef CloseImageRequest MockCloseImageRequest; + typedef CreateImageRequest MockCreateImageRequest; typedef IsPrimaryRequest MockIsPrimaryRequest; typedef OpenImageRequest MockOpenImageRequest; typedef OpenLocalImageRequest MockOpenLocalImageRequest; @@ -321,6 +288,18 @@ public: })))); } + void expect_journaler_register_client(::journal::MockJournaler &mock_journaler, + const librbd::journal::ClientData &client_data, + int r) { + bufferlist bl; + ::encode(client_data, bl); + + EXPECT_CALL(mock_journaler, register_client(ContentsEqual(bl), _)) + .WillOnce(WithArg<1>(Invoke([this, r](Context *on_finish) { + m_threads->work_queue->queue(on_finish, r); + }))); + } + void expect_journaler_update_client(::journal::MockJournaler &mock_journaler, const librbd::journal::ClientData &client_data, int r) { @@ -346,12 +325,12 @@ public: void expect_open_local_image(MockOpenLocalImageRequest &mock_open_local_image_request, librados::IoCtx &io_ctx, const std::string &image_id, - librbd::MockTestImageCtx &mock_image_ctx, int r) { + librbd::MockTestImageCtx *mock_image_ctx, int r) { EXPECT_CALL(mock_open_local_image_request, construct(IsSameIoCtx(&io_ctx), image_id)); EXPECT_CALL(mock_open_local_image_request, send()) - .WillOnce(Invoke([this, &mock_open_local_image_request, &mock_image_ctx, r]() { - *mock_open_local_image_request.image_ctx = &mock_image_ctx; + .WillOnce(Invoke([this, &mock_open_local_image_request, mock_image_ctx, r]() { + *mock_open_local_image_request.image_ctx = mock_image_ctx; m_threads->work_queue->queue(mock_open_local_image_request.on_finish, r); })); @@ -393,6 +372,25 @@ public: Return(r))); } + void expect_create_image(MockCreateImageRequest &mock_create_image_request, + const std::string &image_id, int r) { + EXPECT_CALL(mock_create_image_request, send()) + .WillOnce(Invoke([this, &mock_create_image_request, image_id, r]() { + *mock_create_image_request.local_image_id = image_id; + m_threads->work_queue->queue(mock_create_image_request.on_finish, r); + })); + } + + void expect_image_sync(MockImageSyncThrottler image_sync_throttler, + int r) { + EXPECT_CALL(*image_sync_throttler, start_sync(_, _, _, _, + StrEq("local mirror uuid"), + _, _, _, _, _)) + .WillOnce(WithArg<8>(Invoke([this, r](Context *on_finish) { + m_threads->work_queue->queue(on_finish, r); + }))); + } + bufferlist encode_tag_data(const librbd::journal::TagData &tag_data) { bufferlist bl; ::encode(tag_data, bl); @@ -524,7 +522,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) { mock_local_image_ctx.journal = &mock_journal; MockOpenLocalImageRequest mock_open_local_image_request; expect_open_local_image(mock_open_local_image_request, m_local_io_ctx, - mock_local_image_ctx.id, mock_local_image_ctx, 0); + mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); // remote demotion / promotion event @@ -599,7 +597,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, MultipleRemoteDemotePromotes) { mock_local_image_ctx.journal = &mock_journal; MockOpenLocalImageRequest mock_open_local_image_request; expect_open_local_image(mock_open_local_image_request, m_local_io_ctx, - mock_local_image_ctx.id, mock_local_image_ctx, 0); + mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); // remote demotion / promotion event @@ -684,7 +682,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, LocalDemoteRemotePromote) { mock_local_image_ctx.journal = &mock_journal; MockOpenLocalImageRequest mock_open_local_image_request; expect_open_local_image(mock_open_local_image_request, m_local_io_ctx, - mock_local_image_ctx.id, mock_local_image_ctx, 0); + mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); // remote demotion / promotion event @@ -757,7 +755,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, SplitBrainForcePromote) { mock_local_image_ctx.journal = &mock_journal; MockOpenLocalImageRequest mock_open_local_image_request; expect_open_local_image(mock_open_local_image_request, m_local_io_ctx, - mock_local_image_ctx.id, mock_local_image_ctx, 0); + mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); // remote demotion / promotion event @@ -830,7 +828,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, ResyncRequested) { mock_local_image_ctx.journal = &mock_journal; MockOpenLocalImageRequest mock_open_local_image_request; expect_open_local_image(mock_open_local_image_request, m_local_io_ctx, - mock_local_image_ctx.id, mock_local_image_ctx, 0); + mock_local_image_ctx.id, &mock_local_image_ctx, 0); // resync is requested expect_is_resync_requested(mock_journal, true, 0); @@ -852,6 +850,154 @@ TEST_F(TestMockImageReplayerBootstrapRequest, ResyncRequested) { ASSERT_TRUE(m_do_resync); } +TEST_F(TestMockImageReplayerBootstrapRequest, PrimaryRemote) { + create_local_image(); + + InSequence seq; + + // lookup remote image tag class + cls::journal::Client client; + librbd::journal::ClientData client_data{ + librbd::journal::ImageClientMeta{123}}; + ::encode(client_data, client.data); + ::journal::MockJournaler mock_journaler; + expect_journaler_get_client(mock_journaler, + librbd::Journal<>::IMAGE_CLIENT_ID, + client, 0); + + // lookup local peer in remote journal + client = {}; + expect_journaler_get_client(mock_journaler, "local mirror uuid", + client, -ENOENT); + + // register missing client in remote journal + librbd::journal::MirrorPeerClientMeta mirror_peer_client_meta; + client_data.client_meta = mirror_peer_client_meta; + expect_journaler_register_client(mock_journaler, client_data, 0); + + // open the remote image + librbd::MockJournal mock_journal; + librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); + MockOpenImageRequest mock_open_image_request; + expect_open_image(mock_open_image_request, m_remote_io_ctx, + mock_remote_image_ctx.id, mock_remote_image_ctx, 0); + MockIsPrimaryRequest mock_is_primary_request; + expect_is_primary(mock_is_primary_request, true, 0); + + // create the local image + librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + mock_local_image_ctx.journal = &mock_journal; + + MockCreateImageRequest mock_create_image_request; + expect_create_image(mock_create_image_request, mock_local_image_ctx.id, 0); + + // open the local image + MockOpenLocalImageRequest mock_open_local_image_request; + expect_open_local_image(mock_open_local_image_request, m_local_io_ctx, + mock_local_image_ctx.id, &mock_local_image_ctx, 0); + expect_is_resync_requested(mock_journal, false, 0); + + // update client state back to syncing + mirror_peer_client_meta = {mock_local_image_ctx.id}; + mirror_peer_client_meta.state = librbd::journal::MIRROR_PEER_STATE_SYNCING; + client_data.client_meta = mirror_peer_client_meta; + client.data.clear(); + ::encode(client_data, client.data); + expect_journaler_update_client(mock_journaler, client_data, 0); + + // sync the remote image to the local image + MockImageSyncThrottler mock_image_sync_throttler( + new ImageSyncThrottler()); + expect_image_sync(mock_image_sync_throttler, 0); + + MockCloseImageRequest mock_close_image_request; + expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + + C_SaferCond ctx; + MockBootstrapRequest *request = create_request( + mock_image_sync_throttler, mock_journaler, "", + mock_remote_image_ctx.id, "global image id", "local mirror uuid", + "remote mirror uuid", &ctx); + request->send(); + ASSERT_EQ(0, ctx.wait()); +} + +TEST_F(TestMockImageReplayerBootstrapRequest, PrimaryRemoteLocalDeleted) { + create_local_image(); + + InSequence seq; + + // lookup remote image tag class + cls::journal::Client client; + librbd::journal::ClientData client_data{ + librbd::journal::ImageClientMeta{123}}; + ::encode(client_data, client.data); + ::journal::MockJournaler mock_journaler; + expect_journaler_get_client(mock_journaler, + librbd::Journal<>::IMAGE_CLIENT_ID, + client, 0); + + // lookup local peer in remote journal + librbd::journal::MirrorPeerClientMeta mirror_peer_client_meta{ + "missing image id"}; + mirror_peer_client_meta.state = librbd::journal::MIRROR_PEER_STATE_REPLAYING; + client_data.client_meta = mirror_peer_client_meta; + client.data.clear(); + ::encode(client_data, client.data); + expect_journaler_get_client(mock_journaler, "local mirror uuid", + client, 0); + + // open the remote image + librbd::MockJournal mock_journal; + librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx); + MockOpenImageRequest mock_open_image_request; + expect_open_image(mock_open_image_request, m_remote_io_ctx, + mock_remote_image_ctx.id, mock_remote_image_ctx, 0); + MockIsPrimaryRequest mock_is_primary_request; + expect_is_primary(mock_is_primary_request, true, 0); + + // open the missing local image + MockOpenLocalImageRequest mock_open_local_image_request; + expect_open_local_image(mock_open_local_image_request, m_local_io_ctx, + "missing image id", nullptr, -ENOENT); + + // create the missing local image + librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + mock_local_image_ctx.journal = &mock_journal; + + MockCreateImageRequest mock_create_image_request; + expect_create_image(mock_create_image_request, mock_local_image_ctx.id, 0); + + // open the local image + expect_open_local_image(mock_open_local_image_request, m_local_io_ctx, + mock_local_image_ctx.id, &mock_local_image_ctx, 0); + expect_is_resync_requested(mock_journal, false, 0); + + // update client state back to syncing + mirror_peer_client_meta = {mock_local_image_ctx.id}; + mirror_peer_client_meta.state = librbd::journal::MIRROR_PEER_STATE_SYNCING; + client_data.client_meta = mirror_peer_client_meta; + client.data.clear(); + ::encode(client_data, client.data); + expect_journaler_update_client(mock_journaler, client_data, 0); + + // sync the remote image to the local image + MockImageSyncThrottler mock_image_sync_throttler( + new ImageSyncThrottler()); + expect_image_sync(mock_image_sync_throttler, 0); + + MockCloseImageRequest mock_close_image_request; + expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + + C_SaferCond ctx; + MockBootstrapRequest *request = create_request( + mock_image_sync_throttler, mock_journaler, "", + mock_remote_image_ctx.id, "global image id", "local mirror uuid", + "remote mirror uuid", &ctx); + request->send(); + ASSERT_EQ(0, ctx.wait()); +} + } // namespace image_replayer } // namespace mirror } // namespace rbd diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc index 919bbbfd110..8818945b10f 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc @@ -20,7 +20,6 @@ #include "librbd/Journal.h" #include "librbd/Utils.h" #include "librbd/journal/Types.h" -#include "tools/rbd_mirror/ImageSync.h" #include "tools/rbd_mirror/ProgressContext.h" #include "tools/rbd_mirror/ImageSyncThrottler.h" @@ -414,7 +413,6 @@ void BootstrapRequest::handle_create_local_image(int r) { return; } - m_created_local_image = true; open_local_image(); } @@ -433,8 +431,8 @@ void BootstrapRequest::update_client_image() { dout(20) << dendl; - librbd::journal::MirrorPeerClientMeta client_meta; - client_meta.image_id = m_local_image_id; + librbd::journal::MirrorPeerClientMeta client_meta{m_local_image_id}; + client_meta.state = librbd::journal::MIRROR_PEER_STATE_SYNCING; librbd::journal::ClientData client_data(client_meta); bufferlist data_bl; @@ -464,7 +462,8 @@ void BootstrapRequest::handle_update_client_image(int r) { return; } - m_client_meta->image_id = m_local_image_id; + *m_client_meta = {m_local_image_id}; + m_client_meta->state = librbd::journal::MIRROR_PEER_STATE_SYNCING; get_remote_tags(); } @@ -474,8 +473,7 @@ void BootstrapRequest::get_remote_tags() { update_progress("GET_REMOTE_TAGS"); - if (m_created_local_image || - m_client_meta->state == librbd::journal::MIRROR_PEER_STATE_SYNCING) { + if (m_client_meta->state == librbd::journal::MIRROR_PEER_STATE_SYNCING) { // optimization -- no need to compare remote tags if we just created // the image locally or sync was interrupted image_sync(); diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h index 00636f03aea..6e755689488 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h @@ -167,7 +167,6 @@ private: uint64_t m_remote_tag_class = 0; ImageCtxT *m_remote_image_ctx = nullptr; bool m_primary = false; - bool m_created_local_image = false; int m_ret_val = 0; bufferlist m_out_bl; -- 2.39.5