From 450a13ddc7055cd6092555e13f087f626769ce59 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 14 Feb 2020 10:48:39 -0500 Subject: [PATCH] rbd-mirror: snapshot replayer should only unlink peers for unneeded snaps Once the snapshot will no longer be needed by the peer for delta sync, it should be unlinked. This will ensure the primary image will not remove snapshots that will be needed for the next sync point. Signed-off-by: Jason Dillaman --- .../snapshot/test_mock_Replayer.cc | 51 ++++++------ .../image_replayer/snapshot/Replayer.cc | 77 +++++++++++-------- .../image_replayer/snapshot/Replayer.h | 12 +-- 3 files changed, 75 insertions(+), 65 deletions(-) diff --git a/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc b/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc index 25835d97ec783..e1e83ca454354 100644 --- a/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc +++ b/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc @@ -648,9 +648,6 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, SyncSnapshot) { MockImageCopyRequest mock_image_copy_request; expect_image_copy(mock_image_copy_request, 0, 1, 0, {}, {{1, CEPH_NOSNAP}}, 0); - MockUnlinkPeerRequest mock_unlink_peer_request; - expect_unlink_peer(mock_unlink_peer_request, 1, "remote mirror peer uuid", - 0); expect_mirror_image_snapshot_set_copy_progress( mock_local_image_ctx, 11, true, 0, 0); expect_notify_update(mock_local_image_ctx); @@ -673,11 +670,12 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, SyncSnapshot) { {{1, 11}, {4, CEPH_NOSNAP}}, 14, 0); expect_image_copy(mock_image_copy_request, 1, 4, 11, {}, {{1, 11}, {4, CEPH_NOSNAP}}, 0); - expect_unlink_peer(mock_unlink_peer_request, 4, "remote mirror peer uuid", - 0); + MockUnlinkPeerRequest mock_unlink_peer_request; expect_mirror_image_snapshot_set_copy_progress( mock_local_image_ctx, 14, true, 0, 0); expect_notify_update(mock_local_image_ctx); + expect_unlink_peer(mock_unlink_peer_request, 1, "remote mirror peer uuid", + 0); // idle expect_is_refresh_required(mock_local_image_ctx, true); @@ -754,9 +752,6 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, InterruptedSync) { expect_image_copy(mock_image_copy_request, 0, 1, 0, librbd::deep_copy::ObjectNumber{123U}, {{1, CEPH_NOSNAP}}, 0); - MockUnlinkPeerRequest mock_unlink_peer_request; - expect_unlink_peer(mock_unlink_peer_request, 1, "remote mirror peer uuid", - 0); expect_mirror_image_snapshot_set_copy_progress( mock_local_image_ctx, 11, true, 123, 0); expect_notify_update(mock_local_image_ctx); @@ -832,9 +827,6 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, RemoteImageDemoted) { MockImageCopyRequest mock_image_copy_request; expect_image_copy(mock_image_copy_request, 0, 1, 0, {}, {{1, CEPH_NOSNAP}}, 0); - MockUnlinkPeerRequest mock_unlink_peer_request; - expect_unlink_peer(mock_unlink_peer_request, 1, "remote mirror peer uuid", - 0); expect_mirror_image_snapshot_set_copy_progress( mock_local_image_ctx, 11, true, 0, 0); expect_notify_update(mock_local_image_ctx); @@ -1300,7 +1292,7 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, CopyImageError) { mock_remote_image_ctx)); } -TEST_F(TestMockImageReplayerSnapshotReplayer, UnlinkPeerError) { +TEST_F(TestMockImageReplayerSnapshotReplayer, UpdateNonPrimarySnapshotError) { librbd::MockTestImageCtx mock_local_image_ctx{*m_local_image_ctx}; librbd::MockTestImageCtx mock_remote_image_ctx{*m_remote_image_ctx}; @@ -1350,9 +1342,6 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, UnlinkPeerError) { MockImageCopyRequest mock_image_copy_request; expect_image_copy(mock_image_copy_request, 0, 1, 0, {}, {{1, CEPH_NOSNAP}}, 0); - MockUnlinkPeerRequest mock_unlink_peer_request; - expect_unlink_peer(mock_unlink_peer_request, 1, "remote mirror peer uuid", - 0); expect_mirror_image_snapshot_set_copy_progress( mock_local_image_ctx, 11, true, 0, -EINVAL); @@ -1368,7 +1357,7 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, UnlinkPeerError) { mock_remote_image_ctx)); } -TEST_F(TestMockImageReplayerSnapshotReplayer, UpdateNonPrimarySnapshotError) { +TEST_F(TestMockImageReplayerSnapshotReplayer, UnlinkPeerError) { librbd::MockTestImageCtx mock_local_image_ctx{*m_local_image_ctx}; librbd::MockTestImageCtx mock_remote_image_ctx{*m_remote_image_ctx}; @@ -1401,28 +1390,38 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, UpdateNonPrimarySnapshotError) { {1U, librbd::SnapInfo{"snap1", cls::rbd::MirrorSnapshotNamespace{ cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {"remote mirror peer uuid"}, "", 0U}, + 0, {}, 0, 0, {}}}, + {2U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {"remote mirror peer uuid"}, + "", 0U}, + 0, {}, 0, 0, {}}}}; + mock_local_image_ctx.snap_info = { + {11U, librbd::SnapInfo{"snap1", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY, {}, "remote mirror uuid", + 1, true, 0, {}}, 0, {}, 0, 0, {}}}}; - // sync snap1 + // sync snap2 expect_is_refresh_required(mock_local_image_ctx, false); expect_is_refresh_required(mock_remote_image_ctx, false); MockSnapshotCopyRequest mock_snapshot_copy_request; - expect_snapshot_copy(mock_snapshot_copy_request, 0, 1, 0, {{1, CEPH_NOSNAP}}, + expect_snapshot_copy(mock_snapshot_copy_request, 1, 2, 11, {{2, CEPH_NOSNAP}}, 0); MockGetImageStateRequest mock_get_image_state_request; - expect_get_image_state(mock_get_image_state_request, 1, 0); + expect_get_image_state(mock_get_image_state_request, 2, 0); MockCreateNonPrimaryRequest mock_create_non_primary_request; expect_create_non_primary_request(mock_create_non_primary_request, - false, "remote mirror uuid", 1, - {{1, CEPH_NOSNAP}}, 11, 0); + false, "remote mirror uuid", 2, + {{2, CEPH_NOSNAP}}, 12, 0); MockImageCopyRequest mock_image_copy_request; - expect_image_copy(mock_image_copy_request, 0, 1, 0, {}, - {{1, CEPH_NOSNAP}}, 0); + expect_image_copy(mock_image_copy_request, 1, 2, 11, {}, + {{2, CEPH_NOSNAP}}, 0); + expect_mirror_image_snapshot_set_copy_progress( + mock_local_image_ctx, 12, true, 0, 0); + expect_notify_update(mock_local_image_ctx); MockUnlinkPeerRequest mock_unlink_peer_request; expect_unlink_peer(mock_unlink_peer_request, 1, "remote mirror peer uuid", - 0); - expect_mirror_image_snapshot_set_copy_progress( - mock_local_image_ctx, 11, true, 0, -EINVAL); + -EINVAL); // wake-up replayer update_watch_ctx->handle_notify(); diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc index e2fcc6836c346..0a926429975ce 100644 --- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc +++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc @@ -585,7 +585,7 @@ void Replayer::handle_copy_image(int r) { return; } - unlink_peer(); + update_non_primary_snapshot(true); } template @@ -595,32 +595,6 @@ void Replayer::handle_copy_image_progress(uint64_t offset, uint64_t total) { // TODO } -template -void Replayer::unlink_peer() { - dout(10) << dendl; - - auto ctx = create_context_callback< - Replayer, &Replayer::handle_unlink_peer>(this); - auto req = librbd::mirror::snapshot::UnlinkPeerRequest::create( - m_state_builder->remote_image_ctx, m_remote_snap_id_end, - m_remote_mirror_peer_uuid, ctx); - req->send(); -} - -template -void Replayer::handle_unlink_peer(int r) { - dout(10) << "r=" << r << dendl; - - if (r < 0 && r != -ENOENT) { - derr << "failed to unlink local peer from remote image: " << cpp_strerror(r) - << dendl; - handle_replay_complete(r, "failed to unlink local peer from remote image"); - return; - } - - update_non_primary_snapshot(true); -} - template void Replayer::update_non_primary_snapshot(bool complete) { dout(10) << dendl; @@ -655,11 +629,6 @@ void Replayer::handle_update_non_primary_snapshot(bool complete, int r) { return; } - { - std::unique_lock locker{m_lock}; - notify_status_updated(); - } - notify_image_update(); } @@ -684,6 +653,49 @@ void Replayer::handle_notify_image_update(int r) { return; } + unlink_peer(); +} + +template +void Replayer::unlink_peer() { + if (m_remote_snap_id_start == 0) { + { + std::unique_lock locker{m_lock}; + notify_status_updated(); + } + + refresh_local_image(); + return; + } + + // local snapshot fully synced -- we no longer depend on the sync + // start snapshot in the remote image + dout(10) << "remote_snap_id=" << m_remote_snap_id_start << dendl; + + auto ctx = create_context_callback< + Replayer, &Replayer::handle_unlink_peer>(this); + auto req = librbd::mirror::snapshot::UnlinkPeerRequest::create( + m_state_builder->remote_image_ctx, m_remote_snap_id_start, + m_remote_mirror_peer_uuid, ctx); + req->send(); +} + +template +void Replayer::handle_unlink_peer(int r) { + dout(10) << "r=" << r << dendl; + + if (r < 0 && r != -ENOENT) { + derr << "failed to unlink local peer from remote image: " << cpp_strerror(r) + << dendl; + handle_replay_complete(r, "failed to unlink local peer from remote image"); + return; + } + + { + std::unique_lock locker{m_lock}; + notify_status_updated(); + } + refresh_local_image(); } @@ -699,7 +711,6 @@ void Replayer::register_update_watcher() { m_threads->work_queue->queue(ctx, r); } - template void Replayer::handle_register_update_watcher(int r) { dout(10) << "r=" << r << dendl; diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h index fd30bbdcce334..b28b69c073b35 100644 --- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h +++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h @@ -122,15 +122,15 @@ private: * | COPY_IMAGE | * | | | * | v | - * | UNLINK_PEER | - * | | | - * | v | * | UPDATE_NON_PRIMARY_SNAPSHOT | * | | | * | v | * | NOTIFY_IMAGE_UPDATE | * | | | * | v | + * | UNLINK_PEER | + * | | | + * | v | * | NOTIFY_LISTENER | * | | | * | \------------------------/| @@ -225,15 +225,15 @@ private: void handle_copy_image(int r); void handle_copy_image_progress(uint64_t offset, uint64_t total); - void unlink_peer(); - void handle_unlink_peer(int r); - void update_non_primary_snapshot(bool complete); void handle_update_non_primary_snapshot(bool complete, int r); void notify_image_update(); void handle_notify_image_update(int r); + void unlink_peer(); + void handle_unlink_peer(int r); + void register_update_watcher(); void handle_register_update_watcher(int r); -- 2.39.5