From 2ba05cb028a4b3a9f6ec74aacd28758bb74619e8 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Sat, 28 May 2022 20:06:22 +0200 Subject: [PATCH] rbd-mirror: don't prune non-primary snapshot when restarting delta sync When restarting interrupted sync (signified by the "end" non-primary snapshot with last_copied_object_number > 0), preserve the "start" non-primary snapshot until the sync is completed, like it would have been done had the sync not been interrupted. This ensures that the same m_local_snap_id_start is passed to scan_remote_mirror_snapshots() and ultimately ImageCopyRequest state machine on restart as on initial start. This ends up being yet another fixup for 281af0de86b1 ("rbd-mirror: prune unnecessary non-primary mirror snapshots"), following earlier 7ba9214ea5b7 ("rbd-mirror: don't prune older mirror snapshots when pruning incomplete snapshot") and ecd3778a6f9a ("rbd-mirror: ensure that the last non-primary snapshot cannot be pruned"). Fixes: https://tracker.ceph.com/issues/55796 Signed-off-by: Ilya Dryomov (cherry picked from commit 3ba82f2aa73871af9ef190c06e2c99eed6d21e7b) --- .../snapshot/test_mock_Replayer.cc | 122 +++++++++++++++++- .../image_replayer/snapshot/Replayer.cc | 2 +- 2 files changed, 122 insertions(+), 2 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 8990a6071909f..30bd17aac5eb6 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 @@ -933,7 +933,7 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, SyncSnapshot) { mock_remote_image_ctx)); } -TEST_F(TestMockImageReplayerSnapshotReplayer, InterruptedSync) { +TEST_F(TestMockImageReplayerSnapshotReplayer, InterruptedSyncInitial) { librbd::MockTestImageCtx mock_local_image_ctx{*m_local_image_ctx}; librbd::MockTestImageCtx mock_remote_image_ctx{*m_remote_image_ctx}; @@ -1018,6 +1018,126 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, InterruptedSync) { mock_remote_image_ctx)); } +TEST_F(TestMockImageReplayerSnapshotReplayer, InterruptedSyncDelta) { + librbd::MockTestImageCtx mock_local_image_ctx{*m_local_image_ctx}; + librbd::MockTestImageCtx mock_remote_image_ctx{*m_remote_image_ctx}; + + MockThreads mock_threads(m_threads); + expect_work_queue_repeatedly(mock_threads); + + MockReplayerListener mock_replayer_listener; + expect_notification(mock_threads, mock_replayer_listener); + + InSequence seq; + + MockInstanceWatcher mock_instance_watcher; + MockImageMeta mock_image_meta; + MockStateBuilder mock_state_builder(mock_local_image_ctx, + mock_remote_image_ctx, + mock_image_meta); + MockReplayer mock_replayer{&mock_threads, &mock_instance_watcher, + "local mirror uuid", &m_pool_meta_cache, + &mock_state_builder, &mock_replayer_listener}; + m_pool_meta_cache.set_remote_pool_meta( + m_remote_io_ctx.get_id(), + {"remote mirror uuid", "remote mirror peer uuid"}); + + librbd::UpdateWatchCtx* update_watch_ctx = nullptr; + ASSERT_EQ(0, init_entry_replayer(mock_replayer, mock_threads, + mock_local_image_ctx, + mock_remote_image_ctx, + mock_replayer_listener, + mock_image_meta, + &update_watch_ctx)); + + // inject a incomplete sync snapshot after a complete snapshot + mock_remote_image_ctx.snap_info = { + {1U, librbd::SnapInfo{"snap1", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {"remote mirror peer uuid"}, + "", CEPH_NOSNAP, true, 0, {}}, + 0, {}, 0, 0, {}}}, + {2U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {"remote mirror peer uuid"}, + "", CEPH_NOSNAP, true, 0, {}}, + 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, {{1, CEPH_NOSNAP}}}, + 0, {}, 0, 0, {}}}, + {12U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY, {}, "remote mirror uuid", + 2, false, 123, {{2, CEPH_NOSNAP}}}, + 0, {}, 0, 0, {}}}}; + + // re-sync snap2 + expect_load_image_meta(mock_image_meta, false, 0); + expect_is_refresh_required(mock_local_image_ctx, false); + expect_is_refresh_required(mock_remote_image_ctx, false); + MockGetImageStateRequest mock_get_image_state_request; + expect_get_image_state(mock_get_image_state_request, 12, 0); + expect_notify_sync_request(mock_instance_watcher, mock_local_image_ctx.id, 0); + MockImageCopyRequest mock_image_copy_request; + expect_image_copy(mock_image_copy_request, 1, 2, 11, + librbd::deep_copy::ObjectNumber{123U}, + {{2, CEPH_NOSNAP}}, 0); + MockApplyImageStateRequest mock_apply_state_request; + expect_apply_image_state(mock_apply_state_request, 0); + expect_mirror_image_snapshot_set_copy_progress( + mock_local_image_ctx, 12, true, 123, 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_notify_sync_complete(mock_instance_watcher, mock_local_image_ctx.id); + + // prune non-primary snap1 + expect_load_image_meta(mock_image_meta, false, 0); + expect_is_refresh_required(mock_local_image_ctx, true); + expect_refresh( + mock_local_image_ctx, { + {11U, librbd::SnapInfo{"snap1", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY, {}, "remote mirror uuid", + 1, true, 0, {}}, + 0, {}, 0, 0, {}}}, + {12U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY, {}, "remote mirror uuid", + 2, true, 0, {}}, + 0, {}, 0, 0, {}}}, + }, 0); + expect_is_refresh_required(mock_remote_image_ctx, true); + expect_refresh( + mock_remote_image_ctx, { + {2U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {"remote mirror peer uuid"}, + "", CEPH_NOSNAP, true, 0, {}}, + 0, {}, 0, 0, {}}}, + }, 0); + expect_prune_non_primary_snapshot(mock_local_image_ctx, 11, 0); + + // idle + expect_load_image_meta(mock_image_meta, false, 0); + expect_is_refresh_required(mock_local_image_ctx, true); + expect_refresh( + mock_local_image_ctx, { + {12U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY, {}, "remote mirror uuid", + 2, true, 0, {}}, + 0, {}, 0, 0, {}}}, + }, 0); + expect_is_refresh_required(mock_remote_image_ctx, false); + + // wake-up replayer + update_watch_ctx->handle_notify(); + + // wait for sync to complete + ASSERT_EQ(0, wait_for_notification(2)); + + ASSERT_EQ(0, shut_down_entry_replayer(mock_replayer, mock_threads, + mock_local_image_ctx, + mock_remote_image_ctx)); +} + TEST_F(TestMockImageReplayerSnapshotReplayer, RemoteImageDemoted) { librbd::MockTestImageCtx mock_local_image_ctx{*m_local_image_ctx}; librbd::MockTestImageCtx mock_remote_image_ctx{*m_remote_image_ctx}; diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc index 1c7a90167a2d0..afabe6769c672 100644 --- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc +++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc @@ -516,7 +516,7 @@ void Replayer::scan_local_mirror_snapshots( } image_locker.unlock(); - if (m_local_snap_id_start > 0 && m_local_snap_id_end == CEPH_NOSNAP) { + if (m_local_snap_id_start > 0) { // remove candidate that is required for delta snapshot sync prune_snap_ids.erase(m_local_snap_id_start); } -- 2.39.5