From: Jason Dillaman Date: Fri, 4 May 2018 15:14:34 +0000 (-0400) Subject: rbd-mirror: ensure remote demotion is replayed locally X-Git-Tag: v14.0.0~166^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=d35bc94319c6fa267e973e533f4591568289ee96;p=ceph-ci.git rbd-mirror: ensure remote demotion is replayed locally The bootstrap process cannot immediately quit if it notices the remote image is not primary. Instead, it needs to continue if the local image is still chained to the remote. Fixes: http://tracker.ceph.com/issues/24009 Signed-off-by: Jason Dillaman --- 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 bd8fb6893c4..16ce1ad0ac1 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc @@ -523,6 +523,63 @@ TEST_F(TestMockImageReplayerBootstrapRequest, NonPrimaryRemoteSyncingState) { ASSERT_EQ(-EREMOTEIO, ctx.wait()); } +TEST_F(TestMockImageReplayerBootstrapRequest, NonPrimaryRemoteNotTagOwner) { + 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); + + // 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); + + // test if remote image is primary + MockIsPrimaryRequest mock_is_primary_request; + expect_is_primary(mock_is_primary_request, false, 0); + + // open the local image + librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + 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); + expect_is_resync_requested(mock_journal, false, 0); + + expect_journal_get_tag_tid(mock_journal, 345); + expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::LOCAL_MIRROR_UUID, + librbd::Journal<>::ORPHAN_MIRROR_UUID, + true, 344, 0}); + + MockCloseImageRequest mock_close_image_request; + expect_close_image(mock_close_image_request, mock_local_image_ctx, 0); + expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + + C_SaferCond ctx; + MockInstanceWatcher mock_instance_watcher; + cls::journal::ClientState client_state = cls::journal::CLIENT_STATE_CONNECTED; + librbd::journal::MirrorPeerClientMeta mirror_peer_client_meta{ + mock_local_image_ctx.id}; + mirror_peer_client_meta.state = librbd::journal::MIRROR_PEER_STATE_REPLAYING; + MockBootstrapRequest *request = create_request( + &mock_instance_watcher, mock_journaler, mock_local_image_ctx.id, + mock_remote_image_ctx.id, "global image id", "local mirror uuid", + "remote mirror uuid", &client_state, &mirror_peer_client_meta, &ctx); + request->send(); + ASSERT_EQ(-EREMOTEIO, ctx.wait()); +} + TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) { create_local_image(); @@ -547,7 +604,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) { // test if remote image is primary MockIsPrimaryRequest mock_is_primary_request; - expect_is_primary(mock_is_primary_request, true, 0); + expect_is_primary(mock_is_primary_request, false, 0); // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); @@ -557,6 +614,9 @@ TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) { mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); + expect_journal_get_tag_tid(mock_journal, 345); + expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"}); + // remote demotion / promotion event Tags tags = { {2, 123, encode_tag_data({librbd::Journal<>::LOCAL_MIRROR_UUID, @@ -573,8 +633,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) { true, 4, 369})} }; expect_journaler_get_tags(mock_journaler, 123, tags, 0); - expect_journal_get_tag_tid(mock_journal, 345); - expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"}); MockCloseImageRequest mock_close_image_request; expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); @@ -627,6 +685,10 @@ TEST_F(TestMockImageReplayerBootstrapRequest, MultipleRemoteDemotePromotes) { mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); + expect_journal_get_tag_tid(mock_journal, 345); + expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::ORPHAN_MIRROR_UUID, + "remote mirror uuid", true, 4, 1}); + // remote demotion / promotion event Tags tags = { {2, 123, encode_tag_data({librbd::Journal<>::LOCAL_MIRROR_UUID, @@ -652,9 +714,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, MultipleRemoteDemotePromotes) { true, 7, 1})} }; expect_journaler_get_tags(mock_journaler, 123, tags, 0); - expect_journal_get_tag_tid(mock_journal, 345); - expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::ORPHAN_MIRROR_UUID, - "remote mirror uuid", true, 4, 1}); MockCloseImageRequest mock_close_image_request; expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); @@ -707,6 +766,12 @@ TEST_F(TestMockImageReplayerBootstrapRequest, LocalDemoteRemotePromote) { mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); + expect_journal_get_tag_tid(mock_journal, 346); + expect_journal_get_tag_data(mock_journal, + {librbd::Journal<>::ORPHAN_MIRROR_UUID, + librbd::Journal<>::LOCAL_MIRROR_UUID, + true, 345, 1}); + // remote demotion / promotion event Tags tags = { {2, 123, encode_tag_data({"local mirror uuid", "local mirror uuid", @@ -718,11 +783,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, LocalDemoteRemotePromote) { true, 3, 1})} }; expect_journaler_get_tags(mock_journaler, 123, tags, 0); - expect_journal_get_tag_tid(mock_journal, 346); - expect_journal_get_tag_data(mock_journal, - {librbd::Journal<>::ORPHAN_MIRROR_UUID, - librbd::Journal<>::LOCAL_MIRROR_UUID, - true, 345, 1}); MockCloseImageRequest mock_close_image_request; expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); @@ -775,6 +835,11 @@ TEST_F(TestMockImageReplayerBootstrapRequest, SplitBrainForcePromote) { mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); + expect_journal_get_tag_tid(mock_journal, 345); + expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::LOCAL_MIRROR_UUID, + librbd::Journal<>::ORPHAN_MIRROR_UUID, + true, 344, 0}); + // remote demotion / promotion event Tags tags = { {2, 123, encode_tag_data({librbd::Journal<>::LOCAL_MIRROR_UUID, @@ -785,10 +850,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, SplitBrainForcePromote) { true, 2, 1})} }; expect_journaler_get_tags(mock_journaler, 123, tags, 0); - expect_journal_get_tag_tid(mock_journal, 345); - expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::LOCAL_MIRROR_UUID, - librbd::Journal<>::ORPHAN_MIRROR_UUID, - true, 344, 0}); MockCloseImageRequest mock_close_image_request; expect_close_image(mock_close_image_request, mock_local_image_ctx, 0); @@ -845,6 +906,8 @@ TEST_F(TestMockImageReplayerBootstrapRequest, ResyncRequested) { // resync is requested expect_is_resync_requested(mock_journal, true, 0); + expect_journal_get_tag_tid(mock_journal, 345); + expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"}); MockCloseImageRequest mock_close_image_request; expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); @@ -914,6 +977,9 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrimaryRemote) { mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); + expect_journal_get_tag_tid(mock_journal, 345); + expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"}); + // sync the remote image to the local image MockImageSync mock_image_sync; expect_image_sync(mock_image_sync, 0); @@ -997,6 +1063,9 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrimaryRemoteLocalDeleted) { mock_local_image_ctx.id, &mock_local_image_ctx, 0); expect_is_resync_requested(mock_journal, false, 0); + expect_journal_get_tag_tid(mock_journal, 345); + expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"}); + // sync the remote image to the local image MockImageSync mock_image_sync; expect_image_sync(mock_image_sync, 0); diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc index e13652280fc..8417a005bf7 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc @@ -194,7 +194,12 @@ template void BootstrapRequest::handle_is_primary(int r) { dout(20) << ": r=" << r << dendl; - if (r < 0) { + if (r == -ENOENT) { + dout(5) << ": remote image is not mirrored" << dendl; + m_ret_val = -EREMOTEIO; + close_remote_image(); + return; + } else if (r < 0) { derr << ": error querying remote image primary status: " << cpp_strerror(r) << dendl; m_ret_val = r; @@ -203,11 +208,22 @@ void BootstrapRequest::handle_is_primary(int r) { } if (!m_primary) { - dout(5) << ": remote image is not primary -- skipping image replay" - << dendl; - m_ret_val = -EREMOTEIO; - update_client_state(); - return; + if (m_local_image_id.empty()) { + // no local image and remote isn't primary -- don't sync it + dout(5) << ": remote image is not primary -- not syncing" + << dendl; + m_ret_val = -EREMOTEIO; + close_remote_image(); + return; + } else if (m_client_meta->state != + librbd::journal::MIRROR_PEER_STATE_REPLAYING) { + // ensure we attempt to re-sync to remote if it's re-promoted + dout(5) << ": remote image is not primary -- sync interrupted" + << dendl; + m_ret_val = -EREMOTEIO; + update_client_state(); + return; + } } if (!m_client_meta->image_id.empty()) { @@ -217,6 +233,7 @@ void BootstrapRequest::handle_is_primary(int r) { } if (m_local_image_id.empty()) { + // prepare to create local image update_client_image(); return; } @@ -226,12 +243,6 @@ void BootstrapRequest::handle_is_primary(int r) { template void BootstrapRequest::update_client_state() { - if (m_client_meta->state == librbd::journal::MIRROR_PEER_STATE_REPLAYING) { - // state already set for replaying upon failover - close_remote_image(); - return; - } - dout(20) << dendl; update_progress("UPDATE_CLIENT_STATE"); @@ -300,8 +311,10 @@ void BootstrapRequest::handle_open_local_image(int r) { I *local_image_ctx = (*m_local_image_ctx); { - RWLock::RLocker snap_locker(local_image_ctx->snap_lock); + local_image_ctx->snap_lock.get_read(); if (local_image_ctx->journal == nullptr) { + local_image_ctx->snap_lock.put_read(); + derr << ": local image does not support journaling" << dendl; m_ret_val = -EINVAL; close_local_image(); @@ -310,11 +323,30 @@ void BootstrapRequest::handle_open_local_image(int r) { r = (*m_local_image_ctx)->journal->is_resync_requested(m_do_resync); if (r < 0) { + local_image_ctx->snap_lock.put_read(); + derr << ": failed to check if a resync was requested" << dendl; m_ret_val = r; close_local_image(); return; } + + m_local_tag_tid = local_image_ctx->journal->get_tag_tid(); + m_local_tag_data = local_image_ctx->journal->get_tag_data(); + dout(10) << ": local tag=" << m_local_tag_tid << ", " + << "local tag data=" << m_local_tag_data << dendl; + local_image_ctx->snap_lock.put_read(); + } + + if (m_local_tag_data.mirror_uuid != m_remote_mirror_uuid && !m_primary) { + // if the local mirror is not linked to the (now) non-primary image, + // stop the replay. Otherwise, we ignore that the remote is non-primary + // so that we can replay the demotion + dout(5) << ": remote image is not primary -- skipping image replay" + << dendl; + m_ret_val = -EREMOTEIO; + close_local_image(); + return; } if (*m_do_resync) { @@ -366,8 +398,8 @@ void BootstrapRequest::register_client() { update_progress("REGISTER_CLIENT"); - librbd::journal::MirrorPeerClientMeta mirror_peer_client_meta{ - m_local_image_id}; + assert(m_local_image_id.empty()); + librbd::journal::MirrorPeerClientMeta mirror_peer_client_meta; mirror_peer_client_meta.state = librbd::journal::MIRROR_PEER_STATE_REPLAYING; librbd::journal::ClientData client_data{mirror_peer_client_meta}; @@ -393,7 +425,7 @@ void BootstrapRequest::handle_register_client(int r) { } *m_client_state = cls::journal::CLIENT_STATE_CONNECTED; - *m_client_meta = librbd::journal::MirrorPeerClientMeta(m_local_image_id); + *m_client_meta = librbd::journal::MirrorPeerClientMeta(); m_client_meta->state = librbd::journal::MIRROR_PEER_STATE_REPLAYING; is_primary(); @@ -517,24 +549,6 @@ void BootstrapRequest::handle_get_remote_tags(int r) { // At this point, the local image was existing, non-primary, and replaying; // and the remote image is primary. Attempt to link the local image's most // recent tag to the remote image's tag chain. - uint64_t local_tag_tid; - librbd::journal::TagData local_tag_data; - I *local_image_ctx = (*m_local_image_ctx); - { - RWLock::RLocker snap_locker(local_image_ctx->snap_lock); - if (local_image_ctx->journal == nullptr) { - derr << ": local image does not support journaling" << dendl; - m_ret_val = -EINVAL; - close_local_image(); - return; - } - - local_tag_tid = local_image_ctx->journal->get_tag_tid(); - local_tag_data = local_image_ctx->journal->get_tag_data(); - dout(10) << ": local tag " << local_tag_tid << ": " - << local_tag_data << dendl; - } - bool remote_tag_data_valid = false; librbd::journal::TagData remote_tag_data; boost::optional remote_orphan_tag_tid = @@ -543,9 +557,9 @@ void BootstrapRequest::handle_get_remote_tags(int r) { // decode the remote tags for (auto &remote_tag : m_remote_tags) { - if (local_tag_data.predecessor.commit_valid && - local_tag_data.predecessor.mirror_uuid == m_remote_mirror_uuid && - local_tag_data.predecessor.tag_tid > remote_tag.tid) { + if (m_local_tag_data.predecessor.commit_valid && + m_local_tag_data.predecessor.mirror_uuid == m_remote_mirror_uuid && + m_local_tag_data.predecessor.tag_tid > remote_tag.tid) { dout(15) << ": skipping processed predecessor remote tag " << remote_tag.tid << dendl; continue; @@ -566,7 +580,7 @@ void BootstrapRequest::handle_get_remote_tags(int r) { dout(10) << ": decoded remote tag " << remote_tag.tid << ": " << remote_tag_data << dendl; - if (!local_tag_data.predecessor.commit_valid) { + if (!m_local_tag_data.predecessor.commit_valid) { // newly synced local image (no predecessor) replays from the first tag if (remote_tag_data.mirror_uuid != librbd::Journal<>::LOCAL_MIRROR_UUID) { dout(15) << ": skipping non-primary remote tag" << dendl; @@ -577,17 +591,17 @@ void BootstrapRequest::handle_get_remote_tags(int r) { break; } - if (local_tag_data.mirror_uuid == librbd::Journal<>::ORPHAN_MIRROR_UUID) { + if (m_local_tag_data.mirror_uuid == librbd::Journal<>::ORPHAN_MIRROR_UUID) { // demotion last available local epoch - if (remote_tag_data.mirror_uuid == local_tag_data.mirror_uuid && + if (remote_tag_data.mirror_uuid == m_local_tag_data.mirror_uuid && remote_tag_data.predecessor.commit_valid && remote_tag_data.predecessor.tag_tid == - local_tag_data.predecessor.tag_tid) { + m_local_tag_data.predecessor.tag_tid) { // demotion matches remote epoch if (remote_tag_data.predecessor.mirror_uuid == m_local_mirror_uuid && - local_tag_data.predecessor.mirror_uuid == + m_local_tag_data.predecessor.mirror_uuid == librbd::Journal<>::LOCAL_MIRROR_UUID) { // local demoted and remote has matching event dout(15) << ": found matching local demotion tag" << dendl; @@ -595,7 +609,7 @@ void BootstrapRequest::handle_get_remote_tags(int r) { continue; } - if (local_tag_data.predecessor.mirror_uuid == m_remote_mirror_uuid && + if (m_local_tag_data.predecessor.mirror_uuid == m_remote_mirror_uuid && remote_tag_data.predecessor.mirror_uuid == librbd::Journal<>::LOCAL_MIRROR_UUID) { // remote demoted and local has matching event @@ -621,7 +635,7 @@ void BootstrapRequest::handle_get_remote_tags(int r) { } if (remote_tag_data_valid && - local_tag_data.mirror_uuid == m_remote_mirror_uuid) { + m_local_tag_data.mirror_uuid == m_remote_mirror_uuid) { dout(10) << ": local image is in clean replay state" << dendl; } else if (reconnect_orphan) { dout(10) << ": remote image was demoted/promoted" << dendl; diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h index 8a46e7038d6..a28789b9f8f 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h @@ -8,6 +8,7 @@ #include "include/rados/librados.hpp" #include "common/Mutex.h" #include "cls/journal/cls_journal_types.h" +#include "librbd/journal/Types.h" #include "librbd/journal/TypeTraits.h" #include "tools/rbd_mirror/BaseRequest.h" #include "tools/rbd_mirror/types.h" @@ -174,6 +175,9 @@ private: int m_ret_val = 0; ImageSync *m_image_sync = nullptr; + uint64_t m_local_tag_tid = 0; + librbd::journal::TagData m_local_tag_data; + bufferlist m_out_bl; void get_remote_tag_class(); diff --git a/src/tools/rbd_mirror/image_replayer/IsPrimaryRequest.cc b/src/tools/rbd_mirror/image_replayer/IsPrimaryRequest.cc index ef2008b9994..e728e246171 100644 --- a/src/tools/rbd_mirror/image_replayer/IsPrimaryRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/IsPrimaryRequest.cc @@ -64,7 +64,7 @@ void IsPrimaryRequest::handle_get_mirror_state(int r) { return; } else if (mirror_image.state == cls::rbd::MIRROR_IMAGE_STATE_DISABLING) { dout(5) << ": image mirroring is being disabled" << dendl; - *m_primary = false; + r = -ENOENT; } else { derr << ": image mirroring is disabled" << dendl; r = -EINVAL; @@ -73,6 +73,8 @@ void IsPrimaryRequest::handle_get_mirror_state(int r) { derr << ": failed to decode image mirror state: " << cpp_strerror(r) << dendl; } + } else if (r == -ENOENT) { + dout(5) << ": image is not mirrored" << dendl; } else { derr << ": failed to retrieve image mirror state: " << cpp_strerror(r) << dendl; diff --git a/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc b/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc index 1894905fca1..aca3f04b875 100644 --- a/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc @@ -157,7 +157,11 @@ template void OpenLocalImageRequest::handle_is_primary(int r) { dout(20) << ": r=" << r << dendl; - if (r < 0) { + if (r == -ENOENT) { + dout(5) << ": local image is not mirrored" << dendl; + send_close_image(r); + return; + } else if (r < 0) { derr << ": error querying local image primary status: " << cpp_strerror(r) << dendl; send_close_image(r);